diff --git a/firmware/CHANGELOG.md b/firmware/CHANGELOG.md index a7f56879d8..34f7db5764 100644 --- a/firmware/CHANGELOG.md +++ b/firmware/CHANGELOG.md @@ -11,10 +11,16 @@ Release template (copy/paste this for new release): ## Month 202x Release - "Release Name" ### Breaking Changes + - ### Added + - ### Fixed + - + +### Removed + - --> @@ -26,10 +32,11 @@ Release template (copy/paste this for new release): - VVT minimum RPM setting #4545 - Flexible ignition adder/trim tables #4586 - Enforce board configuration overrides more strictly #4614 - - Fuel Priming reset fix #4627 - Startup Frame should scan for available hardware #4633 + - Don't fire the engine without the ignition on (avoids USB keeping engine alive after ignition off) #4474 ### Fixed + - Fuel Priming reset fix #4627 - Slower than expected RPM information was slowing engine start #4629 ## September 2022 Release - "Day 203" diff --git a/firmware/controllers/actuators/electronic_throttle.cpp b/firmware/controllers/actuators/electronic_throttle.cpp index 21a559d0b7..fc20487cf6 100644 --- a/firmware/controllers/actuators/electronic_throttle.cpp +++ b/firmware/controllers/actuators/electronic_throttle.cpp @@ -529,7 +529,7 @@ expected EtbController::getClosedLoop(percent_t target, percent_t obs // Allow up to 10 percent-seconds of error if (etbIntegralError > 10.0f) { // TODO: figure out how to handle uncalibrated ETB - //engine->limpManager.etbProblem(); + //getLimpManager()->etbProblem(); } // Normal case - use PID to compute closed loop part @@ -548,7 +548,7 @@ void EtbController::setOutput(expected outputValue) { if (!m_motor) return; // If ETB is allowed, output is valid, and we aren't paused, output to motor. - if (engine->limpManager.allowElectronicThrottle() + if (getLimpManager()->allowElectronicThrottle() && outputValue && !engineConfiguration->pauseEtbControl) { m_motor->enable(); diff --git a/firmware/controllers/algo/engine.cpp b/firmware/controllers/algo/engine.cpp index 70b0d42296..301c2b0b76 100644 --- a/firmware/controllers/algo/engine.cpp +++ b/firmware/controllers/algo/engine.cpp @@ -544,7 +544,7 @@ TriggerCentral * getTriggerCentral() { } LimpManager * getLimpManager() { - return &engine->limpManager; + return &engine->module().unmock(); } FuelSchedule *getFuelSchedule() { diff --git a/firmware/controllers/algo/engine.h b/firmware/controllers/algo/engine.h index c1b6363351..c318de6586 100644 --- a/firmware/controllers/algo/engine.h +++ b/firmware/controllers/algo/engine.h @@ -145,6 +145,7 @@ public: #endif // EFI_VEHICLE_SPEED KnockController, SensorChecker, + LimpManager, EngineModule // dummy placeholder so the previous entries can all have commas > engineModules; @@ -310,8 +311,6 @@ public: AirmassModelBase* mockAirmassModel = nullptr; #endif - LimpManager limpManager; - private: void reset(); diff --git a/firmware/controllers/algo/engine2.cpp b/firmware/controllers/algo/engine2.cpp index 5185a47171..0602c0fed1 100644 --- a/firmware/controllers/algo/engine2.cpp +++ b/firmware/controllers/algo/engine2.cpp @@ -192,9 +192,6 @@ void EngineState::periodicFastCallback() { #if EFI_LAUNCH_CONTROL engine->launchController.update(); #endif //EFI_LAUNCH_CONTROL - - engine->limpManager.updateState(rpm, nowNt); - #endif // EFI_ENGINE_CONTROL } diff --git a/firmware/controllers/core/error_handling.cpp b/firmware/controllers/core/error_handling.cpp index 20cab03c5c..7caf71def2 100644 --- a/firmware/controllers/core/error_handling.cpp +++ b/firmware/controllers/core/error_handling.cpp @@ -235,7 +235,7 @@ void firmwareError(obd_code_e code, const char *fmt, ...) { #if EFI_PROD_CODE if (hasFirmwareErrorFlag) return; - engine->limpManager.fatalError(); + getLimpManager()->fatalError(); engine->engineState.warnings.addWarningCode(code); #ifdef EFI_PRINT_ERRORS_AS_WARNINGS va_list ap; diff --git a/firmware/controllers/engine_cycle/rpm_calculator.cpp b/firmware/controllers/engine_cycle/rpm_calculator.cpp index 4bfd2521e3..30772ba46a 100644 --- a/firmware/controllers/engine_cycle/rpm_calculator.cpp +++ b/firmware/controllers/engine_cycle/rpm_calculator.cpp @@ -82,7 +82,7 @@ bool RpmCalculator::isRunning() const { * @return true if engine is spinning (cranking or running) */ bool RpmCalculator::checkIfSpinning(efitick_t nowNt) const { - if (engine->limpManager.shutdownController.isEngineStop(nowNt)) { + if (getLimpManager()->shutdownController.isEngineStop(nowNt)) { return false; } diff --git a/firmware/controllers/engine_cycle/spark_logic.cpp b/firmware/controllers/engine_cycle/spark_logic.cpp index 2bcf3e53f1..19917ea222 100644 --- a/firmware/controllers/engine_cycle/spark_logic.cpp +++ b/firmware/controllers/engine_cycle/spark_logic.cpp @@ -461,7 +461,7 @@ void onTriggerEventSparkLogic(uint32_t trgEventIndex, int rpm, efitick_t edgeTim return; } - LimpState limitedSparkState = engine->limpManager.allowIgnition(); + LimpState limitedSparkState = getLimpManager()->allowIgnition(); // todo: eliminate state copy logic by giving limpManager it's owm limp_manager.txt and leveraging LiveData engine->outputChannels.sparkCutReason = (int8_t)limitedSparkState.reason; diff --git a/firmware/controllers/limp_manager.cpp b/firmware/controllers/limp_manager.cpp index a261408c19..2ed80de941 100644 --- a/firmware/controllers/limp_manager.cpp +++ b/firmware/controllers/limp_manager.cpp @@ -34,10 +34,21 @@ static bool noFiringUntilVvtSync(vvt_mode_e vvtMode) { operationMode == FOUR_STROKE_TWELVE_TIMES_CRANK_SENSOR; } +void LimpManager::onFastCallback() { + updateState(Sensor::getOrZero(SensorType::Rpm), getTimeNowNt()); +} + void LimpManager::updateState(int rpm, efitick_t nowNt) { Clearable allowFuel = engineConfiguration->isInjectionEnabled; Clearable allowSpark = engineConfiguration->isIgnitionEnabled; +#if !EFI_UNIT_TEST + if (!m_ignitionOn) { + allowFuel.clear(ClearReason::IgnitionOff); + allowSpark.clear(ClearReason::IgnitionOff); + } +#endif + { // User-configured hard RPM limit, either constant or CLT-lookup // todo: migrate to engineState->desiredRpmLimit to get this variable logged @@ -159,6 +170,10 @@ todo AndreiKA this change breaks 22 unit tests? m_transientAllowIgnition = allowSpark; } +void LimpManager::onIgnitionStateChanged(bool ignitionOn) { + m_ignitionOn = ignitionOn; +} + void LimpManager::etbProblem() { m_allowEtb.clear(ClearReason::EtbProblem); setFaultRevLimit(1500); diff --git a/firmware/controllers/limp_manager.h b/firmware/controllers/limp_manager.h index 582fd4721f..1a61152172 100644 --- a/firmware/controllers/limp_manager.h +++ b/firmware/controllers/limp_manager.h @@ -20,6 +20,7 @@ enum class ClearReason : uint8_t { FloodClear, // 11 EnginePhase, // 12 KickStart, // 13 + IgnitionOff, // 14 // Keep this list in sync with fuelIgnCutCodeList in rusefi.input! // todo: add a code generator between ClearReason and fuelIgnCutCodeList in rusefi.input @@ -78,13 +79,16 @@ private: bool m_state = false; }; -class LimpManager { +class LimpManager : public EngineModule { public: ShutdownController shutdownController; // This is called from periodicFastCallback to update internal state void updateState(int rpm, efitick_t nowNt); + void onFastCallback() override; + void onIgnitionStateChanged(bool ignitionOn) override; + // Other subsystems call these APIs to determine their behavior bool allowElectronicThrottle() const; @@ -116,6 +120,9 @@ private: Clearable m_transientAllowIgnition = true; bool m_hadOilPressureAfterStart = false; + + // Ignition switch state + bool m_ignitionOn = false; }; LimpManager * getLimpManager(); diff --git a/firmware/controllers/shutdown_controller.cpp b/firmware/controllers/shutdown_controller.cpp index 5562e8ecc7..cc2a160b1f 100644 --- a/firmware/controllers/shutdown_controller.cpp +++ b/firmware/controllers/shutdown_controller.cpp @@ -3,9 +3,7 @@ * */ -#include "shutdown_controller.h" -#include "loggingcentral.h" -#include "limp_manager.h" +#include "pch.h" void doScheduleStopEngine() { efiPrintf("Starting doScheduleStopEngine"); diff --git a/firmware/controllers/trigger/trigger_central.cpp b/firmware/controllers/trigger/trigger_central.cpp index bd264a8f79..53ad046ac8 100644 --- a/firmware/controllers/trigger/trigger_central.cpp +++ b/firmware/controllers/trigger/trigger_central.cpp @@ -629,9 +629,7 @@ void TriggerCentral::decodeMapCam(efitick_t timestamp, float currentPhase) { #if EFI_UNIT_TEST // hack? feature? existing unit test relies on VVT phase available right away // but current implementation which is based on periodicFastCallback would only make result available on NEXT tooth - int rpm = Sensor::getOrZero(SensorType::Rpm); - efitick_t nowNt = getTimeNowNt(); - getLimpManager()->updateState(rpm, nowNt); + getLimpManager()->onFastCallback(); #endif // EFI_UNIT_TEST } diff --git a/firmware/init/sensor/init_sensors.cpp b/firmware/init/sensor/init_sensors.cpp index 3df43e699e..d492627eb3 100644 --- a/firmware/init/sensor/init_sensors.cpp +++ b/firmware/init/sensor/init_sensors.cpp @@ -73,6 +73,16 @@ void initNewSensors() { // Init CLI functionality for sensors (mocking) initSensorCli(); + +#ifdef HARDWARE_CI + + chThdSleepMilliseconds(10); + + if (Sensor::getOrZero(SensorType::BatteryVoltage) < 8) { + // Fake that we have battery voltage, some tests rely on it + Sensor::setMockValue(SensorType::BatteryVoltage, 10); + } +#endif } void stopSensors() { diff --git a/firmware/tunerstudio/rusefi.input b/firmware/tunerstudio/rusefi.input index 61fed39f43..0d78f35455 100644 --- a/firmware/tunerstudio/rusefi.input +++ b/firmware/tunerstudio/rusefi.input @@ -234,7 +234,7 @@ enable2ndByteCanID = false wboFaultCodeList = bits, U08, [0:3], "OK", "Unk", "Unk", "failed to heat", "overheat", "underheat", "no supply" - fuelIgnCutCodeList = bits, U08, [0:7], "None", "fatal error", "setting disabled", "RPM limit", "fault RPM limit", "boost cut", "oil pressure", "stop requested", "ETB problem", "launch control", "max injector duty", "flood clear", "engine sync", "kickstart" + fuelIgnCutCodeList = bits, U08, [0:7], "None", "fatal error", "setting disabled", "RPM limit", "fault RPM limit", "boost cut", "oil pressure", "stop requested", "ETB problem", "launch control", "max injector duty", "flood clear", "engine sync", "kickstart", "ign off" [ConstantsExtensions] ; defaultValue is used to provide TunerStudio with a value to use in the case of diff --git a/unit_tests/tests/actuators/test_etb.cpp b/unit_tests/tests/actuators/test_etb.cpp index 176de6c39a..00bad71221 100644 --- a/unit_tests/tests/actuators/test_etb.cpp +++ b/unit_tests/tests/actuators/test_etb.cpp @@ -732,7 +732,7 @@ TEST(etb, setOutputLimpHome) { EXPECT_CALL(motor, disable()); // Trip a fatal error - engine->limpManager.fatalError(); + getLimpManager()->fatalError(); etb.setOutput(25.0f); } diff --git a/unit_tests/tests/trigger/test_map_cam.cpp b/unit_tests/tests/trigger/test_map_cam.cpp index 9f77edd11d..549c1596a7 100644 --- a/unit_tests/tests/trigger/test_map_cam.cpp +++ b/unit_tests/tests/trigger/test_map_cam.cpp @@ -12,10 +12,10 @@ TEST(trigger, map_cam_by_magic_point) { engineConfiguration->camInputs[0] = Gpio::A0; engineConfiguration->vvtMode[0] = VVT_MAP_V_TWIN; eth.engine.periodicFastCallback(); // trigger limp mode - ASSERT_FALSE(eth.engine.limpManager.allowIgnition()); - ASSERT_FALSE(eth.engine.limpManager.allowInjection()); - ASSERT_EQ(ClearReason::EnginePhase, eth.engine.limpManager.allowIgnition().reason); - ASSERT_EQ(ClearReason::EnginePhase, eth.engine.limpManager.allowInjection().reason); + ASSERT_FALSE(getLimpManager()->allowIgnition()); + ASSERT_FALSE(getLimpManager()->allowInjection()); + ASSERT_EQ(ClearReason::EnginePhase, getLimpManager()->allowIgnition().reason); + ASSERT_EQ(ClearReason::EnginePhase, getLimpManager()->allowInjection().reason); engine->outputChannels.instantMAPValue = 100; @@ -39,8 +39,8 @@ TEST(trigger, map_cam_by_magic_point) { ASSERT_EQ(1, engine->triggerCentral.triggerState.camResyncCounter); ASSERT_EQ(10, engine->triggerCentral.mapVvt_MAP_AT_CYCLE_COUNT); - ASSERT_EQ(ClearReason::None, eth.engine.limpManager.allowIgnition().reason); - ASSERT_EQ(ClearReason::None, eth.engine.limpManager.allowInjection().reason); + ASSERT_EQ(ClearReason::None, getLimpManager()->allowIgnition().reason); + ASSERT_EQ(ClearReason::None, getLimpManager()->allowInjection().reason); // We have "VVT" sync, things should be scheduled! ASSERT_EQ(2, engine->executor.size());