correctly compute engine revolution number (#3586)

* add param

* first revolution is revolution 0, not revolution 1

* would you look at that, we sync one rev quicker!

* changelog

* rpm value

firmware/CHANGELOG.md

@@ -27,6 +27,9 @@ All notable user-facing or behavior-altering changes will be documented in this
 
 ## Month 202x Release - "Release Name"
 
+### Fixed
+ - Faster engine sync + startup on engines with crank-speed primary trigger
+
 ## November 2021 Release - "Loosen Up Lighten Up Day"
 
 ### Added

firmware/controllers/trigger/trigger_decoder.cpp

@@ -420,6 +420,7 @@ bool TriggerState::validateEventCounters(const TriggerWaveform& triggerShape) co
 
 void TriggerState::onShaftSynchronization(
 		const TriggerStateCallback triggerCycleCallback,
+		bool wasSynchronized,
 		const efitick_t nowNt,
 		const TriggerWaveform& triggerShape) {
 
@@ -430,7 +431,14 @@ void TriggerState::onShaftSynchronization(
 
 	startOfCycleNt = nowNt;
 	resetCurrentCycleState();
-	incrementTotalEventCounter();
+
+	if (wasSynchronized) {
+		incrementTotalEventCounter();
+	} else {
+		// We have just synchronized, this is the zeroth revolution
+		totalRevolutionCounter = 0;
+	}
+
 	totalEventCountBase += triggerShape.getSize();
 
 #if EFI_UNIT_TEST
@@ -662,7 +670,7 @@ void TriggerState::decodeTriggerEvent(
 			nextTriggerEvent()
 			;
 
-			onShaftSynchronization(triggerCycleCallback, nowNt, triggerShape);
+			onShaftSynchronization(triggerCycleCallback, wasSynchronized, nowNt, triggerShape);
 
 		} else {	/* if (!isSynchronizationPoint) */
 			nextTriggerEvent()
@@ -721,8 +729,6 @@ uint32_t TriggerState::findTriggerZeroEventIndex(
 		return 0;
 	}
 
-
-	// todo: should this variable be declared 'static' to reduce stack usage?
 	TriggerStimulatorHelper helper;
 
 	uint32_t syncIndex = helper.findTriggerSyncPoint(shape,
@@ -731,7 +737,9 @@ uint32_t TriggerState::findTriggerZeroEventIndex(
 	if (syncIndex == EFI_ERROR_CODE) {
 		return syncIndex;
 	}
-	efiAssert(CUSTOM_ERR_ASSERT, getTotalRevolutionCounter() == 1, "findZero_revCounter", EFI_ERROR_CODE);
+
+	// Assert that we found the sync point on the very first revolution
+	efiAssert(CUSTOM_ERR_ASSERT, getTotalRevolutionCounter() == 0, "findZero_revCounter", EFI_ERROR_CODE);
 
 #if EFI_UNIT_TEST
 	if (printTriggerDebug) {

firmware/controllers/trigger/trigger_decoder.h

@@ -101,6 +101,7 @@ public:
 	bool validateEventCounters(const TriggerWaveform& triggerShape) const;
 	void onShaftSynchronization(
 			const TriggerStateCallback triggerCycleCallback,
+			bool wasSynchronized,
 			const efitick_t nowNt,
 			const TriggerWaveform& triggerShape);
 

firmware/controllers/trigger/trigger_simulator.cpp

@@ -111,7 +111,7 @@ void TriggerStimulatorHelper::assertSyncPositionAndSetDutyCycle(
 				state, shape, i);
 	}
 	int revolutionCounter = state.getTotalRevolutionCounter();
-	if (revolutionCounter != TEST_REVOLUTIONS + 1) {
+	if (revolutionCounter != TEST_REVOLUTIONS) {
 		warning(CUSTOM_OBD_TRIGGER_WAVEFORM, "sync failed/wrong gap parameters trigger=%s revolutionCounter=%d",
 				getTrigger_type_e(triggerConfiguration.TriggerType),
 				revolutionCounter);

unit_tests/tests/ignition_injection/test_fuel_wall_wetting.cpp

@@ -34,7 +34,7 @@ TEST(fuel, testWallWettingEnrichmentScheduling) {
 	eth.setTriggerType(TT_ONE);
 
 
-	eth.fireTriggerEvents2(/* count */ 5, 25 /* ms */);
+	eth.fireTriggerEvents2(/* count */ 4, 25 /* ms */);
 	ASSERT_EQ( 1200,  GET_RPM()) << "RPM";
 
 	int expectedInvocationCounter = 1;

unit_tests/tests/ignition_injection/test_one_cylinder_logic.cpp

@@ -19,7 +19,7 @@ TEST(issues, issueOneCylinderSpecialCase968) {
 
 	ASSERT_EQ( 0,  engine->executor.size()) << "start";
 
-	eth.fireTriggerEvents2(/* count */ 3, 50 /* ms */);
+	eth.fireTriggerEvents2(/* count */ 2, 50 /* ms */);
 	ASSERT_EQ( 0,  GET_RPM()) << "RPM";
 	ASSERT_EQ( 0,  engine->executor.size()) << "first revolution(s)";
 

unit_tests/tests/test_accel_enrichment.cpp

@@ -49,7 +49,7 @@ TEST(fuel, testTpsAccelEnrichmentScheduling) {
 
 	Sensor::setMockValue(SensorType::Tps1, 7);
 
-	eth.fireTriggerEvents2(/* count */ 5, 25 /* ms */);
+	eth.fireTriggerEvents2(/* count */ 4, 25 /* ms */);
 	ASSERT_EQ( 1200,  GET_RPM()) << "RPM";
 	int expectedInvocationCounter = 1;
 	ASSERT_EQ(expectedInvocationCounter, engine->tpsAccelEnrichment.onUpdateInvocationCounter);

unit_tests/tests/trigger/test_2jz_vvt.cpp

@@ -17,7 +17,7 @@ TEST(sensors, test2jz) {
 	eth.setTriggerType(TT_ONE);
 
 	ASSERT_EQ( 0,  GET_RPM()) << "test2jz RPM";
-	for (int i = 0; i < 3;i++) {
+	for (int i = 0; i < 2;i++) {
 		eth.fireRise(25);
 		ASSERT_EQ( 0,  GET_RPM()) << "test2jz RPM at " << i;
 	}

unit_tests/tests/trigger/test_cam_vvt_input.cpp

@@ -131,7 +131,7 @@ TEST(trigger, testNB2CamInput) {
 	engineConfiguration->useOnlyRisingEdgeForTrigger = true;
 
 	ASSERT_EQ( 0,  GET_RPM());
-	for (int i = 0; i < 7;i++) {
+	for (int i = 0; i < 4;i++) {
 		eth.fireRise(25);
 		ASSERT_EQ( 0,  GET_RPM());
 	}
@@ -139,7 +139,7 @@ TEST(trigger, testNB2CamInput) {
 	// first time we have RPM
 	ASSERT_EQ(1200,  GET_RPM());
 
-	int totalRevolutionCountBeforeVvtSync = 10;
+	int totalRevolutionCountBeforeVvtSync = 6;
 	// need to be out of VVT sync to see VVT sync in action
 	eth.fireRise(25);
 	eth.fireRise(25);
@@ -182,5 +182,5 @@ TEST(trigger, testNB2CamInput) {
 	EXPECT_FLOAT_EQ(27'000'000, dutyCycleNt);
 	EXPECT_FLOAT_EQ(0.056944445f, engine->triggerCentral.vvtShape[0].expectedDutyCycle[0]);
 
-	EXPECT_EQ(28, waveChart.getSize());
+	EXPECT_EQ(22, waveChart.getSize());
 }

unit_tests/tests/trigger/test_nissan_vq_vvt.cpp

@@ -115,7 +115,7 @@ TEST(nissan, vq_vvt) {
 	}
 
 	eth.executeUntil(1473000);
-	ASSERT_EQ(0, GET_RPM());
+	ASSERT_EQ(167, GET_RPM());
 
 	eth.executeUntil(1475000);
 	ASSERT_EQ(167, GET_RPM());

unit_tests/tests/trigger/test_quad_cam.cpp

@@ -25,7 +25,7 @@ TEST(trigger, testQuadCam) {
 	engineConfiguration->vvtCamSensorUseRise = true;
 
 	ASSERT_EQ(0, GET_RPM());
-	for (int i = 0; i < 3;i++) {
+	for (int i = 0; i < 2;i++) {
 		eth.fireRise(25);
 		ASSERT_EQ( 0,  GET_RPM());
 	}

unit_tests/tests/trigger/test_real_cranking_nissan_vq40.cpp

@@ -22,7 +22,7 @@ TEST(realCrankingVQ40, normalCranking) {
 		float vvt1 = engine->triggerCentral.getVVTPosition(/*bankIndex*/0, /*camIndex*/0);
 
 		if (vvt1 != 0 && !hasSeenFirstVvt) {
-			EXPECT_NEAR(vvt1, -38.69, 1);
+			EXPECT_NEAR(vvt1, 24.91, 1);
 			hasSeenFirstVvt = true;
 		}
 	}

unit_tests/tests/trigger/test_real_gm_24x.cpp

@@ -19,5 +19,5 @@ TEST(crankingGm24x, gmRealCrankingFromFile) {
 	}
 
 	ASSERT_EQ( 0, eth.recentWarnings()->getCount())<< "warningCounter#vwRealCranking";
-	ASSERT_EQ( 139, GET_RPM())<< reader.lineIndex();
+	ASSERT_EQ( 128, GET_RPM())<< reader.lineIndex();
 }

unit_tests/tests/trigger/test_real_volkswagen.cpp

@@ -22,7 +22,7 @@ TEST(crankingVW, vwRealCrankingFromFile) {
 	}
 
 	ASSERT_EQ( 0, eth.recentWarnings()->getCount())<< "warningCounter#vwRealCranking";
-	ASSERT_EQ( 1687, GET_RPM())<< reader.lineIndex();
+	ASSERT_EQ( 1683, GET_RPM())<< reader.lineIndex();
 }
 
 TEST(crankingVW, crankingTwiceWithGap) {
@@ -40,7 +40,7 @@ TEST(crankingVW, crankingTwiceWithGap) {
 		}
 
 		ASSERT_EQ(0, eth.recentWarnings()->getCount())<< "warningCounter#vwRealCranking";
-		ASSERT_EQ(1687, GET_RPM())<< reader.lineIndex();
+		ASSERT_EQ(1683, GET_RPM())<< reader.lineIndex();
 	}
 
 	auto now = getTimeNowNt();
@@ -57,7 +57,7 @@ TEST(crankingVW, crankingTwiceWithGap) {
 		}
 
 		ASSERT_EQ(0, eth.recentWarnings()->getCount());
-		ASSERT_EQ(1687, GET_RPM())<< reader.lineIndex();
+		ASSERT_EQ(1683, GET_RPM())<< reader.lineIndex();
 	}
 
 	{
@@ -72,6 +72,6 @@ TEST(crankingVW, crankingTwiceWithGap) {
 		}
 
 		ASSERT_EQ(0, eth.recentWarnings()->getCount());
-		ASSERT_EQ(1688, GET_RPM())<< reader.lineIndex();
+		ASSERT_EQ(1683, GET_RPM())<< reader.lineIndex();
 	}
 }

unit_tests/tests/trigger/test_symmetrical_crank.cpp

@@ -70,7 +70,7 @@ TEST(engine, testSymmetricalCrank) {
 
 
 
-	for (int i = 0; i < 6; i++) {
+	for (int i = 0; i < 3; i++) {
 		postFourEvents(&eth, mult);
 		ASSERT_EQ( 0,  GET_RPM()) << "RPM#0";
 	}