rpm calculator refactoring, gm 24x test (#3585)

* data * stub test * use getTimeSinceSyncPoint * s * s * no * test * stray ; * those were 1/10 what they were supposed to be * actually check something * dead log line?
2021-11-20 03:59:02 -08:00 · 2021-11-20 03:59:02 -08:00 · a342bee57e
parent bdc6340df7
commit a342bee57e
9 changed files with 175 additions and 32 deletions
--- a/firmware/controllers/algo/engine.cpp
+++ b/firmware/controllers/algo/engine.cpp
@ -500,7 +500,6 @@ void Engine::watchdog() {
 	ignitionEvents.isReady = false;
 #if EFI_PROD_CODE || EFI_SIMULATOR
 	efiPrintf("engine has STOPPED");
-	efiPrintf("templog engine has STOPPED %f", secondsSinceTriggerEvent);
 	triggerInfo();
 #endif

--- a/firmware/controllers/engine_controller.cpp
+++ b/firmware/controllers/engine_controller.cpp
@ -211,13 +211,7 @@ static void doPeriodicSlowCallback() {
 		}
 	}

-	/**
-	 * Update engine RPM state if needed (check timeouts).
-	 */
-	bool isSpinning = engine->rpmCalculator.checkIfSpinning(nowNt);
-	if (!isSpinning) {
-		engine->rpmCalculator.setStopSpinning();
-	}
+	engine->rpmCalculator.onSlowCallback();

 	if (engine->directSelfStimulation || engine->rpmCalculator.isStopped()) {
 		/**
--- a/firmware/controllers/engine_cycle/map_averaging.cpp
+++ b/firmware/controllers/engine_cycle/map_averaging.cpp
@ -124,7 +124,7 @@ void mapAveragingAdcCallback(adcsample_t adcValue) {
 			float voltage = adcToVoltsDivided(adcValue);
 			float currentPressure = convertMap(voltage).value_or(0);
 			scAddData(
-					getCrankshaftAngleNt(getTimeNowNt()),
+					engine->triggerCentral.getCurrentEnginePhase(getTimeNowNt()).value_or(0),
 					currentPressure);
 		}
 	}
--- a/firmware/controllers/engine_cycle/rpm_calculator.cpp
+++ b/firmware/controllers/engine_cycle/rpm_calculator.cpp
@ -99,11 +99,6 @@ bool RpmCalculator::isRunning() const {
 * @return true if engine is spinning (cranking or running)
 */
 bool RpmCalculator::checkIfSpinning(efitick_t nowNt) const {
-	/**
-	 * note that the result of this subtraction could be negative, that would happen if
-	 * we have a trigger event between the time we've invoked 'getTimeNow' and here
-	 */
-
 	// Anything below 60 rpm is not running
 	bool noRpmEventsForTooLong = lastTdcTimer.getElapsedSeconds(nowNt) > NO_RPM_EVENTS_TIMEOUT_SECS;

@ -193,6 +188,15 @@ uint32_t RpmCalculator::getRevolutionCounterM(void) const {
 	return revolutionCounterSinceBoot;
 }

+void RpmCalculator::onSlowCallback() {
+	/**
+	 * Update engine RPM state if needed (check timeouts).
+	 */
+	if (!checkIfSpinning(getTimeNowNt())) {
+		engine->rpmCalculator.setStopSpinning();
+	}
+}
+
 void RpmCalculator::setStopped() {
 	revolutionCounterSinceStart = 0;

@ -281,7 +285,7 @@ void rpmShaftPositionCallback(trigger_event_e ckpSignalType,
 	// this 'index==0' case is here so that it happens after cycle callback so
 	// it goes into sniffer report into the first position
 	if (engine->sensorChartMode == SC_TRIGGER) {
-		angle_t crankAngle = getCrankshaftAngleNt(nowNt);
+		angle_t crankAngle = engine->triggerCentral.getCurrentEnginePhase(nowNt).value_or(0);
 		int signal = 1000 * ckpSignalType + index;
 		scAddData(crankAngle, signal);
 	}
@ -349,20 +353,6 @@ void tdcMarkCallback(
 	}
 }

-
-/**
- * @return Current crankshaft angle, 0 to 720 for four-stroke
- */
-float getCrankshaftAngleNt(efitick_t timeNt) {
-	float timeSinceZeroAngle = engine->rpmCalculator.lastTdcTimer.getElapsedSeconds(timeNt);
-
-	int rpm = GET_RPM();
-
-	float oneDegreeSeconds = (60.0f / 360) / rpm;
-
-	return rpm == 0 ? NAN : timeSinceZeroAngle / oneDegreeSeconds;
-}
-
 void initRpmCalculator() {

 #if ! HW_CHECK_MODE
--- a/firmware/controllers/engine_cycle/rpm_calculator.h
+++ b/firmware/controllers/engine_cycle/rpm_calculator.h
@ -48,6 +48,9 @@ public:
 	int mockRpm;
 #endif /* EFI_PROD_CODE */
 	RpmCalculator();
+
+	void onSlowCallback();
+
 	/**
 	 * Returns true if the engine is not spinning (RPM==0)
 	 */
@ -174,8 +177,6 @@ void tdcMarkCallback(
 */
 void initRpmCalculator();

-float getCrankshaftAngleNt(efitick_t timeNt);
-
 #define getRevolutionCounter() (engine->rpmCalculator.getRevolutionCounterM())

 #if EFI_ENGINE_SNIFFER
--- a/firmware/hw_layer/hardware.cpp
+++ b/firmware/hw_layer/hardware.cpp
@ -183,7 +183,7 @@ void onFastAdcComplete(adcsample_t*) {
 #if EFI_SENSOR_CHART && EFI_SHAFT_POSITION_INPUT
 	if (engine->sensorChartMode == SC_AUX_FAST1) {
 		float voltage = getAdcValue("fAux1", engineConfiguration->auxFastSensor1_adcChannel);
-		scAddData(getCrankshaftAngleNt(getTimeNowNt()), voltage);
+		scAddData(engine->triggerCentral.getCurrentEnginePhase(getTimeNowNt()).value_or(0), voltage);
 	}
 #endif /* EFI_SENSOR_CHART */

--- a/unit_tests/tests/tests.mk
+++ b/unit_tests/tests/tests.mk
@ -11,6 +11,7 @@ TESTS_SRC_CPP = \
 	tests/trigger/test_real_cranking_nissan_vq40.cpp \
 	tests/trigger/test_real_volkswagen.cpp \
 	tests/trigger/test_real_nb2_cranking.cpp \
+	tests/trigger/test_real_gm_24x.cpp \
 	tests/trigger/test_rpm_multiplier.cpp \
 	tests/trigger/test_quad_cam.cpp \
 	tests/trigger/test_nissan_vq_vvt.cpp \
--- a/unit_tests/tests/trigger/resources/gm_24x_cranking.csv
+++ b/unit_tests/tests/trigger/resources/gm_24x_cranking.csv
@ -0,0 +1,135 @@
+time, crank
+47.597971, 1
+47.631381, 0
+47.656059, 1
+47.668234, 0
+47.689604, 1
+47.700624, 0
+47.710404, 1
+47.730572, 0
+47.739694, 1
+47.758835, 0
+47.776974, 1
+47.786761, 0
+47.805719, 1
+47.816164, 0
+47.837471, 1
+47.849791, 0
+47.862130, 1
+47.889479, 0
+47.914454, 1
+47.926046, 0
+47.936103, 1
+47.955086, 0
+47.971760, 1
+47.980169, 0
+47.996388, 1
+48.004388, 0
+48.021609, 1
+48.030380, 0
+48.049612, 1
+48.058998, 0
+48.078252, 1
+48.086758, 0
+48.095224, 1
+48.109850, 0
+48.116918, 1
+48.130222, 0
+48.137384, 1
+48.152483, 0
+48.159526, 1
+48.174390, 0
+48.181975, 1
+48.197868, 0
+48.213808, 1
+48.221072, 0
+48.228142, 1
+48.240671, 0
+48.246579, 1
+48.257815, 0
+48.263505, 1
+48.275332, 0
+48.288638, 1
+48.296245, 0
+48.308558, 1
+48.315921, 0
+48.322560, 1
+48.335724, 0
+48.341722, 1
+48.353670, 0
+48.364657, 1
+48.370491, 0
+48.381976, 1
+48.388469, 0
+48.401740, 1
+48.407779, 0
+48.414032, 1
+48.427697, 0
+48.440830, 1
+48.447550, 0
+48.453561, 1
+48.465500, 0
+48.476356, 1
+48.481998, 0
+48.493292, 1
+48.499177, 0
+48.512619, 1
+48.518553, 0
+48.531264, 1
+48.537720, 0
+48.550525, 1
+48.556675, 0
+48.562961, 1
+48.574350, 0
+48.580064, 1
+48.590903, 0
+48.596670, 1
+48.608361, 0
+48.615078, 1
+48.627252, 0
+48.633536, 1
+48.646428, 0
+48.659391, 1
+48.665613, 0
+48.671793, 1
+48.683302, 0
+48.688916, 1
+48.699849, 0
+48.705524, 1
+48.717411, 0
+48.730182, 1
+48.736189, 0
+48.748706, 1
+48.755259, 0
+48.761259, 1
+48.774327, 0
+48.780156, 1
+48.791945, 0
+48.802732, 1
+48.808454, 0
+48.819609, 1
+48.825859, 0
+48.838560, 1
+48.844496, 0
+48.850499, 1
+48.863628, 0
+48.876288, 1
+48.882830, 0
+48.888748, 1
+48.900567, 0
+48.911459, 1
+48.917171, 0
+48.928742, 1
+48.934838, 0
+48.947719, 1
+48.953444, 0
+48.966796, 1
+48.972943, 0
+48.986464, 1
+48.992516, 0
+48.998747, 1
+49.009980, 0
+49.015545, 1
+49.026049, 0
+49.031561, 1
+49.042665, 0
--- a/unit_tests/tests/trigger/test_real_gm_24x.cpp
+++ b/unit_tests/tests/trigger/test_real_gm_24x.cpp
@ -0,0 +1,23 @@
+#include "pch.h"
+
+#include "logicdata_csv_reader.h"
+
+TEST(crankingGm24x, gmRealCrankingFromFile) {
+	CsvReader reader(1, /* vvtCount */ 0);
+	int indeces[1] = {0};
+
+	reader.open("tests/trigger/resources/gm_24x_cranking.csv", indeces);
+	EngineTestHelper eth(TEST_ENGINE);
+	engineConfiguration->isFasterEngineSpinUpEnabled = true;
+
+	eth.setTriggerType(TT_GM_LS_24);
+
+	while (reader.haveMore()) {
+		reader.processLine(&eth);
+
+		engine->rpmCalculator.onSlowCallback();
+	}
+
+	ASSERT_EQ( 0, eth.recentWarnings()->getCount())<< "warningCounter#vwRealCranking";
+	ASSERT_EQ( 139, GET_RPM())<< reader.lineIndex();
+}