From a50f8e0077e68fa7d2efb4d47100bf12f110534a Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 31 May 2021 03:18:15 -0700 Subject: [PATCH] Idle test that fails (#2781) * make member func * failing test --- .../controllers/actuators/idle_thread.cpp | 4 +- firmware/controllers/actuators/idle_thread.h | 3 + unit_tests/tests/test_idle_controller.cpp | 62 +++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/firmware/controllers/actuators/idle_thread.cpp b/firmware/controllers/actuators/idle_thread.cpp index ba18c895b2..abec567677 100644 --- a/firmware/controllers/actuators/idle_thread.cpp +++ b/firmware/controllers/actuators/idle_thread.cpp @@ -340,7 +340,7 @@ static void undoIdleBlipIfNeeded() { /** * @return idle valve position percentage for automatic closed loop mode */ -static percent_t automaticIdleController(float tpsPos, float rpm, int targetRpm, IIdleController::Phase phase DECLARE_ENGINE_PARAMETER_SUFFIX) { +float IdleController::getClosedLoop(IIdleController::Phase phase, float tpsPos, int rpm, int targetRpm) { auto idlePid = getIdlePid(PASS_ENGINE_PARAMETER_SIGNATURE); if (shouldResetPid) { @@ -515,7 +515,7 @@ static percent_t automaticIdleController(float tpsPos, float rpm, int targetRpm, // let's re-apply CLT correction iacPosition = manualIdleController(cltCorrection PASS_ENGINE_PARAMETER_SUFFIX); } else { - iacPosition = automaticIdleController(tps.Value, rpm, targetRpm, phase PASS_ENGINE_PARAMETER_SUFFIX); + iacPosition = getClosedLoop(phase, tps.Value, rpm, targetRpm); } iacPosition = clampPercentValue(iacPosition); diff --git a/firmware/controllers/actuators/idle_thread.h b/firmware/controllers/actuators/idle_thread.h index b0d707a47e..2b5919cbed 100644 --- a/firmware/controllers/actuators/idle_thread.h +++ b/firmware/controllers/actuators/idle_thread.h @@ -51,6 +51,9 @@ public: float getIdleTimingAdjustment(int rpm); float getIdleTimingAdjustment(int rpm, int targetRpm, Phase phase); + // CLOSED LOOP CORRECTION + float getClosedLoop(IIdleController::Phase phase, float tpsPos, int rpm, int targetRpm); + // Allow querying state from outside bool isIdling() { return m_lastPhase == Phase::Idling; diff --git a/unit_tests/tests/test_idle_controller.cpp b/unit_tests/tests/test_idle_controller.cpp index 134700543b..9d0ce03b26 100644 --- a/unit_tests/tests/test_idle_controller.cpp +++ b/unit_tests/tests/test_idle_controller.cpp @@ -270,3 +270,65 @@ TEST(idle_v2, openLoopRunningTaper) { } EXPECT_FLOAT_EQ(25, dut.getOpenLoop(ICP::Idling, 30, 0)); } + +extern int timeNowUs; + +TEST(idle_v2, closedLoopBasic) { + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); + IdleController dut; + INJECT_ENGINE_REFERENCE(&dut); + + // Not testing PID here, so we can set very simple PID gains + CONFIG(idleRpmPid).pFactor = 0.5; // 0.5 output per 1 RPM error = 50% per 100 rpm + CONFIG(idleRpmPid).iFactor = 0; + CONFIG(idleRpmPid).dFactor = 0; + CONFIG(idleRpmPid).offset = 0; + CONFIG(idleRpmPid).iFactor = 0; + CONFIG(idleRpmPid).periodMs = 0; + CONFIG(idleRpmPid).minValue = -50; + CONFIG(idleRpmPid).maxValue = 50; + + CONFIG(idlePidRpmDeadZone) = 0; + + // burn one update then advance time 5 seconds to avoid difficulty from wasResetPid + dut.getClosedLoop(ICP::Idling, 0, 900, 900); + timeNowUs += 5'000'000; + + // Test above target, should return negative + // TODO: this fails, returns 55, why? + // EXPECT_FLOAT_EQ(-25, dut.getClosedLoop(ICP::Idling, 0, /*rpm*/ 950, /*tgt*/ 900)); + + // Below target, should return positive + // TODO: this fails, returns 55, why? + // EXPECT_FLOAT_EQ(25, dut.getClosedLoop(ICP::Idling, 0, /*rpm*/ 850, /*tgt*/ 900)); +} + +TEST(idle_v2, closedLoopDeadzone) { + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); + IdleController dut; + INJECT_ENGINE_REFERENCE(&dut); + + // Not testing PID here, so we can set very simple PID gains + CONFIG(idleRpmPid).pFactor = 0.5; // 0.5 output per 1 RPM error = 50% per 100 rpm + CONFIG(idleRpmPid).iFactor = 0; + CONFIG(idleRpmPid).dFactor = 0; + CONFIG(idleRpmPid).offset = 0; + CONFIG(idleRpmPid).iFactor = 0; + CONFIG(idleRpmPid).periodMs = 0; + CONFIG(idleRpmPid).minValue = -50; + CONFIG(idleRpmPid).maxValue = 50; + + CONFIG(idlePidRpmDeadZone) = 25; + + // burn one then advance time 5 seconds to avoid difficulty from wasResetPid + dut.getClosedLoop(ICP::Idling, 0, 900, 900); + timeNowUs += 5'000'000; + + // Test above target, should return negative + // TODO: this fails, returns 55, why? + // EXPECT_FLOAT_EQ(-25, dut.getClosedLoop(ICP::Idling, 0, /*rpm*/ 950, /*tgt*/ 900)); + + // Inside deadzone, should return same as last time + // TODO: this fails, returns 55, why? + // EXPECT_FLOAT_EQ(-25, dut.getClosedLoop(ICP::Idling, 0, /*rpm*/ 900, /*tgt*/ 900)); +}