create limp manager (#2142)

* move rev limit to limp manager

* call fatal error

* include order

* fix bug

* tests

* comment

Co-authored-by: Matthew Kennedy <makenne@microsoft.com>

firmware/controllers/algo/engine.cpp

@@ -433,6 +433,7 @@ void Engine::injectEngineReferences() {
 
 	INJECT_ENGINE_REFERENCE(&primaryTriggerConfiguration);
 	INJECT_ENGINE_REFERENCE(&vvtTriggerConfiguration);
+	INJECT_ENGINE_REFERENCE(&limpManager);
 
 	primaryTriggerConfiguration.update();
 	vvtTriggerConfiguration.update();

firmware/controllers/algo/engine.h

@@ -18,6 +18,7 @@
 #include "local_version_holder.h"
 #include "buttonshift.h"
 #include "gear_controller.h"
+#include "limp_manager.h"
 
 #if EFI_SIGNAL_EXECUTOR_ONE_TIMER
 // PROD real firmware uses this implementation
@@ -262,7 +263,6 @@ public:
 	efitimeus_t acSwitchLastChangeTime = 0;
 
 	bool isRunningPwmTest = false;
-	bool isRpmHardLimit = false;
 
 	int getRpmHardLimit(DECLARE_ENGINE_PARAMETER_SIGNATURE);
 
@@ -375,6 +375,8 @@ public:
 
 	AirmassModelBase* mockAirmassModel = nullptr;
 
+	LimpManager limpManager;
+
 private:
 	/**
 	 * By the way:

firmware/controllers/algo/engine2.cpp

@@ -181,6 +181,8 @@ void EngineState::periodicFastCallback(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
 	updateLaunchConditions(PASS_ENGINE_PARAMETER_SIGNATURE);
 #endif //EFI_LAUNCH_CONTROL
 
+	engine->limpManager.updateState(rpm);
+
 #endif // EFI_ENGINE_CONTROL
 }
 

firmware/controllers/controllers.mk

@@ -56,6 +56,7 @@ CONTROLLERS_SRC_CPP = \
 	$(CONTROLLERS_DIR)/gear_controller.cpp \
 	$(CONTROLLERS_DIR)/start_stop.cpp \
 	$(CONTROLLERS_DIR)/simple_tcu.cpp \
+	$(CONTROLLERS_DIR)/limp_manager.cpp \
 
 CONTROLLERS_INC=\
 	$(CONTROLLERS_DIR) \

firmware/controllers/core/error_handling.cpp

@@ -244,6 +244,7 @@ void firmwareError(obd_code_e code, const char *fmt, ...) {
 #if EFI_PROD_CODE
 	if (hasFirmwareErrorFlag)
 		return;
+	engine->limpManager.fatalError();
 	engine->engineState.warnings.addWarningCode(code);
 #ifdef EFI_PRINT_ERRORS_AS_WARNINGS
 	va_list ap;

firmware/controllers/engine_cycle/main_trigger_callback.cpp

@@ -423,16 +423,10 @@ void mainTriggerCallback(uint32_t trgEventIndex, efitick_t edgeTimestamp DECLARE
 		// TODO: add 'pin shutdown' invocation somewhere - coils might be still open here!
 		return;
 	}
-	bool limitedSpark = ENGINE(isRpmHardLimit);
-	bool limitedFuel = ENGINE(isRpmHardLimit);
 
-	if (CONFIG(boostCutPressure) != 0) {
-		// todo: move part of this to periodicFast? probably not cool to decode MAP sensor inside trigger callback?
-		if (getMap(PASS_ENGINE_PARAMETER_SIGNATURE) > CONFIG(boostCutPressure)) {
-			limitedSpark = true;
-			limitedFuel = true;
-		}
-	}
+	bool limitedSpark = !ENGINE(limpManager).allowIgnition();
+	bool limitedFuel = !ENGINE(limpManager).allowInjection();
+
 #if EFI_LAUNCH_CONTROL
 	if (engine->isLaunchCondition && !limitedSpark && !limitedFuel) {
 		/* in case we are not already on a limited conditions, check launch as well */

firmware/controllers/engine_cycle/rpm_calculator.cpp

@@ -268,7 +268,6 @@ void rpmShaftPositionCallback(trigger_event_e ckpSignalType,
 		}
 		rpmState->onNewEngineCycle();
 		rpmState->lastRpmEventTimeNt = nowNt;
-		engine->isRpmHardLimit = GET_RPM() > engine->getRpmHardLimit(PASS_ENGINE_PARAMETER_SIGNATURE);
 	}
 
 

firmware/controllers/limp_manager.cpp

@@ -0,0 +1,66 @@
+#include "limp_manager.h"
+#include "engine.h"
+#include "map.h"
+#include "efilib.h"
+
+EXTERN_ENGINE;
+
+void LimpManager::updateState(int rpm) {
+	// User-configured hard RPM limit
+	bool isRevLimited = rpm > engine->getRpmHardLimit(PASS_ENGINE_PARAMETER_SIGNATURE);
+
+	// TODO: user configurable what gets limited
+	bool limitFuel = isRevLimited;
+	bool limitSpark = isRevLimited;
+
+	// Force fuel limiting on the fault rev limit
+	if (rpm > m_faultRevLimit) {
+		limitFuel = true;
+	}
+
+	// Limit fuel only on boost pressure (limiting spark bends valves)
+	if (CONFIG(boostCutPressure) != 0) {
+		if (getMap(PASS_ENGINE_PARAMETER_SIGNATURE) > CONFIG(boostCutPressure)) {
+			limitFuel = true;
+		}
+	}
+
+	m_transientLimitInjection = limitFuel;
+	m_transientLimitIgnition = limitSpark;
+}
+
+void LimpManager::etbProblem() {
+	m_allowEtb.clear();
+	setFaultRevLimit(1500);
+}
+
+void LimpManager::fatalError() {
+	m_allowEtb.clear();
+	m_allowIgnition.clear();
+	m_allowInjection.clear();
+	m_allowTriggerInput.clear();
+
+	setFaultRevLimit(0);
+}
+
+void LimpManager::setFaultRevLimit(int limit) {
+	// Only allow decreasing the limit
+	// aka uses the limit of the worst fault to yet occur
+	m_faultRevLimit = minI(m_faultRevLimit, limit);
+}
+
+bool LimpManager::allowElectronicThrottle() const {
+	return m_allowEtb;
+}
+
+bool LimpManager::allowTriggerInput() const {
+	return m_allowTriggerInput;
+}
+
+bool LimpManager::allowInjection() const {
+	return !m_transientLimitInjection && m_allowInjection;
+}
+
+bool LimpManager::allowIgnition() const {
+	return !m_transientLimitIgnition && m_allowIgnition;
+}

firmware/controllers/limp_manager.h

@@ -0,0 +1,54 @@
+#pragma once
+
+#include "engine_ptr.h"
+
+#include <cstdint>
+
+// Only allows clearing the value, but never resetting it.
+class Clearable {
+public:
+	void clear() {
+		m_value = false;
+	}
+
+	operator bool() const {
+		return m_value;
+	}
+
+private:
+	bool m_value = true;
+};
+
+class LimpManager {
+public:
+	DECLARE_ENGINE_PTR;
+
+	// This is called from periodicFastCallback to update internal state
+	void updateState(int rpm);
+
+	// Other subsystems call these APIs to determine their behavior
+	bool allowElectronicThrottle() const;
+
+	bool allowInjection() const;
+	bool allowIgnition() const;
+
+	bool allowTriggerInput() const;
+
+	// Other subsystems call these APIs to indicate a problem has occured
+	void etbProblem();
+	void fatalError();
+
+private:
+	void setFaultRevLimit(int limit);
+
+	// Start with no fault rev limit
+	int32_t m_faultRevLimit = INT32_MAX;
+
+	Clearable m_allowEtb;
+	Clearable m_allowInjection;
+	Clearable m_allowIgnition;
+	Clearable m_allowTriggerInput;
+
+	bool m_transientLimitInjection = false;
+	bool m_transientLimitIgnition = false;
+};

unit_tests/tests/test_limp.cpp

@@ -0,0 +1,76 @@
+#include "limp_manager.h"
+#include "engine_test_helper.h"
+#include <gtest/gtest.h>
+
+TEST(limp, testFatalError) {
+	LimpManager dut;
+
+	// Everything should work by default
+	ASSERT_TRUE(dut.allowElectronicThrottle());
+	ASSERT_TRUE(dut.allowIgnition());
+	ASSERT_TRUE(dut.allowInjection());
+	ASSERT_TRUE(dut.allowTriggerInput());
+
+	dut.fatalError();
+
+	// Fatal error should kill everything
+	EXPECT_FALSE(dut.allowElectronicThrottle());
+	EXPECT_FALSE(dut.allowIgnition());
+	EXPECT_FALSE(dut.allowInjection());
+	EXPECT_FALSE(dut.allowTriggerInput());
+}
+
+TEST(limp, revLimit) {
+	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
+
+	engineConfiguration->rpmHardLimit = 2500;
+
+	LimpManager dut;
+	INJECT_ENGINE_REFERENCE(&dut);
+
+	// Under rev limit, inj/ign allowed
+	dut.updateState(2000);
+	EXPECT_TRUE(dut.allowIgnition());
+	EXPECT_TRUE(dut.allowInjection());
+
+	// Over rev limit, no injection
+	dut.updateState(3000);
+	EXPECT_FALSE(dut.allowIgnition());
+	EXPECT_FALSE(dut.allowInjection());
+
+	// Now recover back to under limit
+	dut.updateState(2000);
+	EXPECT_TRUE(dut.allowIgnition());
+	EXPECT_TRUE(dut.allowInjection());
+}
+
+TEST(limp, boostCut) {
+	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
+
+	// Cut above 100kPa
+	engineConfiguration->boostCutPressure = 100;
+
+	LimpManager dut;
+	INJECT_ENGINE_REFERENCE(&dut);
+
+	// Below threshold, injection allowed
+	engine->mockMapValue = 80;
+	dut.updateState(1000);
+	EXPECT_TRUE(dut.allowInjection());
+
+	// Above threshold, injection cut
+	engine->mockMapValue = 120;
+	dut.updateState(1000);
+	EXPECT_FALSE(dut.allowInjection());
+
+	// Below threshold, should recover
+	engine->mockMapValue = 80;
+	dut.updateState(1000);
+	EXPECT_TRUE(dut.allowInjection());
+
+	// SPECIAL CASE: threshold of 0 means never boost cut
+	engineConfiguration->boostCutPressure = 0;
+	engine->mockMapValue = 500;
+	dut.updateState(1000);
+	EXPECT_TRUE(dut.allowInjection());
+}

unit_tests/tests/tests.mk

@@ -64,4 +64,5 @@ TESTS_SRC_CPP = \
 	tests/test_binary_log.cpp \
 	tests/test_dynoview.cpp \
 	tests/test_gpio.cpp \
+	tests/test_limp.cpp \