remove trigger duty cycle calculation (#4213)

* This field was ignored.

* move pad out

* gone

* make trigger configuration a little clearer

* even simpler!

* format

* test fix

* remove duty cycle thing

firmware/controllers/trigger/decoders/trigger_structure.cpp

@@ -59,7 +59,6 @@ void TriggerWaveform::initialize(operation_mode_e operationMode) {
 	isSecondWheelCam = false;
 	needSecondTriggerInput = false;
 	shapeWithoutTdc = false;
-	memset(expectedDutyCycle, 0, sizeof(expectedDutyCycle));
 
 	setTriggerSynchronizationGap(2);
 	for (int gapIndex = 1; gapIndex < GAP_TRACKING_LENGTH ; gapIndex++) {

firmware/controllers/trigger/decoders/trigger_structure.h

@@ -121,12 +121,6 @@ public:
 	 */
 	int version = 0;
 
-	/**
-	 * duty cycle for each individual trigger channel
-	 */
-	float expectedDutyCycle[PWM_PHASE_MAX_WAVE_PER_PWM];
-
-
 	/**
 	 * Depending on trigger shape, we use betweeb one and three previous gap ranges to detect synchronizaiton.
 	 *

firmware/controllers/trigger/trigger_central.cpp

@@ -299,7 +299,6 @@ void hwHandleVvtCamSignal(trigger_value_e front, efitick_t nowNt, int index) {
 				"vvt",
 			tc->vvtShape[camIndex],
 			nullptr,
-			nullptr,
 			engine->vvtTriggerConfiguration[camIndex],
 			front == TV_RISE ? SHAFT_PRIMARY_RISING : SHAFT_PRIMARY_FALLING, nowNt);
 		// yes we log data from all VVT channels into same fields for now
@@ -677,7 +676,6 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timesta
 	auto decodeResult = triggerState.decodeTriggerEvent(
 			"trigger",
 			triggerShape,
-			nullptr,
 			engine,
 			engine->primaryTriggerConfiguration,
 			signal, timestamp);
@@ -799,7 +797,6 @@ void triggerInfo(void) {
 
 	efiPrintf("trigger type=%d/need2ndChannel=%s", engineConfiguration->trigger.type,
 			boolToString(TRIGGER_WAVEFORM(needSecondTriggerInput)));
-	efiPrintf("expected duty #0=%.2f/#1=%.2f", TRIGGER_WAVEFORM(expectedDutyCycle[0]), TRIGGER_WAVEFORM(expectedDutyCycle[1]));
 
 	efiPrintf("synchronizationNeeded=%s/isError=%s/total errors=%d ord_err=%d/total revolutions=%d/self=%s",
 			boolToString(ts->isSynchronizationNeeded),

firmware/controllers/trigger/trigger_decoder.cpp

@@ -74,7 +74,6 @@ void TriggerDecoderBase::resetTriggerState() {
 	startOfCycleNt = 0;
 
 	resetCurrentCycleState();
-	memset(expectedTotalTime, 0, sizeof(expectedTotalTime));
 
 	totalEventCountBase = 0;
 	isFirstEvent = true;
@@ -86,11 +85,6 @@ void TriggerDecoderBase::setTriggerErrorState() {
 
 void TriggerDecoderBase::resetCurrentCycleState() {
 	memset(currentCycle.eventCount, 0, sizeof(currentCycle.eventCount));
-	memset(currentCycle.timeOfPreviousEventNt, 0, sizeof(currentCycle.timeOfPreviousEventNt));
-#if EFI_UNIT_TEST
-	memcpy(currentCycle.totalTimeNtCopy, currentCycle.totalTimeNt, sizeof(currentCycle.totalTimeNt));
-#endif
-	memset(currentCycle.totalTimeNt, 0, sizeof(currentCycle.totalTimeNt));
 	currentCycle.current_index = 0;
 }
 
@@ -349,15 +343,6 @@ static trigger_value_e eventType[6] = { TV_FALL, TV_RISE, TV_FALL, TV_RISE, TV_F
 
 #define nextTriggerEvent() \
  { \
-	uint32_t prevTime = currentCycle.timeOfPreviousEventNt[triggerWheel]; \
-	if (prevTime != 0) { \
-		/* even event - apply the value*/ \
-		currentCycle.totalTimeNt[triggerWheel] += (nowNt - prevTime); \
-		currentCycle.timeOfPreviousEventNt[triggerWheel] = 0; \
-	} else { \
-		/* odd event - start accumulation */ \
-		currentCycle.timeOfPreviousEventNt[triggerWheel] = nowNt; \
-	} \
 	if (triggerConfiguration.UseOnlyRisingEdgeForTrigger) {currentCycle.current_index++;} \
 	currentCycle.current_index++; \
 	PRINT_INC_INDEX; \
@@ -435,16 +420,9 @@ bool TriggerDecoderBase::validateEventCounters(const TriggerWaveform& triggerSha
 }
 
 void TriggerDecoderBase::onShaftSynchronization(
-		const TriggerStateCallback triggerCycleCallback,
 		bool wasSynchronized,
 		const efitick_t nowNt,
 		const TriggerWaveform& triggerShape) {
-
-
-	if (triggerCycleCallback) {
-		triggerCycleCallback(this);
-	}
-
 	startOfCycleNt = nowNt;
 	resetCurrentCycleState();
 
@@ -477,7 +455,6 @@ void TriggerDecoderBase::onShaftSynchronization(
 expected<TriggerDecodeResult> TriggerDecoderBase::decodeTriggerEvent(
 		const char *msg,
 		const TriggerWaveform& triggerShape,
-		const TriggerStateCallback triggerCycleCallback,
 		TriggerStateListener* triggerStateListener,
 		const TriggerConfiguration& triggerConfiguration,
 		const trigger_event_e signal,
@@ -582,14 +559,6 @@ expected<TriggerDecodeResult> TriggerDecoderBase::decodeTriggerEvent(
 
 				int rpm = Sensor::getOrZero(SensorType::Rpm);
 				floatms_t engineCycleDuration = getEngineCycleDuration(rpm);
-				if (!engineConfiguration->useOnlyRisingEdgeForTrigger) {
-					int time = currentCycle.totalTimeNt[0];
-					efiPrintf("%s duty %f %d",
-						name,
-						time / engineCycleDuration,
-						time
-						);
-				}
 
 				for (int i = 0;i<triggerShape.gapTrackingLength;i++) {
 					float ratioFrom = triggerShape.syncronizationRatioFrom[i];
@@ -693,7 +662,7 @@ expected<TriggerDecodeResult> TriggerDecoderBase::decodeTriggerEvent(
 			nextTriggerEvent()
 			;
 
-			onShaftSynchronization(triggerCycleCallback, wasSynchronized, nowNt, triggerShape);
+			onShaftSynchronization(wasSynchronized, nowNt, triggerShape);
 		} else {	/* if (!isSynchronizationPoint) */
 			nextTriggerEvent()
 			;
@@ -791,13 +760,6 @@ bool TriggerDecoderBase::isSyncPoint(const TriggerWaveform& triggerShape, trigge
 	return true;
 }
 
-static void onFindIndexCallback(TriggerDecoderBase *state) {
-	for (int i = 0; i < PWM_PHASE_MAX_WAVE_PER_PWM; i++) {
-		// todo: that's not the best place for this intermediate data storage, fix it!
-		state->expectedTotalTime[i] = state->currentCycle.totalTimeNt[i];
-	}
-}
-
 /**
  * Trigger shape is defined in a way which is convenient for trigger shape definition
  * On the other hand, trigger decoder indexing begins from synchronization event.
@@ -836,14 +798,7 @@ uint32_t TriggerDecoderBase::findTriggerZeroEventIndex(
 	}
 #endif /* EFI_UNIT_TEST */
 
-	/**
-	 * Now that we have just located the synch point, we can simulate the whole cycle
-	 * in order to calculate expected duty cycle
-	 *
-	 * todo: add a comment why are we doing '2 * shape->getSize()' here?
-	 */
-
-	helper.assertSyncPositionAndSetDutyCycle(onFindIndexCallback, triggerConfiguration,
+	helper.assertSyncPosition(triggerConfiguration,
 			syncIndex, *this, shape);
 
 	return syncIndex % shape.getSize();

firmware/controllers/trigger/trigger_decoder.h

@@ -39,8 +39,6 @@ protected:
 	virtual trigger_config_s getType() const = 0;
 };
 
-typedef void (*TriggerStateCallback)(TriggerDecoderBase*);
-
 typedef struct {
 	/**
 	 * index within trigger revolution, from 0 to trigger event count
@@ -52,22 +50,7 @@ typedef struct {
 	 * see TriggerWaveform
 	 */
 	size_t eventCount[PWM_PHASE_MAX_WAVE_PER_PWM];
-	/**
-	 * This array is used to calculate duty cycle of each trigger channel.
-	 * Current implementation is a bit funny - it does not really consider if an event
-	 * is a rise or a fall, it works based on the event order within synchronization cycle.
-	 *
-	 * 32 bit value is good enough here, overflows will happen but they would work just fine.
-	 */
-	uint32_t timeOfPreviousEventNt[PWM_PHASE_MAX_WAVE_PER_PWM];
-	/**
-	 * Here we accumulate the amount of time this signal was ON within current trigger cycle
-	 */
-	uint32_t totalTimeNt[PWM_PHASE_MAX_WAVE_PER_PWM];
 
-#if EFI_UNIT_TEST
-	uint32_t totalTimeNtCopy[PWM_PHASE_MAX_WAVE_PER_PWM];
-#endif // EFI_UNIT_TEST
 } current_cycle_state_s;
 
 struct TriggerDecodeResult {
@@ -95,14 +78,12 @@ public:
 	expected<TriggerDecodeResult> decodeTriggerEvent(
 			const char *msg,
 			const TriggerWaveform& triggerShape,
-			const TriggerStateCallback triggerCycleCallback,
 			TriggerStateListener* triggerStateListener,
 			const TriggerConfiguration& triggerConfiguration,
 			const trigger_event_e signal,
 			const efitime_t nowUs);
 
 	void onShaftSynchronization(
-			const TriggerStateCallback triggerCycleCallback,
 			bool wasSynchronized,
 			const efitick_t nowNt,
 			const TriggerWaveform& triggerShape);
@@ -129,8 +110,6 @@ public:
 	current_cycle_state_s currentCycle;
 	const char *name = nullptr;
 
-	int expectedTotalTime[PWM_PHASE_MAX_WAVE_PER_PWM];
-
 	/**
 	 * how many times since ECU reboot we had unexpected number of teeth in trigger cycle
 	 */

firmware/controllers/trigger/trigger_simulator.cpp

@@ -37,7 +37,6 @@ int getSimulatedEventTime(const TriggerWaveform& shape, int i) {
 }
 
 void TriggerStimulatorHelper::feedSimulatedEvent(
-		const TriggerStateCallback triggerCycleCallback,
 		const TriggerConfiguration& triggerConfiguration,
 		TriggerDecoderBase& state,
 		const TriggerWaveform& shape,
@@ -83,7 +82,6 @@ void TriggerStimulatorHelper::feedSimulatedEvent(
 				state.decodeTriggerEvent(
 					"sim",
 						shape,
-					triggerCycleCallback,
 					/* override */ nullptr,
 					triggerConfiguration,
 					event, time);
@@ -93,8 +91,7 @@ void TriggerStimulatorHelper::feedSimulatedEvent(
 
 }
 
-void TriggerStimulatorHelper::assertSyncPositionAndSetDutyCycle(
-		const TriggerStateCallback triggerCycleCallback,
+void TriggerStimulatorHelper::assertSyncPosition(
 		const TriggerConfiguration& triggerConfiguration,
 		const uint32_t syncIndex,
 		TriggerDecoderBase& state,
@@ -108,10 +105,9 @@ void TriggerStimulatorHelper::assertSyncPositionAndSetDutyCycle(
 	 * let's feed two more cycles to validate shape definition
 	 */
 	for (uint32_t i = syncIndex + 1; i <= syncIndex + TEST_REVOLUTIONS * shape.getSize(); i++) {
-		feedSimulatedEvent(triggerCycleCallback,
-				triggerConfiguration,
-				state, shape, i);
+		feedSimulatedEvent(triggerConfiguration, state, shape, i);
 	}
+
 	int revolutionCounter = state.getTotalRevolutionCounter();
 	if (revolutionCounter != TEST_REVOLUTIONS) {
 		warning(CUSTOM_OBD_TRIGGER_WAVEFORM, "sync failed/wrong gap parameters trigger=%s revolutionCounter=%d",
@@ -128,11 +124,6 @@ void TriggerStimulatorHelper::assertSyncPositionAndSetDutyCycle(
 					revolutionCounter);
 		}
 #endif /* EFI_UNIT_TEST */
-
-
-	for (int i = 0; i < PWM_PHASE_MAX_WAVE_PER_PWM; i++) {
-		shape.expectedDutyCycle[i] = 1.0 * state.expectedTotalTime[i] / SIMULATION_CYCLE_PERIOD;
-	}
 }
 
 /**
@@ -143,9 +134,7 @@ uint32_t TriggerStimulatorHelper::findTriggerSyncPoint(
 		const TriggerConfiguration& triggerConfiguration,
 		TriggerDecoderBase& state) {
 	for (int i = 0; i < 4 * PWM_PHASE_MAX_COUNT; i++) {
-		feedSimulatedEvent(nullptr,
-				triggerConfiguration,
-				state, shape, i);
+		feedSimulatedEvent(triggerConfiguration, state, shape, i);
 
 		if (state.getShaftSynchronized()) {
 			return i;

firmware/controllers/trigger/trigger_simulator.h

@@ -22,8 +22,7 @@ public:
 			const TriggerConfiguration& triggerConfiguration,
 			TriggerDecoderBase& state);
 
-	void assertSyncPositionAndSetDutyCycle(
-			const TriggerStateCallback triggerCycleCallback,
+	void assertSyncPosition(
 			const TriggerConfiguration& triggerConfiguration,
 			const uint32_t index,
 			TriggerDecoderBase& state,
@@ -31,7 +30,7 @@ public:
 			);
 
 	// send next event so that we can see how state reacts
-	void feedSimulatedEvent(const TriggerStateCallback triggerCycleCallback,
+	void feedSimulatedEvent(
 			const TriggerConfiguration& triggerConfiguration,
 			TriggerDecoderBase& state,
 			const TriggerWaveform& shape,

unit_tests/tests/trigger/test_all_triggers.cpp

@@ -73,7 +73,6 @@ TEST_P(AllTriggersFixture, TestTrigger) {
 
 	fprintf(fp, "%s=%s\n", TRIGGER_HAS_SECOND_CHANNEL, shape->needSecondTriggerInput ? "true" : "false");
 	fprintf(fp, "%s=%s\n", TRIGGER_IS_SECOND_WHEEL_CAM, shape->isSecondWheelCam ? "true" : "false");
-	fprintf(fp, "# duty %.2f %.2f\n", shape->expectedDutyCycle[0], shape->expectedDutyCycle[1]);
 
 	for (size_t i = 0; i < shape->getLength(); i++) {
 		int triggerDefinitionCoordinate = (shape->getTriggerWaveformSynchPointIndex() + i) % shape->getSize();

unit_tests/tests/trigger/test_cam_vvt_input.cpp

@@ -177,9 +177,5 @@ TEST(trigger, testNB2CamInput) {
 	// actually position based on VVT!
 	ASSERT_EQ(totalRevolutionCountBeforeVvtSync + 3, engine->triggerCentral.triggerState.getTotalRevolutionCounter());
 
-	float dutyCycleNt = engine->triggerCentral.vvtState[0][0].currentCycle.totalTimeNtCopy[0];
-	EXPECT_FLOAT_EQ(27'000'000, dutyCycleNt);
-	EXPECT_FLOAT_EQ(0.056944445f, engine->triggerCentral.vvtShape[0].expectedDutyCycle[0]);
-
 	EXPECT_EQ(40, waveChart.getSize());
 }

unit_tests/tests/trigger/test_trigger_decoder.cpp

@@ -119,28 +119,28 @@ TEST(trigger, testSomethingWeird) {
 
 	ASSERT_FALSE(sta->shaft_is_synchronized) << "shaft_is_synchronized";
 	int r = 10;
-	sta->decodeTriggerEvent("t", engine->triggerCentral.triggerShape, nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r);
+	sta->decodeTriggerEvent("t", engine->triggerCentral.triggerShape, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r);
 	ASSERT_FALSE(sta->shaft_is_synchronized) << "shaft_is_synchronized"; // still no synchronization
-	sta->decodeTriggerEvent("t", engine->triggerCentral.triggerShape, nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, ++r);
+	sta->decodeTriggerEvent("t", engine->triggerCentral.triggerShape, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, ++r);
 	ASSERT_TRUE(sta->shaft_is_synchronized); // first signal rise synchronize
 	ASSERT_EQ(0, sta->getCurrentIndex());
-	sta->decodeTriggerEvent("t", engine->triggerCentral.triggerShape, nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r++);
+	sta->decodeTriggerEvent("t", engine->triggerCentral.triggerShape, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r++);
 	ASSERT_EQ(1, sta->getCurrentIndex());
 
 	for (int i = 2; i < 10;) {
-		sta->decodeTriggerEvent("t", engine->triggerCentral.triggerShape, nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, r++);
+		sta->decodeTriggerEvent("t", engine->triggerCentral.triggerShape, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, r++);
 		assertEqualsM("even", i++, sta->getCurrentIndex());
-		sta->decodeTriggerEvent("t", engine->triggerCentral.triggerShape, nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r++);
+		sta->decodeTriggerEvent("t", engine->triggerCentral.triggerShape, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r++);
 		assertEqualsM("odd", i++, sta->getCurrentIndex());
 	}
 
-	sta->decodeTriggerEvent("test", engine->triggerCentral.triggerShape, nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, r++);
+	sta->decodeTriggerEvent("test", engine->triggerCentral.triggerShape, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, r++);
 	ASSERT_EQ(10, sta->getCurrentIndex());
 
-	sta->decodeTriggerEvent("test", engine->triggerCentral.triggerShape, nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r++);
+	sta->decodeTriggerEvent("test", engine->triggerCentral.triggerShape, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r++);
 	ASSERT_EQ(11, sta->getCurrentIndex());
 
-	sta->decodeTriggerEvent("test", engine->triggerCentral.triggerShape, nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, r++);
+	sta->decodeTriggerEvent("test", engine->triggerCentral.triggerShape, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, r++);
 	ASSERT_EQ(0, sta->getCurrentIndex()); // new revolution
 }
 
@@ -236,9 +236,6 @@ static void testTriggerDecoder2(const char *msg, engine_type_e type, int synchPo
 	ASSERT_FALSE(t->shapeDefinitionError) << "isError";
 
 	assertEqualsM("synchPointIndex", synchPointIndex, t->getTriggerWaveformSynchPointIndex());
-
-	ASSERT_NEAR(channel1duty, t->expectedDutyCycle[0], 0.0001) << msg << " channel1duty";
-	ASSERT_NEAR(channel2duty, t->expectedDutyCycle[1], 0.0001) << msg << " channel2duty";
 }
 
 static void testTriggerDecoder3(const char *msg, engine_type_e type, int synchPointIndex, float channel1duty, float channel2duty, float expectedGap) {

unit_tests/tests/trigger/test_trigger_decoder_2.cpp

@@ -39,7 +39,7 @@ static auto makeTriggerShape(operation_mode_e mode, const TriggerConfiguration&
 	return shape;
 }
 
-#define doTooth(dut, shape, cfg, t) dut.decodeTriggerEvent("", shape, nullptr, nullptr, cfg, SHAFT_PRIMARY_RISING, t)
+#define doTooth(dut, shape, cfg, t) dut.decodeTriggerEvent("", shape, nullptr, cfg, SHAFT_PRIMARY_RISING, t)
 
 TEST(TriggerDecoder, FindsFirstSyncPoint) {
 	MockTriggerConfiguration cfg(true, {TT_TOOTHED_WHEEL, 4, 1});