From d671a437d25f787e611f773291eb874dc4426558 Mon Sep 17 00:00:00 2001 From: alrijleh Date: Fri, 20 Sep 2024 00:27:50 -0400 Subject: [PATCH 1/6] basic setting in place --- firmware/controllers/actuators/idle_thread.cpp | 9 +++++++++ firmware/controllers/actuators/idle_thread_io.cpp | 3 +-- firmware/integration/rusefi_config.txt | 1 + firmware/tunerstudio/tunerstudio.template.ini | 1 + 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/firmware/controllers/actuators/idle_thread.cpp b/firmware/controllers/actuators/idle_thread.cpp index c54d127044..4d84ceab07 100644 --- a/firmware/controllers/actuators/idle_thread.cpp +++ b/firmware/controllers/actuators/idle_thread.cpp @@ -68,6 +68,15 @@ IIdleController::Phase IdleController::determinePhase(int rpm, int targetRpm, Se return Phase::Running; } + // if car is gear, disable closed loop idle + // this applies to cars like NA/NB miatas where the clutch up switch only return true if in gear + bool clutchUp = !engineConfiguration->disableIdleClutchUp + || !isBrainPinValid(engineConfiguration->clutchUpPin) + || engine->engineState.clutchUpState; + if (looksLikeCoasting && clutchUp) { + return Phase::Coasting; + } + // If still in the cranking taper, disable closed loop idle if (looksLikeCrankToIdle) { return Phase::CrankToIdleTaper; diff --git a/firmware/controllers/actuators/idle_thread_io.cpp b/firmware/controllers/actuators/idle_thread_io.cpp index 6145526fb2..7bf8dd9c3d 100644 --- a/firmware/controllers/actuators/idle_thread_io.cpp +++ b/firmware/controllers/actuators/idle_thread_io.cpp @@ -58,10 +58,9 @@ percent_t getIdlePosition() { void startPedalPins() { #if EFI_PROD_CODE - // this is neutral/no gear switch input. on Miata it's wired both to clutch pedal and neutral in gearbox - // this switch is not used yet startInputPinIfValid("clutch down switch", engineConfiguration->clutchDownPin, engineConfiguration->clutchDownPinMode); + // this is neutral/no gear switch input. on Miata it's wired both to clutch pedal and neutral in gearbox startInputPinIfValid("clutch up switch", engineConfiguration->clutchUpPin, engineConfiguration->clutchUpPinMode); startInputPinIfValid("throttle pedal up switch", engineConfiguration->throttlePedalUpPin, engineConfiguration->throttlePedalUpPinMode); diff --git a/firmware/integration/rusefi_config.txt b/firmware/integration/rusefi_config.txt index bdd612dbcc..2620036512 100644 --- a/firmware/integration/rusefi_config.txt +++ b/firmware/integration/rusefi_config.txt @@ -988,6 +988,7 @@ bit useSeparateAdvanceForCranking,"Table","Fixed (auto taper)";In Constant mode, bit useAdvanceCorrectionsForCranking;This enables the various ignition corrections during cranking (IAT, CLT, FSIO and PID idle).\nYou probably don't need this. bit flexCranking;Enable a second cranking table to use for E100 flex fuel, interpolating between the two based on flex fuel sensor. bit useIacPidMultTable;This flag allows to use a special 'PID Multiplier' table (0.0-1.0) to compensate for nonlinear nature of IAC-RPM controller +bit disableIdleClutchUp;Disable closed loop idle if ClutchUp is true. Only works for vehicles where the signal is only present when in gear, like NA/NB miatas. This helps prevent integral saturation while coasting at low RPM bit isBoostControlEnabled bit launchSmoothRetard;Interpolates the Ignition Retard from 0 to 100% within the RPM Range bit isPhaseSyncRequiredForIgnition;Some engines are OK running semi-random sequential while other engine require phase synchronization diff --git a/firmware/tunerstudio/tunerstudio.template.ini b/firmware/tunerstudio/tunerstudio.template.ini index 4ac15a2cba..557256a733 100644 --- a/firmware/tunerstudio/tunerstudio.template.ini +++ b/firmware/tunerstudio/tunerstudio.template.ini @@ -3366,6 +3366,7 @@ cmd_set_engine_type_default = "@@TS_IO_TEST_COMMAND_char@@@@ts_command_e_TS_ field = "RPM upper limit", idlePidRpmUpperLimit field = "RPM deadzone", idlePidRpmDeadZone field = "Max vehicle speed", maxIdleVss + field = "Disable Idle in Gear", disableIdleClutchUp, {clutchUpPin != @@ADC_CHANNEL_NONE@@} dialog = idleExtra, "Extra Idle Features" field = "Use idle ignition table", useSeparateAdvanceForIdle From 20243f265d69ec8ab403bd22ec5c1585d595f61b Mon Sep 17 00:00:00 2001 From: alrijleh Date: Thu, 3 Oct 2024 22:52:13 -0400 Subject: [PATCH 2/6] updated logic, added unit test --- .../controllers/actuators/idle_thread.cpp | 10 +++++----- unit_tests/tests/test_idle_controller.cpp | 19 ++++++++++++++++++- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/firmware/controllers/actuators/idle_thread.cpp b/firmware/controllers/actuators/idle_thread.cpp index 3badb1853b..b4a220aae0 100644 --- a/firmware/controllers/actuators/idle_thread.cpp +++ b/firmware/controllers/actuators/idle_thread.cpp @@ -69,11 +69,11 @@ IIdleController::Phase IdleController::determinePhase(float rpm, float targetRpm } // if car is gear, disable closed loop idle - // this applies to cars like NA/NB miatas where the clutch up switch only return true if in gear - bool clutchUp = !engineConfiguration->disableIdleClutchUp - || !isBrainPinValid(engineConfiguration->clutchUpPin) - || engine->engineState.clutchUpState; - if (looksLikeCoasting && clutchUp) { + // this applies to cars like NA/NB miatas where the clutch up switch only returns true if in gear + bool transmissionInGear = engineConfiguration->disableIdleClutchUp + && isBrainPinValid(engineConfiguration->clutchUpPin) + && engine->engineState.clutchUpState; + if (looksLikeCoasting || transmissionInGear) { return Phase::Coasting; } diff --git a/unit_tests/tests/test_idle_controller.cpp b/unit_tests/tests/test_idle_controller.cpp index 88a3add6ef..ab5a6737d8 100644 --- a/unit_tests/tests/test_idle_controller.cpp +++ b/unit_tests/tests/test_idle_controller.cpp @@ -105,6 +105,23 @@ TEST(idle_v2, testDeterminePhase) { // Below TPS but above RPM should be outside the zone EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, 1000, 0, 0, 10)); EXPECT_EQ(ICP::Coasting, dut.determinePhase(5000, 1000, 0, 0, 10)); + + //enable feature to prevent idle state when coasting in gear + engineConfiguration->disableIdleClutchUp = true; + setMockState(engineConfiguration->clutchUpPin, false); + engineConfiguration->clutchUpPin = Gpio::G2; + engineConfiguration->clutchUpPinMode = PI_PULLDOWN; + engine->updateSwitchInputs(); + + //should be same beaviour as before when clutch is pressed/car in neutral + EXPECT_EQ(ICP::Idling, dut.determinePhase(1099, 1000, 0, 0, 10)); + EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, 1000, 0, 0, 10)); + + //clutchUp indicates car in gear with clutch not pressed, should recognize as coasting instead of idle now + setMockState(engineConfiguration->clutchUpPin, true); + engine->updateSwitchInputs(); + EXPECT_EQ(ICP::Coasting, dut.determinePhase(1099, 1000, 0, 0, 10)); + EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, 1000, 0, 0, 10)); } TEST(idle_v2, crankingOpenLoop) { @@ -528,4 +545,4 @@ TEST(idle_v2, IntegrationClamping) { // Result would be 75 + 75 = 150, but it should clamp to 100 EXPECT_EQ(100, dut.getIdlePosition(950)); -} +} \ No newline at end of file From 16fff0b70595381bcea4403b009b3f42eedbee39 Mon Sep 17 00:00:00 2001 From: alrijleh Date: Fri, 4 Oct 2024 01:27:32 -0400 Subject: [PATCH 3/6] ensure that crankToIdleTaper state can be reached --- firmware/controllers/actuators/idle_thread.cpp | 18 ++++++++---------- unit_tests/tests/test_idle_controller.cpp | 9 ++++++--- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/firmware/controllers/actuators/idle_thread.cpp b/firmware/controllers/actuators/idle_thread.cpp index b4a220aae0..db641acdec 100644 --- a/firmware/controllers/actuators/idle_thread.cpp +++ b/firmware/controllers/actuators/idle_thread.cpp @@ -57,7 +57,14 @@ IIdleController::Phase IdleController::determinePhase(float rpm, float targetRpm float maximumIdleRpm = targetRpm + engineConfiguration->idlePidRpmUpperLimit; looksLikeCoasting = rpm > maximumIdleRpm; looksLikeCrankToIdle = crankingTaperFraction < 1; - if (looksLikeCoasting && !looksLikeCrankToIdle) { + + // if car is gear, disable closed loop idle + // this applies to cars like NA/NB miatas where the clutch up switch only returns true if also in gear + bool transmissionInGear = engineConfiguration->disableIdleClutchUp + && isBrainPinValid(engineConfiguration->clutchUpPin) + && engine->engineState.clutchUpState; + + if ( (looksLikeCoasting || transmissionInGear) && !looksLikeCrankToIdle) { return Phase::Coasting; } @@ -68,15 +75,6 @@ IIdleController::Phase IdleController::determinePhase(float rpm, float targetRpm return Phase::Running; } - // if car is gear, disable closed loop idle - // this applies to cars like NA/NB miatas where the clutch up switch only returns true if in gear - bool transmissionInGear = engineConfiguration->disableIdleClutchUp - && isBrainPinValid(engineConfiguration->clutchUpPin) - && engine->engineState.clutchUpState; - if (looksLikeCoasting || transmissionInGear) { - return Phase::Coasting; - } - // If still in the cranking taper, disable closed loop idle if (looksLikeCrankToIdle) { return Phase::CrankToIdleTaper; diff --git a/unit_tests/tests/test_idle_controller.cpp b/unit_tests/tests/test_idle_controller.cpp index ab5a6737d8..c8a9762639 100644 --- a/unit_tests/tests/test_idle_controller.cpp +++ b/unit_tests/tests/test_idle_controller.cpp @@ -106,22 +106,25 @@ TEST(idle_v2, testDeterminePhase) { EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, 1000, 0, 0, 10)); EXPECT_EQ(ICP::Coasting, dut.determinePhase(5000, 1000, 0, 0, 10)); - //enable feature to prevent idle state when coasting in gear + // enable feature to prevent idle state when coasting in gear engineConfiguration->disableIdleClutchUp = true; setMockState(engineConfiguration->clutchUpPin, false); engineConfiguration->clutchUpPin = Gpio::G2; engineConfiguration->clutchUpPinMode = PI_PULLDOWN; engine->updateSwitchInputs(); - //should be same beaviour as before when clutch is pressed/car in neutral + // should be same behaviour as before when clutch is pressed/car in neutral EXPECT_EQ(ICP::Idling, dut.determinePhase(1099, 1000, 0, 0, 10)); EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, 1000, 0, 0, 10)); + EXPECT_EQ(ICP::CrankToIdleTaper, dut.determinePhase(1000, 1000, 0, 0, 0.5f)); + EXPECT_EQ(ICP::Running, dut.determinePhase(1000, 1000, 10, 0, 10)); - //clutchUp indicates car in gear with clutch not pressed, should recognize as coasting instead of idle now + // clutchUp indicates car in gear with clutch not pressed, should recognize as coasting instead of idle now setMockState(engineConfiguration->clutchUpPin, true); engine->updateSwitchInputs(); EXPECT_EQ(ICP::Coasting, dut.determinePhase(1099, 1000, 0, 0, 10)); EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, 1000, 0, 0, 10)); + } TEST(idle_v2, crankingOpenLoop) { From 844634ea01ab729e1ddd7052e7f4f932d23a6350 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 30 Jun 2025 00:06:10 -0700 Subject: [PATCH 4/6] format --- unit_tests/tests/test_idle_controller.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/unit_tests/tests/test_idle_controller.cpp b/unit_tests/tests/test_idle_controller.cpp index de2f7d011d..e04a93728e 100644 --- a/unit_tests/tests/test_idle_controller.cpp +++ b/unit_tests/tests/test_idle_controller.cpp @@ -114,7 +114,7 @@ TEST(idle_v2, testDeterminePhase) { EXPECT_EQ(ICP::Running, dut.determinePhase(1101, targetInfo, 10, 0, 10)); - EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, 1000, 0, 0, 10)); + EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, 1000, 0, 0, 10)); EXPECT_EQ(ICP::Coasting, dut.determinePhase(5000, 1000, 0, 0, 10)); // enable feature to prevent idle state when coasting in gear @@ -123,7 +123,7 @@ TEST(idle_v2, testDeterminePhase) { engineConfiguration->clutchUpPin = Gpio::G2; engineConfiguration->clutchUpPinMode = PI_PULLDOWN; engine->updateSwitchInputs(); - + // should be same behaviour as before when clutch is pressed/car in neutral EXPECT_EQ(ICP::Idling, dut.determinePhase(1099, 1000, 0, 0, 10)); EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, 1000, 0, 0, 10)); @@ -136,7 +136,7 @@ TEST(idle_v2, testDeterminePhase) { EXPECT_EQ(ICP::Coasting, dut.determinePhase(1099, 1000, 0, 0, 10)); EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, 1000, 0, 0, 10)); - // Below TPS but above RPM should be outside the zone + // Below TPS but above RPM should be outside the zone EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, targetInfo, 0, 0, 10)); EXPECT_EQ(ICP::Coasting, dut.determinePhase(5000, targetInfo, 0, 0, 10)); @@ -556,4 +556,4 @@ TEST(idle_v2, IntegrationClamping) { // Result would be 75 + 75 = 150, but it should clamp to 100 EXPECT_EQ(100, dut.getIdlePosition(950)); -} \ No newline at end of file +} From 2d9341d5bd29e8f45d5eca16797d8780da6b17ef Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 30 Jun 2025 00:08:46 -0700 Subject: [PATCH 5/6] fix build --- unit_tests/tests/test_idle_controller.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/unit_tests/tests/test_idle_controller.cpp b/unit_tests/tests/test_idle_controller.cpp index e04a93728e..390b9a8d18 100644 --- a/unit_tests/tests/test_idle_controller.cpp +++ b/unit_tests/tests/test_idle_controller.cpp @@ -114,8 +114,8 @@ TEST(idle_v2, testDeterminePhase) { EXPECT_EQ(ICP::Running, dut.determinePhase(1101, targetInfo, 10, 0, 10)); - EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, 1000, 0, 0, 10)); - EXPECT_EQ(ICP::Coasting, dut.determinePhase(5000, 1000, 0, 0, 10)); + EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, targetInfo, 0, 0, 10)); + EXPECT_EQ(ICP::Coasting, dut.determinePhase(5000, targetInfo, 0, 0, 10)); // enable feature to prevent idle state when coasting in gear engineConfiguration->disableIdleClutchUp = true; @@ -125,16 +125,16 @@ TEST(idle_v2, testDeterminePhase) { engine->updateSwitchInputs(); // should be same behaviour as before when clutch is pressed/car in neutral - EXPECT_EQ(ICP::Idling, dut.determinePhase(1099, 1000, 0, 0, 10)); - EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, 1000, 0, 0, 10)); - EXPECT_EQ(ICP::CrankToIdleTaper, dut.determinePhase(1000, 1000, 0, 0, 0.5f)); - EXPECT_EQ(ICP::Running, dut.determinePhase(1000, 1000, 10, 0, 10)); + EXPECT_EQ(ICP::Idling, dut.determinePhase(1099, targetInfo, 0, 0, 10)); + EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, targetInfo, 0, 0, 10)); + EXPECT_EQ(ICP::CrankToIdleTaper, dut.determinePhase(1000, targetInfo, 0, 0, 0.5f)); + EXPECT_EQ(ICP::Running, dut.determinePhase(1000, targetInfo, 10, 0, 10)); // clutchUp indicates car in gear with clutch not pressed, should recognize as coasting instead of idle now setMockState(engineConfiguration->clutchUpPin, true); engine->updateSwitchInputs(); - EXPECT_EQ(ICP::Coasting, dut.determinePhase(1099, 1000, 0, 0, 10)); - EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, 1000, 0, 0, 10)); + EXPECT_EQ(ICP::Coasting, dut.determinePhase(1099, targetInfo, 0, 0, 10)); + EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, targetInfo, 0, 0, 10)); // Below TPS but above RPM should be outside the zone EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, targetInfo, 0, 0, 10)); From 82d889b9684e95c96ff3a574042b9794501caa9a Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 30 Jun 2025 00:10:23 -0700 Subject: [PATCH 6/6] group --- unit_tests/tests/test_idle_controller.cpp | 42 +++++++++++------------ 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/unit_tests/tests/test_idle_controller.cpp b/unit_tests/tests/test_idle_controller.cpp index 390b9a8d18..fa4bd3edb3 100644 --- a/unit_tests/tests/test_idle_controller.cpp +++ b/unit_tests/tests/test_idle_controller.cpp @@ -113,28 +113,26 @@ TEST(idle_v2, testDeterminePhase) { // above upper limit and on throttle should be out of idle zone EXPECT_EQ(ICP::Running, dut.determinePhase(1101, targetInfo, 10, 0, 10)); - - EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, targetInfo, 0, 0, 10)); - EXPECT_EQ(ICP::Coasting, dut.determinePhase(5000, targetInfo, 0, 0, 10)); - - // enable feature to prevent idle state when coasting in gear - engineConfiguration->disableIdleClutchUp = true; - setMockState(engineConfiguration->clutchUpPin, false); - engineConfiguration->clutchUpPin = Gpio::G2; - engineConfiguration->clutchUpPinMode = PI_PULLDOWN; - engine->updateSwitchInputs(); - - // should be same behaviour as before when clutch is pressed/car in neutral - EXPECT_EQ(ICP::Idling, dut.determinePhase(1099, targetInfo, 0, 0, 10)); - EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, targetInfo, 0, 0, 10)); - EXPECT_EQ(ICP::CrankToIdleTaper, dut.determinePhase(1000, targetInfo, 0, 0, 0.5f)); - EXPECT_EQ(ICP::Running, dut.determinePhase(1000, targetInfo, 10, 0, 10)); - - // clutchUp indicates car in gear with clutch not pressed, should recognize as coasting instead of idle now - setMockState(engineConfiguration->clutchUpPin, true); - engine->updateSwitchInputs(); - EXPECT_EQ(ICP::Coasting, dut.determinePhase(1099, targetInfo, 0, 0, 10)); - EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, targetInfo, 0, 0, 10)); + { + // enable feature to prevent idle state when coasting in gear + engineConfiguration->disableIdleClutchUp = true; + setMockState(engineConfiguration->clutchUpPin, false); + engineConfiguration->clutchUpPin = Gpio::G2; + engineConfiguration->clutchUpPinMode = PI_PULLDOWN; + engine->updateSwitchInputs(); + + // should be same behaviour as before when clutch is pressed/car in neutral + EXPECT_EQ(ICP::Idling, dut.determinePhase(1099, targetInfo, 0, 0, 10)); + EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, targetInfo, 0, 0, 10)); + EXPECT_EQ(ICP::CrankToIdleTaper, dut.determinePhase(1000, targetInfo, 0, 0, 0.5f)); + EXPECT_EQ(ICP::Running, dut.determinePhase(1000, targetInfo, 10, 0, 10)); + + // clutchUp indicates car in gear with clutch not pressed, should recognize as coasting instead of idle now + setMockState(engineConfiguration->clutchUpPin, true); + engine->updateSwitchInputs(); + EXPECT_EQ(ICP::Coasting, dut.determinePhase(1099, targetInfo, 0, 0, 10)); + EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, targetInfo, 0, 0, 10)); + } // Below TPS but above RPM should be outside the zone EXPECT_EQ(ICP::Coasting, dut.determinePhase(1101, targetInfo, 0, 0, 10));