From d728b1ca48cd94e3864306542ada63443252fded Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Tue, 28 Apr 2020 13:52:40 -0700 Subject: [PATCH] Minor ETB improvements (#1381) * fix pid reset and pedal failure * pause control later * test pauseEtbControl * update tooltip + field name * update test --- .../actuators/electronic_throttle.cpp | 17 ++++----- firmware/integration/rusefi_config.txt | 2 +- firmware/tunerstudio/rusefi.input | 2 +- unit_tests/tests/test_etb.cpp | 35 ++++++++++++++++--- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/firmware/controllers/actuators/electronic_throttle.cpp b/firmware/controllers/actuators/electronic_throttle.cpp index 059d04cee5..b75ca3bb1e 100644 --- a/firmware/controllers/actuators/electronic_throttle.cpp +++ b/firmware/controllers/actuators/electronic_throttle.cpp @@ -124,7 +124,7 @@ void EtbController::reset() { } void EtbController::onConfigurationChange(pid_s* previousConfiguration) { - if (m_motor && m_pid.isSame(previousConfiguration)) { + if (m_motor && !m_pid.isSame(previousConfiguration)) { m_shouldResetPid = true; } } @@ -147,21 +147,17 @@ expected EtbController::getSetpoint() const { return unexpected; } - if (engineConfiguration->pauseEtbControl) { - return unexpected; - } - // If the pedal map hasn't been set, we can't provide a setpoint. if (!m_pedalMap) { return unexpected; } auto pedalPosition = Sensor::get(SensorType::AcceleratorPedal); - if (!pedalPosition.Valid) { - return unexpected; - } - float sanitizedPedal = clampF(0, pedalPosition.Value, 100); + // If the pedal has failed, just use 0 position. + // This is safer than disabling throttle control - we can at least push the throttle closed + // and let the engine idle. + float sanitizedPedal = clampF(0, pedalPosition.value_or(0), 100); float rpm = GET_RPM(); float targetFromTable = m_pedalMap->getValue(rpm / RPM_1_BYTE_PACKING_MULT, sanitizedPedal); @@ -301,7 +297,8 @@ void EtbController::setOutput(expected outputValue) { if (!m_motor) return; - if (outputValue) { + // If output is valid and we aren't paused, output to motor. + if (outputValue && !engineConfiguration->pauseEtbControl) { m_motor->enable(); m_motor->set(ETB_PERCENT_TO_DUTY(outputValue.Value)); } else { diff --git a/firmware/integration/rusefi_config.txt b/firmware/integration/rusefi_config.txt index ce68940b80..a50d971b87 100644 --- a/firmware/integration/rusefi_config.txt +++ b/firmware/integration/rusefi_config.txt @@ -713,7 +713,7 @@ bit is_enabled_spi_2 bit useIdleTimingPidControl bit useTPSBasedVeTable bit is_enabled_spi_4 - bit pauseEtbControl + bit pauseEtbControl;+Disable the electronic throttle motor for testing.\nThis mode is for testing ETB position sensors, etc without actually driving the throttle. bit alignEngineSnifferAtTDC bit useETBforIdleControl;+This setting allows the ETB to act as the idle air control valve and move to regulate the airflow at idle. bit idleIncrementalPidCic diff --git a/firmware/tunerstudio/rusefi.input b/firmware/tunerstudio/rusefi.input index c98a9a93cf..531a2c0b19 100644 --- a/firmware/tunerstudio/rusefi.input +++ b/firmware/tunerstudio/rusefi.input @@ -2793,7 +2793,7 @@ cmd_set_engine_type_default = "w\x00\x31\x00\x00" dialog = etbDialogLeft field = "https://rusefi.com/s/etb" field = "Detailed status in console", isVerboseETB - field = "Pause ETB control", pauseEtbControl + field = "Disable ETB Motor", pauseEtbControl field = etbCalibrationOnStart, etbCalibrationOnStart ; we need the term about stepper idle in here, because there's a bug in TS that you can't have different visibility ; criteria for the same panel when used in multiple places diff --git a/unit_tests/tests/test_etb.cpp b/unit_tests/tests/test_etb.cpp index 98e184203b..b554e4952c 100644 --- a/unit_tests/tests/test_etb.cpp +++ b/unit_tests/tests/test_etb.cpp @@ -144,9 +144,9 @@ TEST(etb, testSetpointOnlyPedal) { Sensor::setMockValue(SensorType::AcceleratorPedal, 20); EXPECT_EQ(20, etb.getSetpoint().value_or(-1)); - // Test invalid pedal position - should give unexpected + // Test invalid pedal position - should give 0 position Sensor::resetMockValue(SensorType::AcceleratorPedal); - EXPECT_EQ(etb.getSetpoint(), unexpected); + EXPECT_EQ(0, etb.getSetpoint().value_or(-1)); } TEST(etb, setpointIdle) { @@ -223,9 +223,12 @@ TEST(etb, etbTpsSensor) { } TEST(etb, setOutputInvalid) { + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); + StrictMock motor; EtbController etb; + INJECT_ENGINE_REFERENCE(&etb); etb.init(&motor, 0, nullptr, nullptr); // Should be disabled in case of unexpected @@ -235,9 +238,11 @@ TEST(etb, setOutputInvalid) { } TEST(etb, setOutputValid) { + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); StrictMock motor; EtbController etb; + INJECT_ENGINE_REFERENCE(&etb); etb.init(&motor, 0, nullptr, nullptr); // Should be enabled and value set @@ -249,10 +254,11 @@ TEST(etb, setOutputValid) { } TEST(etb, setOutputValid2) { + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); StrictMock motor; EtbController etb; - + INJECT_ENGINE_REFERENCE(&etb); etb.init(&motor, 0, nullptr, nullptr); // Should be enabled and value set @@ -264,9 +270,11 @@ TEST(etb, setOutputValid2) { } TEST(etb, setOutputOutOfRangeHigh) { + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); StrictMock motor; EtbController etb; + INJECT_ENGINE_REFERENCE(&etb); etb.init(&motor, 0, nullptr, nullptr); // Should be enabled and value set @@ -278,9 +286,11 @@ TEST(etb, setOutputOutOfRangeHigh) { } TEST(etb, setOutputOutOfRangeLow) { + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); StrictMock motor; EtbController etb; + INJECT_ENGINE_REFERENCE(&etb); etb.init(&motor, 0, nullptr, nullptr); // Should be enabled and value set @@ -291,12 +301,29 @@ TEST(etb, setOutputOutOfRangeLow) { etb.setOutput(-110); } +TEST(etb, setOutputPauseControl) { + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); + StrictMock motor; + + EtbController etb; + INJECT_ENGINE_REFERENCE(&etb); + etb.init(&motor, 0, nullptr, nullptr); + + // Pause control - should get no output + engineConfiguration->pauseEtbControl = true; + + // Disable should be called, and set shouldn't be called + EXPECT_CALL(motor, disable()); + + etb.setOutput(25.0f); +} + TEST(etb, closedLoopPid) { pid_s pid = {}; pid.pFactor = 5; pid.maxValue = 75; pid.minValue = -60; - + EtbController etb; etb.init(nullptr, 0, &pid, nullptr);