RPM limit fix and unit-tests (#5238)

* fix rpm limit timing/fuel corrections

* rpm limit timing - more unit-tests

firmware/controllers/algo/engine2.cpp

@@ -136,6 +136,8 @@ void EngineState::periodicFastCallback() {
 	engine->fuelComputer.running.coolantTemperatureCoefficient = getCltFuelCorrection();
 
 	engine->module<DfcoController>()->update();
+	// should be called before getInjectionMass() and getLimitingTimingRetard()
+	getLimpManager()->updateRevLimit(rpm);
 
 	// post-cranking fuel enrichment.
 	// for compatibility reasons, apply only if the factor is greater than unity (only allow adding fuel)

firmware/controllers/limp_manager.cpp

@@ -38,6 +38,21 @@ void LimpManager::onFastCallback() {
 	updateState(Sensor::getOrZero(SensorType::Rpm), getTimeNowNt());
 }
 
+void LimpManager::updateRevLimit(int rpm) {
+	// User-configured hard RPM limit, either constant or CLT-lookup
+	m_revLimit = engineConfiguration->useCltBasedRpmLimit
+		? interpolate2d(Sensor::getOrZero(SensorType::Clt), engineConfiguration->cltRevLimitRpmBins, engineConfiguration->cltRevLimitRpm)
+		: (float)engineConfiguration->rpmHardLimit;
+
+	// Require configurable rpm drop before resuming
+	m_revLimitLow = m_revLimit - engineConfiguration->rpmHardLimitHyst;
+
+	m_timingRetard = interpolateClamped(m_revLimitLow, 0, m_revLimit, engineConfiguration->rpmSoftLimitTimingRetard, rpm);
+	
+	percent_t fuelAdded = interpolateClamped(m_revLimitLow, 0, m_revLimit, engineConfiguration->rpmSoftLimitFuelAdded, rpm);
+	m_fuelCorrection = 1.0f + fuelAdded / 100;
+}
+
 void LimpManager::updateState(int rpm, efitick_t nowNt) {
 	Clearable allowFuel = engineConfiguration->isInjectionEnabled;
 	Clearable allowSpark = engineConfiguration->isIgnitionEnabled;
@@ -53,28 +68,16 @@ void LimpManager::updateState(int rpm, efitick_t nowNt) {
         allowSpark.clear(ClearReason::Lua);
     }
 
-	{
-		// User-configured hard RPM limit, either constant or CLT-lookup
-		// todo: migrate to engineState->desiredRpmLimit to get this variable logged
-		float revLimit = engineConfiguration->useCltBasedRpmLimit
-			? interpolate2d(Sensor::getOrZero(SensorType::Clt), engineConfiguration->cltRevLimitRpmBins, engineConfiguration->cltRevLimitRpm)
-			: (float)engineConfiguration->rpmHardLimit;
 
-		// Require configurable rpm drop before resuming
+	updateRevLimit(rpm);
-		float revLimitLow = revLimit - engineConfiguration->rpmHardLimitHyst;
+	if (m_revLimitHysteresis.test(rpm, m_revLimit, m_revLimitLow)) {
-		if (m_revLimitHysteresis.test(rpm, revLimit, revLimitLow)) {
+		if (engineConfiguration->cutFuelOnHardLimit) {
-			if (engineConfiguration->cutFuelOnHardLimit) {
+			allowFuel.clear(ClearReason::HardLimit);
-				allowFuel.clear(ClearReason::HardLimit);
-			}
-
-			if (engineConfiguration->cutSparkOnHardLimit) {
-				allowSpark.clear(ClearReason::HardLimit);
-			}
 		}
 
-		m_timingRetard = interpolateClamped(revLimitLow, 0, revLimit, engineConfiguration->rpmSoftLimitTimingRetard, rpm);
+		if (engineConfiguration->cutSparkOnHardLimit) {
-		percent_t fuelAdded = interpolateClamped(revLimitLow, 0, revLimit, engineConfiguration->rpmSoftLimitFuelAdded, rpm);
+			allowSpark.clear(ClearReason::HardLimit);
-		m_fuelCorrection = 1.0f + fuelAdded / 100;
+		}
 	}
 
 #if EFI_SHAFT_POSITION_INPUT

firmware/controllers/limp_manager.h

@@ -113,6 +113,7 @@ public:
 
 	bool allowTriggerInput() const;
 
+	void updateRevLimit(int rpm);
 	angle_t getLimitingTimingRetard() const;
 	float getLimitingFuelCorrection() const;
 
@@ -145,6 +146,10 @@ private:
 
 	angle_t m_timingRetard = 0;
 	float m_fuelCorrection = 1.0f;
+
+	// todo: migrate to engineState->desiredRpmLimit to get this variable logged
+	float m_revLimit;
+	float m_revLimitLow;
 };
 
 LimpManager * getLimpManager();

unit_tests/tests/test_limp.cpp

@@ -144,6 +144,7 @@ TEST(limp, revSoftLimit) {
 	eth.engine.periodicFastCallback();
 
 	EXPECT_FLOAT_EQ(0, getLimpManager()->getLimitingTimingRetard());
+	EXPECT_FLOAT_EQ((float)eth.engine.ignitionState.correctedIgnitionAdvance - (float)eth.engine.ignitionState.baseIgnitionAdvance, 0);
 	EXPECT_FLOAT_EQ(1, getLimpManager()->getLimitingFuelCorrection());
 	// this is normal injection mode, no limiting fuel corrections
 	ASSERT_NEAR(normalRunningFuel, getRunningFuel(baseFuel), EPS4D) << "base fuel";
@@ -153,6 +154,7 @@ TEST(limp, revSoftLimit) {
 	eth.engine.periodicFastCallback();
 
 	EXPECT_FLOAT_EQ(10, getLimpManager()->getLimitingTimingRetard());	// 10 deg
+	EXPECT_FLOAT_EQ((float)eth.engine.ignitionState.correctedIgnitionAdvance - (float)eth.engine.ignitionState.baseIgnitionAdvance, -10.0f);
 	EXPECT_FLOAT_EQ(1.2f, getLimpManager()->getLimitingFuelCorrection());	// 20%
 	ASSERT_NEAR(normalRunningFuel * 1.2f, getRunningFuel(baseFuel), EPS4D) << "base fuel";	// 20%
 
@@ -160,6 +162,7 @@ TEST(limp, revSoftLimit) {
 	Sensor::setMockValue(SensorType::Rpm, 2400);
 	eth.engine.periodicFastCallback();
 	EXPECT_FLOAT_EQ(5, getLimpManager()->getLimitingTimingRetard());	// 5 deg
+	EXPECT_FLOAT_EQ((float)eth.engine.ignitionState.correctedIgnitionAdvance - (float)eth.engine.ignitionState.baseIgnitionAdvance, -5.0f);
 	EXPECT_FLOAT_EQ(1.1f, getLimpManager()->getLimitingFuelCorrection());	// 10%
 	ASSERT_NEAR(normalRunningFuel * 1.1f, getRunningFuel(baseFuel), EPS4D) << "base fuel";	// 10%
 }