From 367436454da0fac43637242b000d8560a873fe64 Mon Sep 17 00:00:00 2001 From: rusefillc Date: Sat, 3 Oct 2020 22:00:00 -0400 Subject: [PATCH] HW CI has failed 11 times since Oct 2 #1849 Revert "ETB function config (#1831)" This reverts commit af59c45a --- .../actuators/electronic_throttle.cpp | 41 ++++-------- firmware/controllers/core/error_handling.cpp | 6 +- firmware/integration/rusefi_config.txt | 3 +- firmware/tunerstudio/rusefi.input | 2 - unit_tests/tests/test_etb.cpp | 66 ++----------------- unit_tests/unit_test_framework.h | 2 - 6 files changed, 20 insertions(+), 100 deletions(-) diff --git a/firmware/controllers/actuators/electronic_throttle.cpp b/firmware/controllers/actuators/electronic_throttle.cpp index 136b412d30..4ad660a94a 100644 --- a/firmware/controllers/actuators/electronic_throttle.cpp +++ b/firmware/controllers/actuators/electronic_throttle.cpp @@ -172,8 +172,6 @@ bool EtbController::init(etb_function_e function, DcMotor *motor, pid_s *pidPara m_pid.initPidClass(pidParameters); m_pedalMap = pedalMap; - reset(); - return true; } @@ -758,10 +756,6 @@ 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 @@ -828,42 +822,29 @@ 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); - // TODO: remove etbActualCount - engine->etbActualCount = ETB_COUNT; + engine->etbActualCount = Sensor::hasSensor(SensorType::Tps2) ? 2 : 1; - bool mustHaveEtbConfigured = Sensor::hasSensor(SensorType::AcceleratorPedalPrimary); - bool anyEtbConfigured = false; - - for (int i = 0 ; i < ETB_COUNT; i++) { + for (int i = 0 ; i < engine->etbActualCount; 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) { - auto controller = engine->etbControllers[i]; - if (!controller) { - continue; - } + // TODO: configure per-motor in config so wastegate/VW idle works + auto func = i == 0 ? ETB_Throttle1 : ETB_Throttle2; - auto func = CONFIG(etbFunctions[i]); - - anyEtbConfigured |= controller->init(func, motor, &engineConfiguration->etb, &pedal2tpsMap); + engine->etbControllers[i]->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) { @@ -877,6 +858,8 @@ 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 89a43c592a..9ee57977d5 100644 --- a/firmware/controllers/core/error_handling.cpp +++ b/firmware/controllers/core/error_handling.cpp @@ -236,10 +236,6 @@ 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) @@ -289,7 +285,7 @@ void firmwareError(obd_code_e code, const char *fmt, ...) { printf("\r\n"); #if EFI_SIMULATOR || EFI_UNIT_TEST - throw std::logic_error(fmt); + throw "fatal error"; #endif /* EFI_SIMULATOR */ #endif } diff --git a/firmware/integration/rusefi_config.txt b/firmware/integration/rusefi_config.txt index 6ed97fa755..a75fdf6707 100644 --- a/firmware/integration/rusefi_config.txt +++ b/firmware/integration/rusefi_config.txt @@ -951,8 +951,7 @@ 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; - 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 + uint8_t[2] solenoidPadding;;"units", 1, 0, -20, 100, 0 spi_device_e drv8860spiDevice; brain_pin_e drv8860_cs; diff --git a/firmware/tunerstudio/rusefi.input b/firmware/tunerstudio/rusefi.input index 31d745560e..2c5058f0ea 100644 --- a/firmware/tunerstudio/rusefi.input +++ b/firmware/tunerstudio/rusefi.input @@ -3044,8 +3044,6 @@ 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 a58f4d96d1..5522ca1b7d 100644 --- a/unit_tests/tests/test_etb.cpp +++ b/unit_tests/tests/test_etb.cpp @@ -27,34 +27,9 @@ TEST(etb, initializationNoPedal) { engine->etbControllers[i] = &mocks[i]; } - EXPECT_CALL(mocks[0], init(ETB_Throttle1, _, _, _)).WillOnce(Return(false)); - EXPECT_CALL(mocks[1], init(ETB_Throttle2, _, _, _)).WillOnce(Return(false)); + // We expect no throttle init stuff to be called - lack of pedal should disable ETB - // 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)); + doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE); } TEST(etb, initializationSingleThrottle) { @@ -70,39 +45,11 @@ 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); - // 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)); + // We do not expect throttle #2 to be initialized doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE); } @@ -123,14 +70,13 @@ 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 95823788b3..fbf6d5b7c4 100644 --- a/unit_tests/unit_test_framework.h +++ b/unit_tests/unit_test_framework.h @@ -32,5 +32,3 @@ 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)