diff --git a/firmware/controllers/actuators/electronic_throttle.cpp b/firmware/controllers/actuators/electronic_throttle.cpp index 4ad660a94a..136b412d30 100644 --- a/firmware/controllers/actuators/electronic_throttle.cpp +++ b/firmware/controllers/actuators/electronic_throttle.cpp @@ -172,6 +172,8 @@ bool EtbController::init(etb_function_e function, DcMotor *motor, pid_s *pidPara m_pid.initPidClass(pidParameters); m_pedalMap = pedalMap; + reset(); + return true; } @@ -756,6 +758,10 @@ void setDefaultEtbParameters(DECLARE_CONFIG_PARAMETER_SIGNATURE) { } } + // Default is to run each throttle off its respective hbridge + engineConfiguration->etbFunctions[0] = ETB_Throttle1; + engineConfiguration->etbFunctions[1] = ETB_Throttle2; + engineConfiguration->etbFreq = DEFAULT_ETB_PWM_FREQUENCY; // voltage, not ADC like with TPS @@ -822,29 +828,42 @@ void doInitElectronicThrottle(DECLARE_ENGINE_PARAMETER_SIGNATURE) { addConsoleActionI("etb_freq", setEtbFrequency); #endif /* EFI_PROD_CODE */ - // If you don't have a pedal we have no business here. - if (!Sensor::hasSensor(SensorType::AcceleratorPedalPrimary)) { - return; - } - pedal2tpsMap.init(config->pedalToTpsTable, config->pedalToTpsPedalBins, config->pedalToTpsRpmBins); - engine->etbActualCount = Sensor::hasSensor(SensorType::Tps2) ? 2 : 1; + // TODO: remove etbActualCount + engine->etbActualCount = ETB_COUNT; - for (int i = 0 ; i < engine->etbActualCount; i++) { + bool mustHaveEtbConfigured = Sensor::hasSensor(SensorType::AcceleratorPedalPrimary); + bool anyEtbConfigured = false; + + for (int i = 0 ; i < ETB_COUNT; i++) { auto motor = initDcMotor(i, CONFIG(etb_use_two_wires) PASS_ENGINE_PARAMETER_SUFFIX); // If this motor is actually set up, init the etb if (motor) { - // TODO: configure per-motor in config so wastegate/VW idle works - auto func = i == 0 ? ETB_Throttle1 : ETB_Throttle2; + auto controller = engine->etbControllers[i]; + if (!controller) { + continue; + } - engine->etbControllers[i]->init(func, motor, &engineConfiguration->etb, &pedal2tpsMap); + auto func = CONFIG(etbFunctions[i]); + + anyEtbConfigured |= controller->init(func, motor, &engineConfiguration->etb, &pedal2tpsMap); INJECT_ENGINE_REFERENCE(engine->etbControllers[i]); } } + if (!anyEtbConfigured) { + // It's not valid to have a PPS without any ETBs - check that at least one ETB was enabled along with the pedal + if (mustHaveEtbConfigured) { + firmwareError(OBD_PCM_Processor_Fault, "A pedal position sensor was configured, but no electronic throttles are configured."); + } + + // Don't start the thread if no throttles are in use. + return; + } + #if 0 && ! EFI_UNIT_TEST percent_t startupThrottlePosition = getTPS(PASS_ENGINE_PARAMETER_SIGNATURE); if (absF(startupThrottlePosition - engineConfiguration->etbNeutralPosition) > STARTUP_NEUTRAL_POSITION_ERROR_THRESHOLD) { @@ -858,8 +877,6 @@ void doInitElectronicThrottle(DECLARE_ENGINE_PARAMETER_SIGNATURE) { } #endif /* EFI_UNIT_TEST */ - etbPidReset(PASS_ENGINE_PARAMETER_SIGNATURE); - #if !EFI_UNIT_TEST etbThread.Start(); #endif diff --git a/firmware/controllers/core/error_handling.cpp b/firmware/controllers/core/error_handling.cpp index 9ee57977d5..89a43c592a 100644 --- a/firmware/controllers/core/error_handling.cpp +++ b/firmware/controllers/core/error_handling.cpp @@ -236,6 +236,10 @@ void onUnlockHook(void) { #endif /* ENABLE_PERF_TRACE */ } +#if EFI_SIMULATOR || EFI_UNIT_TEST +#include +#endif + void firmwareError(obd_code_e code, const char *fmt, ...) { #if EFI_PROD_CODE if (hasFirmwareErrorFlag) @@ -285,7 +289,7 @@ void firmwareError(obd_code_e code, const char *fmt, ...) { printf("\r\n"); #if EFI_SIMULATOR || EFI_UNIT_TEST - throw "fatal error"; + throw std::logic_error(fmt); #endif /* EFI_SIMULATOR */ #endif } diff --git a/firmware/integration/rusefi_config.txt b/firmware/integration/rusefi_config.txt index a75fdf6707..6ed97fa755 100644 --- a/firmware/integration/rusefi_config.txt +++ b/firmware/integration/rusefi_config.txt @@ -951,7 +951,8 @@ custom maf_sensor_type_e 4 bits, S32, @OFFSET@, [0:1], @@maf_sensor_type_e_enum@ output_pin_e[TCU_SOLENOID_COUNT iterate] tcu_solenoid; - uint8_t[2] solenoidPadding;;"units", 1, 0, -20, 100, 0 + custom etb_function_e 1 bits, U08, @OFFSET@, [0:2], "None", "Throttle 1", "Throttle 2", "Idle Valve", "Wastegate" + etb_function_e[ETB_COUNT iterate] etbFunctions spi_device_e drv8860spiDevice; brain_pin_e drv8860_cs; diff --git a/firmware/tunerstudio/rusefi.input b/firmware/tunerstudio/rusefi.input index 2c5058f0ea..31d745560e 100644 --- a/firmware/tunerstudio/rusefi.input +++ b/firmware/tunerstudio/rusefi.input @@ -3044,6 +3044,8 @@ cmd_set_engine_type_default = "@@TS_IO_TEST_COMMAND_char@@\x00\x31\x00\x00" field = "https://rusefi.com/s/etb" field = "Detailed status in console", isVerboseETB field = "Disable ETB Motor", pauseEtbControl + field = "H-Bridge #1 function", etbFunctions1 + field = "H-Bridge #2 function", etbFunctions2 ; 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 panel = hbridgeHardware, { throttlePedalPositionAdcChannel != @@ADC_CHANNEL_NONE@@ || (useStepperIdle && useHbridges) } diff --git a/unit_tests/tests/test_etb.cpp b/unit_tests/tests/test_etb.cpp index 5522ca1b7d..a58f4d96d1 100644 --- a/unit_tests/tests/test_etb.cpp +++ b/unit_tests/tests/test_etb.cpp @@ -27,9 +27,34 @@ TEST(etb, initializationNoPedal) { engine->etbControllers[i] = &mocks[i]; } - // We expect no throttle init stuff to be called - lack of pedal should disable ETB + EXPECT_CALL(mocks[0], init(ETB_Throttle1, _, _, _)).WillOnce(Return(false)); + EXPECT_CALL(mocks[1], init(ETB_Throttle2, _, _, _)).WillOnce(Return(false)); - doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE); + // This shouldn't throw, since no throttles are configured, but no pedal is configured either + EXPECT_NO_FATAL_ERROR(doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE)); +} + +TEST(etb, initializationMissingThrottle) { + StrictMock mocks[ETB_COUNT]; + + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); + + for (int i = 0; i < ETB_COUNT; i++) { + engine->etbControllers[i] = &mocks[i]; + } + + engineConfiguration->etbFunctions[0] = ETB_None; + engineConfiguration->etbFunctions[1] = ETB_None; + + EXPECT_CALL(mocks[0], init(ETB_None, _, _, _)).WillOnce(Return(false)); + EXPECT_CALL(mocks[1], init(ETB_None, _, _, _)).WillOnce(Return(false)); + + // Must have a sensor configured before init + Sensor::setMockValue(SensorType::AcceleratorPedal, 0); + Sensor::setMockValue(SensorType::AcceleratorPedalPrimary, 0); + + // This should throw: a pedal is configured but no throttles + EXPECT_FATAL_ERROR(doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE)); } TEST(etb, initializationSingleThrottle) { @@ -45,11 +70,39 @@ TEST(etb, initializationSingleThrottle) { Sensor::setMockValue(SensorType::AcceleratorPedal, 0); Sensor::setMockValue(SensorType::AcceleratorPedalPrimary, 0); + engineConfiguration->etbFunctions[0] = ETB_Throttle1; + engineConfiguration->etbFunctions[1] = ETB_None; + // Expect mock0 to be init as throttle 1, and PID params EXPECT_CALL(mocks[0], init(ETB_Throttle1, _, &engineConfiguration->etb, Ne(nullptr))).WillOnce(Return(true)); - EXPECT_CALL(mocks[0], reset); - // We do not expect throttle #2 to be initialized + // Expect mock1 to be init as none + EXPECT_CALL(mocks[1], init(ETB_None, _, _, _)).WillOnce(Return(true)); + + doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE); +} + +TEST(etb, initializationSingleThrottleInSecondSlot) { + StrictMock mocks[ETB_COUNT]; + + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); + + for (int i = 0; i < ETB_COUNT; i++) { + engine->etbControllers[i] = &mocks[i]; + } + + // Must have a sensor configured before init + Sensor::setMockValue(SensorType::AcceleratorPedal, 0); + Sensor::setMockValue(SensorType::AcceleratorPedalPrimary, 0); + + engineConfiguration->etbFunctions[0] = ETB_None; + engineConfiguration->etbFunctions[1] = ETB_Throttle1; + + // Expect mock0 to be init as none + EXPECT_CALL(mocks[0], init(ETB_None, _, _, _)).WillOnce(Return(true)); + + // Expect mock1 to be init as throttle 1, and PID params + EXPECT_CALL(mocks[1], init(ETB_Throttle1, _, &engineConfiguration->etb, Ne(nullptr))).WillOnce(Return(true)); doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE); } @@ -70,13 +123,14 @@ TEST(etb, initializationDualThrottle) { // The presence of a second TPS indicates dual throttle Sensor::setMockValue(SensorType::Tps2, 25.0f); + engineConfiguration->etbFunctions[0] = ETB_Throttle1; + engineConfiguration->etbFunctions[1] = ETB_Throttle2; + // Expect mock0 to be init as throttle 1, and PID params EXPECT_CALL(mocks[0], init(ETB_Throttle1, _, &engineConfiguration->etb, Ne(nullptr))).WillOnce(Return(true)); - EXPECT_CALL(mocks[0], reset); // Expect mock1 to be init as throttle 2, and PID params EXPECT_CALL(mocks[1], init(ETB_Throttle2, _, &engineConfiguration->etb, Ne(nullptr))).WillOnce(Return(true)); - EXPECT_CALL(mocks[1], reset); doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE); } diff --git a/unit_tests/unit_test_framework.h b/unit_tests/unit_test_framework.h index fbf6d5b7c4..95823788b3 100644 --- a/unit_tests/unit_test_framework.h +++ b/unit_tests/unit_test_framework.h @@ -32,3 +32,5 @@ void assertEqualsM(const char *msg, float expected, float actual); void assertEqualsLM(const char *msg, long expected, long actual); void assertEqualsM4(const char *prefix, const char *msg, float expected, float actual); +#define EXPECT_NO_FATAL_ERROR EXPECT_NO_THROW +#define EXPECT_FATAL_ERROR(expr) EXPECT_THROW((expr), std::logic_error)