Move phaseCount into MultiChannelStateSequence (#3520)

We can them drop the field from a bunch of callers, simplifying the code.

firmware/controllers/core/state_sequence.cpp

@@ -53,13 +53,13 @@ float MultiChannelStateSequence::getSwitchTime(const int index) const {
 	return switchTimes[index];
 }
 
-void MultiChannelStateSequence::checkSwitchTimes(const int size, const float scale) const {
+void MultiChannelStateSequence::checkSwitchTimes(const float scale) const {
-	if (switchTimes[size - 1] != 1) {
+	if (switchTimes[phaseCount - 1] != 1) {
 		firmwareError(CUSTOM_ERR_WAVE_1, "last switch time has to be 1/%f not %.2f/%f", scale,
-				switchTimes[size - 1], scale * switchTimes[size - 1]);
+				switchTimes[phaseCount - 1], scale * switchTimes[phaseCount - 1]);
 		return;
 	}
-	for (int i = 0; i < size - 1; i++) {
+	for (int i = 0; i < phaseCount - 1; i++) {
 		if (switchTimes[i] >= switchTimes[i + 1]) {
 			firmwareError(CUSTOM_ERR_WAVE_2, "invalid switchTimes @%d: %.2f/%.2f", i, switchTimes[i], switchTimes[i + 1]);
 		}
@@ -77,16 +77,16 @@ pin_state_t MultiChannelStateSequence::getChannelState(const int channelIndex, c
 /**
  * returns the index at which given value would need to be inserted into sorted array
  */
-int MultiChannelStateSequence::findInsertionAngle(const float angle, const int size) const {
+int MultiChannelStateSequence::findInsertionAngle(const float angle) const {
-	for (int i = size - 1; i >= 0; i--) {
+	for (int i = phaseCount - 1; i >= 0; i--) {
 		if (angle > switchTimes[i])
 			return i + 1;
 	}
 	return 0;
 }
 
-int MultiChannelStateSequence::findAngleMatch(const float angle, const int size) const {
+int MultiChannelStateSequence::findAngleMatch(const float angle) const {
-	for (int i = 0; i < size; i++) {
+	for (int i = 0; i < phaseCount; i++) {
 		if (isSameF(switchTimes[i], angle))
 			return i;
 	}

firmware/controllers/core/state_sequence.h

@@ -8,6 +8,7 @@
 #pragma once
 
 #include "rusefi_enums.h"
+#include <stdint.h>
 
 /**
  * This layer has two primary usages:
@@ -72,16 +73,17 @@ public:
 	void reset(void);
 	float getSwitchTime(const int phaseIndex) const;
 	void setSwitchTime(const int phaseIndex, const float value);
-	void checkSwitchTimes(const int size, const float scale) const;
+	void checkSwitchTimes(const float scale) const;
 	pin_state_t getChannelState(const int channelIndex, const int phaseIndex) const;
 
-	int findAngleMatch(const float angle, const int size) const;
+	int findAngleMatch(const float angle) const;
-	int findInsertionAngle(const float angle, const int size) const;
+	int findInsertionAngle(const float angle) const;
 
 	/**
 	 * Number of signal channels
 	 */
-	int waveCount;
+	uint16_t phaseCount;
+	uint16_t waveCount;
 	SingleChannelStateSequence *channels = nullptr;
 //private:
 	/**

firmware/controllers/system/timer/pwm_generator_logic.cpp

@@ -23,6 +23,7 @@ SimplePwm::SimplePwm()
 	, seq(_switchTimes, &sr)
 {
 	seq.waveCount = 1;
+	seq.phaseCount = 2;
 }
 
 SimplePwm::SimplePwm(const char *name) : SimplePwm()  {
@@ -36,7 +37,6 @@ PwmConfig::PwmConfig() {
 	periodNt = NAN;
 	mode = PM_NORMAL;
 	memset(&outputPins, 0, sizeof(outputPins));
-	phaseCount = 0;
 	pwmCycleCallback = nullptr;
 	stateChangeCallback = nullptr;
 	executor = nullptr;
@@ -198,7 +198,7 @@ efitick_t PwmConfig::togglePwmState() {
 	int cbStateIndex;
 	if (mode == PM_NORMAL) {
 		// callback state index is offset by one. todo: why? can we simplify this?
-		cbStateIndex = safe.phaseIndex == 0 ? phaseCount - 1 : safe.phaseIndex - 1;
+		cbStateIndex = safe.phaseIndex == 0 ? multiChannelStateSequence->phaseCount - 1 : safe.phaseIndex - 1;
 	} else if (mode == PM_ZERO) {
 		cbStateIndex = 0;
 	} else {
@@ -220,7 +220,7 @@ efitick_t PwmConfig::togglePwmState() {
 	bool isVeryBehindSchedule = nextSwitchTimeNt < getTimeNowNt() - MS2NT(10);
 
 	safe.phaseIndex++;
-	if (isVeryBehindSchedule || safe.phaseIndex == phaseCount || mode != PM_NORMAL) {
+	if (isVeryBehindSchedule || safe.phaseIndex == multiChannelStateSequence->phaseCount || mode != PM_NORMAL) {
 		safe.phaseIndex = 0; // restart
 		safe.iteration++;
 
@@ -265,11 +265,10 @@ static void timerCallback(PwmConfig *state) {
  * Incoming parameters are potentially just values on current stack, so we have to copy
  * into our own permanent storage, right?
  */
-void copyPwmParameters(PwmConfig *state, int phaseCount, MultiChannelStateSequence const * seq) {
+void copyPwmParameters(PwmConfig *state, MultiChannelStateSequence const * seq) {
-	state->phaseCount = phaseCount;
 	state->multiChannelStateSequence = seq;
 	if (state->mode == PM_NORMAL) {
-		state->multiChannelStateSequence->checkSwitchTimes(phaseCount, 1);
+		state->multiChannelStateSequence->checkSwitchTimes(1);
 	}
 }
 
@@ -278,7 +277,6 @@ void copyPwmParameters(PwmConfig *state, int phaseCount, MultiChannelStateSequen
  * See also startSimplePwm
  */
 void PwmConfig::weComplexInit(const char *msg, ExecutorInterface *executor,
-		const int phaseCount,
 		MultiChannelStateSequence const * seq,
 		pwm_cycle_callback *pwmCycleCallback, pwm_gen_callback *stateChangeCallback) {
 	UNUSED(msg);
@@ -286,11 +284,11 @@ void PwmConfig::weComplexInit(const char *msg, ExecutorInterface *executor,
 	isStopRequested = false;
 
 	efiAssertVoid(CUSTOM_ERR_6582, periodNt != 0, "period is not initialized");
-	if (phaseCount == 0) {
+	if (seq->phaseCount == 0) {
 		firmwareError(CUSTOM_ERR_PWM_1, "signal length cannot be zero");
 		return;
 	}
-	if (phaseCount > PWM_PHASE_MAX_COUNT) {
+	if (seq->phaseCount > PWM_PHASE_MAX_COUNT) {
 		firmwareError(CUSTOM_ERR_PWM_2, "too many phases in PWM");
 		return;
 	}
@@ -299,7 +297,7 @@ void PwmConfig::weComplexInit(const char *msg, ExecutorInterface *executor,
 	this->pwmCycleCallback = pwmCycleCallback;
 	this->stateChangeCallback = stateChangeCallback;
 
-	copyPwmParameters(this, phaseCount, seq);
+	copyPwmParameters(this, seq);
 
 	safe.phaseIndex = 0;
 	safe.periodNt = -1;
@@ -327,7 +325,7 @@ void startSimplePwm(SimplePwm *state, const char *msg, ExecutorInterface *execut
 
 	state->setFrequency(frequency);
 	state->setSimplePwmDutyCycle(dutyCycle); // TODO: DUP ABOVE?
-	state->weComplexInit(msg, executor, 2, &state->seq, NULL, (pwm_gen_callback*)applyPinState);
+	state->weComplexInit(msg, executor, &state->seq, NULL, (pwm_gen_callback*)applyPinState);
 }
 
 void startSimplePwmExt(SimplePwm *state, const char *msg,

firmware/controllers/system/timer/pwm_generator_logic.h

@@ -57,7 +57,7 @@ public:
 
 	void weComplexInit(const char *msg,
 			ExecutorInterface *executor,
-			const int phaseCount, MultiChannelStateSequence const * seq,
+			MultiChannelStateSequence const * seq,
 			pwm_cycle_callback *pwmCycleCallback,
 			pwm_gen_callback *callback);
 
@@ -88,10 +88,6 @@ public:
 	scheduling_s scheduling;
 
 	pwm_config_safe_state_s safe;
-	/**
-	 * Number of events in the cycle
-	 */
-	int phaseCount;
 
 	/**
 	 * this callback is invoked before each wave generation cycle
@@ -163,5 +159,5 @@ void startSimplePwmHard(SimplePwm *state, const char *msg,
 		brain_pin_e brainPin, OutputPin *output, float frequency,
 		float dutyCycle);
 
-void copyPwmParameters(PwmConfig *state, int phaseCount, MultiChannelStateSequence const * seq);
+void copyPwmParameters(PwmConfig *state, MultiChannelStateSequence const * seq);
 

firmware/controllers/trigger/decoders/trigger_structure.cpp

@@ -81,13 +81,13 @@ void TriggerWaveform::initialize(operation_mode_e operationMode) {
 	gapBothDirections = false;
 
 	this->operationMode = operationMode;
-	privateTriggerDefinitionSize = 0;
 	triggerShapeSynchPointIndex = 0;
 	memset(initialState, 0, sizeof(initialState));
 	memset(switchTimesBuffer, 0, sizeof(switchTimesBuffer));
 	memset(expectedEventCount, 0, sizeof(expectedEventCount));
 	wave.reset();
 	wave.waveCount = TRIGGER_CHANNEL_COUNT;
+	wave.phaseCount = 0;
 	previousAngle = 0;
 	memset(isRiseEvent, 0, sizeof(isRiseEvent));
 #if EFI_UNIT_TEST
@@ -97,7 +97,7 @@ void TriggerWaveform::initialize(operation_mode_e operationMode) {
 }
 
 size_t TriggerWaveform::getSize() const {
-	return privateTriggerDefinitionSize;
+	return wave.phaseCount;
 }
 
 int TriggerWaveform::getTriggerWaveformSynchPointIndex() const {
@@ -147,9 +147,9 @@ angle_t TriggerWaveform::getAngle(int index) const {
 	 * See also trigger_central.cpp
 	 * See also getEngineCycleEventCount()
 	 */
-	efiAssert(CUSTOM_ERR_ASSERT, privateTriggerDefinitionSize != 0, "shapeSize=0", NAN);
+	efiAssert(CUSTOM_ERR_ASSERT, wave.phaseCount != 0, "shapeSize=0", NAN);
-	int crankCycle = index / privateTriggerDefinitionSize;
+	int crankCycle = index / wave.phaseCount;
-	int remainder = index % privateTriggerDefinitionSize;
+	int remainder = index % wave.phaseCount;
 
 	auto cycleStartAngle = getCycleDuration() * crankCycle;
 	auto positionWithinCycle = getSwitchAngle(remainder);
@@ -229,9 +229,9 @@ void TriggerWaveform::addEvent(angle_t angle, trigger_wheel_e const channelIndex
 #endif
 
 #if EFI_UNIT_TEST
-	assertIsInBounds(privateTriggerDefinitionSize, triggerSignalIndeces, "trigger shape overflow");
+	assertIsInBounds(wave.phaseCount, triggerSignalIndeces, "trigger shape overflow");
-	triggerSignalIndeces[privateTriggerDefinitionSize] = channelIndex;
+	triggerSignalIndeces[wave.phaseCount] = channelIndex;
-	triggerSignalStates[privateTriggerDefinitionSize] = state;
+	triggerSignalStates[wave.phaseCount] = state;
 #endif // EFI_UNIT_TEST
 
 
@@ -243,21 +243,21 @@ void TriggerWaveform::addEvent(angle_t angle, trigger_wheel_e const channelIndex
 	}
 
 	efiAssertVoid(CUSTOM_ERR_6599, angle > 0 && angle <= 1, "angle should be positive not above 1");
-	if (privateTriggerDefinitionSize > 0) {
+	if (wave.phaseCount > 0) {
 		if (angle <= previousAngle) {
 			warning(CUSTOM_ERR_TRG_ANGLE_ORDER, "invalid angle order %s %s: new=%.2f/%f and prev=%.2f/%f, size=%d",
 					getTrigger_wheel_e(channelIndex),
 					getTrigger_value_e(state),
 					angle, angle * getCycleDuration(),
 					previousAngle, previousAngle * getCycleDuration(),
-					privateTriggerDefinitionSize);
+					wave.phaseCount);
 			setShapeDefinitionError(true);
 			return;
 		}
 	}
 	previousAngle = angle;
-	if (privateTriggerDefinitionSize == 0) {
+	if (wave.phaseCount == 0) {
-		privateTriggerDefinitionSize = 1;
+		wave.phaseCount = 1;
 		for (int i = 0; i < PWM_PHASE_MAX_WAVE_PER_PWM; i++) {
 			SingleChannelStateSequence *wave = &this->wave.channels[i];
 
@@ -275,14 +275,14 @@ void TriggerWaveform::addEvent(angle_t angle, trigger_wheel_e const channelIndex
 		return;
 	}
 
-	int exactMatch = wave.findAngleMatch(angle, privateTriggerDefinitionSize);
+	int exactMatch = wave.findAngleMatch(angle);
 	if (exactMatch != (int)EFI_ERROR_CODE) {
 		warning(CUSTOM_ERR_SAME_ANGLE, "same angle: not supported");
 		setShapeDefinitionError(true);
 		return;
 	}
 
-	int index = wave.findInsertionAngle(angle, privateTriggerDefinitionSize);
+	int index = wave.findInsertionAngle(angle);
 
 	/**
 	 * todo: it would be nice to be able to provide trigger angles without sorting them externally
@@ -299,11 +299,11 @@ void TriggerWaveform::addEvent(angle_t angle, trigger_wheel_e const channelIndex
 */
 	isRiseEvent[index] = TV_RISE == state;
 
-	if ((unsigned)index != privateTriggerDefinitionSize) {
+	if ((unsigned)index != wave.phaseCount) {
 		firmwareError(ERROR_TRIGGER_DRAMA, "are we ever here?");
 	}
 
-	privateTriggerDefinitionSize++;
+	wave.phaseCount++;
 
 	for (int i = 0; i < PWM_PHASE_MAX_WAVE_PER_PWM; i++) {
 		pin_state_t value = wave.getChannelState(/* channelIndex */i, index - 1);
@@ -752,7 +752,7 @@ void TriggerWaveform::initializeTriggerWaveform(operation_mode_e ambiguousOperat
 	version++;
 
 	if (!shapeDefinitionError) {
-		wave.checkSwitchTimes(getSize(), getCycleDuration());
+		wave.checkSwitchTimes(getCycleDuration());
 	}
 
 	if (bothFrontsRequired && useOnlyRisingEdgeForTrigger) {

firmware/controllers/trigger/decoders/trigger_structure.h

@@ -193,6 +193,13 @@ public:
 	int triggerSignalStates[PWM_PHASE_MAX_COUNT];
 #endif
 
+	/**
+	 * wave.phaseCount is total count of shaft events per CAM or CRANK shaft revolution.
+	 * TODO this should be migrated to CRANKshaft revolution, this would go together
+	 * this variable is public for performance reasons (I want to avoid costs of method if it's not inlined)
+	 * but name is supposed to hint at the fact that decoders should not be assigning to it
+	 * Please use "getTriggerSize()" macro or "getSize()" method to read this value
+	 */
 	MultiChannelStateSequence wave;
 
 	// todo: add a runtime validation which would verify that this field was set properly
@@ -201,15 +208,6 @@ public:
 
 	bool isRiseEvent[PWM_PHASE_MAX_COUNT];
 
-	/**
-	 * Total count of shaft events per CAM or CRANK shaft revolution.
-	 * TODO this should be migrated to CRANKshaft revolution, this would go together
-	 * this variable is public for performance reasons (I want to avoid costs of method if it's not inlined)
-	 * but name is supposed to hint at the fact that decoders should not be assigning to it
-	 * Please use "getTriggerSize()" macro or "getSize()" method to read this value
-	 */
-	unsigned int privateTriggerDefinitionSize;
-
 	bool useOnlyRisingEdgeForTriggerTemp;
 
 	/* (0..1] angle range */
@@ -325,4 +323,4 @@ void setToothedWheelConfiguration(TriggerWaveform *s, int total, int skipped, op
 
 #define TRIGGER_WAVEFORM(x) ENGINE(triggerCentral.triggerShape).x
 
-#define getTriggerSize() TRIGGER_WAVEFORM(privateTriggerDefinitionSize)
+#define getTriggerSize() TRIGGER_WAVEFORM(wave.phaseCount)

firmware/controllers/trigger/trigger_emulator_algo.cpp

@@ -20,8 +20,8 @@ int getPreviousIndex(const int currentIndex, const int size) {
 	return (currentIndex + size - 1) % size;
 }
 
-bool needEvent(const int currentIndex, const int size, const MultiChannelStateSequence& mcss, int channelIndex) {
+bool needEvent(const int currentIndex, const MultiChannelStateSequence& mcss, int channelIndex) {
-	int prevIndex = getPreviousIndex(currentIndex, size);
+	int prevIndex = getPreviousIndex(currentIndex, mcss.phaseCount);
 	pin_state_t previousValue = mcss.getChannelState(channelIndex, /*phaseIndex*/prevIndex);
 	pin_state_t currentValue = mcss.getChannelState(channelIndex, /*phaseIndex*/currentIndex);
 
@@ -39,13 +39,13 @@ TriggerEmulatorHelper::TriggerEmulatorHelper() {
 
 static OutputPin emulatorOutputs[PWM_PHASE_MAX_WAVE_PER_PWM];
 
-void TriggerEmulatorHelper::handleEmulatorCallback(const int size, const MultiChannelStateSequence& multiChannelStateSequence, int stateIndex DECLARE_ENGINE_PARAMETER_SUFFIX) {
+void TriggerEmulatorHelper::handleEmulatorCallback(const MultiChannelStateSequence& multiChannelStateSequence, int stateIndex DECLARE_ENGINE_PARAMETER_SUFFIX) {
 	efitick_t stamp = getTimeNowNt();
 	
 	// todo: code duplication with TriggerStimulatorHelper::feedSimulatedEvent?
 
 	for (size_t i = 0; i < PWM_PHASE_MAX_WAVE_PER_PWM; i++) {
-		if (needEvent(stateIndex, size, multiChannelStateSequence, i)) {
+		if (needEvent(stateIndex, multiChannelStateSequence, i)) {
 			pin_state_t currentValue = multiChannelStateSequence.getChannelState(/*phaseIndex*/i, stateIndex);
 
 			handleShaftSignal(i, currentValue, stamp PASS_ENGINE_PARAMETER_SUFFIX);
@@ -106,7 +106,7 @@ static void updateTriggerWaveformIfNeeded(PwmConfig *state DECLARE_ENGINE_PARAME
 
 
 		TriggerWaveform *s = &engine->triggerCentral.triggerShape;
-		copyPwmParameters(state, s->getSize(), &s->wave);
+		copyPwmParameters(state, &s->wave);
 		state->safe.periodNt = -1; // this would cause loop re-initialization
 	}
 }
@@ -123,7 +123,7 @@ static void emulatorApplyPinState(int stateIndex, PwmConfig *state) /* pwm_gen_c
 		/**
 		 * this callback would invoke the input signal handlers directly
 		 */
-		helper.handleEmulatorCallback(state->phaseCount,
+		helper.handleEmulatorCallback(
 				*state->multiChannelStateSequence,
 				stateIndex);
 	}
@@ -144,10 +144,9 @@ static void initTriggerPwm(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
 
 	TriggerWaveform *s = &engine->triggerCentral.triggerShape;
 	setTriggerEmulatorRPM(engineConfiguration->triggerSimulatorFrequency PASS_ENGINE_PARAMETER_SUFFIX);
-	int phaseCount = s->getSize();
 	triggerSignal.weComplexInit("position sensor",
 			&engine->executor,
-			phaseCount, &s->wave,
+			&s->wave,
 			updateTriggerWaveformIfNeeded, (pwm_gen_callback*)emulatorApplyPinState);
 
 	hasInitTriggerEmulator = true;

firmware/controllers/trigger/trigger_emulator_algo.h

@@ -24,11 +24,11 @@ void disableTriggerStimulator();
 class TriggerEmulatorHelper {
 public:
     TriggerEmulatorHelper();
-	void handleEmulatorCallback(const int size, const MultiChannelStateSequence& mcss, int stateIndex DECLARE_ENGINE_PARAMETER_SUFFIX);
+	void handleEmulatorCallback(const MultiChannelStateSequence& mcss, int stateIndex DECLARE_ENGINE_PARAMETER_SUFFIX);
 };
 
 void initTriggerEmulatorLogic(DECLARE_ENGINE_PARAMETER_SIGNATURE);
 
 int getPreviousIndex(const int currentIndex, const int size);
-bool needEvent(const int currentIndex, const int size, const MultiChannelStateSequence& mcss, int channelIndex);
+bool needEvent(const int currentIndex, const MultiChannelStateSequence& mcss, int channelIndex);
 

firmware/controllers/trigger/trigger_simulator.cpp

@@ -45,7 +45,6 @@ void TriggerStimulatorHelper::feedSimulatedEvent(
 		) {
 	efiAssertVoid(CUSTOM_ERR_6593, shape.getSize() > 0, "size not zero");
 	int stateIndex = i % shape.getSize();
-	int size = shape.getSize();
 
 	int time = getSimulatedEventTime(shape, i);
 
@@ -77,7 +76,7 @@ void TriggerStimulatorHelper::feedSimulatedEvent(
 
 
 	for (size_t i = 0; i < PWM_PHASE_MAX_WAVE_PER_PWM; i++) {
-		if (needEvent(stateIndex, size, multiChannelStateSequence, i)) {
+		if (needEvent(stateIndex, multiChannelStateSequence, i)) {
 			pin_state_t currentValue = multiChannelStateSequence.getChannelState(/*phaseIndex*/i, stateIndex);
 			trigger_event_e event = (currentValue ? riseEvents : fallEvents)[i];
 			if (isUsefulSignal(event, triggerConfiguration)) {

unit_tests/tests/ignition_injection/test_fuel_map.cpp

@@ -121,12 +121,12 @@ static void confgiureFordAspireTriggerWaveform(TriggerWaveform * s) {
 	ASSERT_FLOAT_EQ(121.90 / 720, s->wave.getSwitchTime(1));
 	ASSERT_FLOAT_EQ(657.03 / 720, s->wave.getSwitchTime(8));
 
-	ASSERT_EQ( 0,  s->wave.findAngleMatch(53.747 / 720.0, s->getSize())) << "expecting 0";
+	ASSERT_EQ( 0,  s->wave.findAngleMatch(53.747 / 720.0)) << "expecting 0";
-	assertEqualsM("expecting not found", -1, s->wave.findAngleMatch(53 / 720.0, s->getSize()));
+	assertEqualsM("expecting not found", -1, s->wave.findAngleMatch(53 / 720.0));
-	ASSERT_EQ(7, s->wave.findAngleMatch(588.045 / 720.0, s->getSize()));
+	ASSERT_EQ(7, s->wave.findAngleMatch(588.045 / 720.0));
 
-	ASSERT_EQ( 0,  s->wave.findInsertionAngle(23.747 / 720.0, s->getSize())) << "expecting 0";
+	ASSERT_EQ( 0,  s->wave.findInsertionAngle(23.747 / 720.0)) << "expecting 0";
-	ASSERT_EQ( 1,  s->wave.findInsertionAngle(63.747 / 720.0, s->getSize())) << "expecting 1";
+	ASSERT_EQ( 1,  s->wave.findInsertionAngle(63.747 / 720.0)) << "expecting 1";
 }
 
 TEST(misc, testAngleResolver) {

unit_tests/tests/trigger/test_miata_na_tdc.cpp

@@ -20,7 +20,7 @@ TEST(miata, miata_na_tdc) {
 		int time = getSimulatedEventTime(shape, i);
 		eth.setTimeAndInvokeEventsUs(time);
 
-		emulatorHelper.handleEmulatorCallback(shape.getSize(),
+		emulatorHelper.handleEmulatorCallback(
 				shape.wave,
 				i  % shape.getSize() PASS_ENGINE_PARAMETER_SUFFIX);
 	}