From ff4ce2fb1f5f706094f3f54a52442da9430304fb Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Sat, 26 Dec 2020 14:30:46 -0800 Subject: [PATCH] create limp manager (#2142) * move rev limit to limp manager * call fatal error * include order * fix bug * tests * comment Co-authored-by: Matthew Kennedy --- firmware/controllers/algo/engine.cpp | 1 + firmware/controllers/algo/engine.h | 4 +- firmware/controllers/algo/engine2.cpp | 2 + firmware/controllers/controllers.mk | 1 + firmware/controllers/core/error_handling.cpp | 1 + .../engine_cycle/main_trigger_callback.cpp | 12 +-- .../engine_cycle/rpm_calculator.cpp | 1 - firmware/controllers/limp_manager.cpp | 66 ++++++++++++++++ firmware/controllers/limp_manager.h | 54 +++++++++++++ unit_tests/tests/test_limp.cpp | 76 +++++++++++++++++++ unit_tests/tests/tests.mk | 1 + 11 files changed, 208 insertions(+), 11 deletions(-) create mode 100644 firmware/controllers/limp_manager.cpp create mode 100644 firmware/controllers/limp_manager.h create mode 100644 unit_tests/tests/test_limp.cpp diff --git a/firmware/controllers/algo/engine.cpp b/firmware/controllers/algo/engine.cpp index f2cc7c1d64..6976036b3b 100644 --- a/firmware/controllers/algo/engine.cpp +++ b/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(); diff --git a/firmware/controllers/algo/engine.h b/firmware/controllers/algo/engine.h index af52a52c9c..eac02b47da 100644 --- a/firmware/controllers/algo/engine.h +++ b/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: diff --git a/firmware/controllers/algo/engine2.cpp b/firmware/controllers/algo/engine2.cpp index 1db4713fdf..699a964490 100644 --- a/firmware/controllers/algo/engine2.cpp +++ b/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 } diff --git a/firmware/controllers/controllers.mk b/firmware/controllers/controllers.mk index 5a7e271b10..c2e6f1186b 100644 --- a/firmware/controllers/controllers.mk +++ b/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) \ diff --git a/firmware/controllers/core/error_handling.cpp b/firmware/controllers/core/error_handling.cpp index 14bea08bc1..fb7344d404 100644 --- a/firmware/controllers/core/error_handling.cpp +++ b/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; diff --git a/firmware/controllers/engine_cycle/main_trigger_callback.cpp b/firmware/controllers/engine_cycle/main_trigger_callback.cpp index a06747e54a..644af6c227 100644 --- a/firmware/controllers/engine_cycle/main_trigger_callback.cpp +++ b/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 */ diff --git a/firmware/controllers/engine_cycle/rpm_calculator.cpp b/firmware/controllers/engine_cycle/rpm_calculator.cpp index 5c4a6c3c37..ce618846b2 100644 --- a/firmware/controllers/engine_cycle/rpm_calculator.cpp +++ b/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); } diff --git a/firmware/controllers/limp_manager.cpp b/firmware/controllers/limp_manager.cpp new file mode 100644 index 0000000000..606359b178 --- /dev/null +++ b/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; +} diff --git a/firmware/controllers/limp_manager.h b/firmware/controllers/limp_manager.h new file mode 100644 index 0000000000..183524b77d --- /dev/null +++ b/firmware/controllers/limp_manager.h @@ -0,0 +1,54 @@ +#pragma once + +#include "engine_ptr.h" + +#include + +// 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; +}; diff --git a/unit_tests/tests/test_limp.cpp b/unit_tests/tests/test_limp.cpp new file mode 100644 index 0000000000..128b418e5d --- /dev/null +++ b/unit_tests/tests/test_limp.cpp @@ -0,0 +1,76 @@ +#include "limp_manager.h" +#include "engine_test_helper.h" +#include + +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()); +} diff --git a/unit_tests/tests/tests.mk b/unit_tests/tests/tests.mk index 8200b2bb72..abf0461973 100644 --- a/unit_tests/tests/tests.mk +++ b/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 \