From 47fbb92bffab8a89a2ba9b4adb1acb63a7e369a5 Mon Sep 17 00:00:00 2001 From: rusefillc Date: Wed, 18 Nov 2020 23:54:30 -0500 Subject: [PATCH] Starter seems to be engaged forever fix #1965 --- firmware/controllers/algo/engine.h | 1 + firmware/controllers/engine_controller_misc.cpp | 15 +++++++++++++++ firmware/controllers/system/efi_gpio.cpp | 4 ++-- firmware/controllers/system/efi_gpio.h | 2 +- firmware/hw_layer/io_pins.h | 1 - unit_tests/tests/test_start_stop.cpp | 8 ++++---- 6 files changed, 23 insertions(+), 8 deletions(-) diff --git a/firmware/controllers/algo/engine.h b/firmware/controllers/algo/engine.h index a887e27f0b..6acd9ccade 100644 --- a/firmware/controllers/algo/engine.h +++ b/firmware/controllers/algo/engine.h @@ -90,6 +90,7 @@ public: PrimaryTriggerConfiguration primaryTriggerConfiguration; VvtTriggerConfiguration vvtTriggerConfiguration; + efitick_t startStopStateLastPushTime = 0; #if EFI_SHAFT_POSITION_INPUT void OnTriggerStateDecodingError(); diff --git a/firmware/controllers/engine_controller_misc.cpp b/firmware/controllers/engine_controller_misc.cpp index 61e5a63d47..bf50342751 100644 --- a/firmware/controllers/engine_controller_misc.cpp +++ b/firmware/controllers/engine_controller_misc.cpp @@ -149,6 +149,7 @@ static void onStartStopButtonToggle(DECLARE_ENGINE_PARAMETER_SIGNATURE) { if (engine->rpmCalculator.isStopped()) { bool wasStarterEngaged = enginePins.starterControl.getAndSet(1); if (!wasStarterEngaged) { + engine->startStopStateLastPushTime = getTimeNowNt(); scheduleMsg(&sharedLogger, "Let's crank this engine for up to %dseconds!", CONFIG(startCrankingDuration)); } } else if (engine->rpmCalculator.isRunning()) { @@ -167,12 +168,26 @@ void slowStartStopButtonCallback(DECLARE_ENGINE_PARAMETER_SIGNATURE) { } engine->startStopState = startStopState; + if (engine->startStopStateLastPushTime == 0) { + // nothing is going on with startStop button + return; + } + // todo: should this be simply FSIO? if (engine->rpmCalculator.isRunning()) { // turn starter off once engine is running bool wasStarterEngaged = enginePins.starterControl.getAndSet(0); if (wasStarterEngaged) { scheduleMsg(&sharedLogger, "Engine runs we can disengage the starter"); + engine->startStopStateLastPushTime = 0; + } + } + + if (getTimeNowNt() - engine->startStopStateLastPushTime > NT_PER_SECOND * CONFIG(startCrankingDuration)) { + bool wasStarterEngaged = enginePins.starterControl.getAndSet(0); + if (wasStarterEngaged) { + scheduleMsg(&sharedLogger, "Cranking timeout %d seconds", CONFIG(startCrankingDuration)); + engine->startStopStateLastPushTime = 0; } } } diff --git a/firmware/controllers/system/efi_gpio.cpp b/firmware/controllers/system/efi_gpio.cpp index 237988273b..d79efc4e23 100644 --- a/firmware/controllers/system/efi_gpio.cpp +++ b/firmware/controllers/system/efi_gpio.cpp @@ -352,7 +352,7 @@ void InjectorOutputPin::reset() { } // todo: this could be refactored by calling some super-reset method - currentLogicValue = INITIAL_PIN_STATE; + currentLogicValue = 0; } IgnitionOutputPin::IgnitionOutputPin() { @@ -524,7 +524,7 @@ void OutputPin::initPin(const char *msg, brain_pin_e brainPin, const pin_output_ } #endif - this->currentLogicValue = INITIAL_PIN_STATE; + this->currentLogicValue = 0; // The order of the next two calls may look strange, which is a good observation. // We call them in this order so that the pin is set to a known state BEFORE diff --git a/firmware/controllers/system/efi_gpio.h b/firmware/controllers/system/efi_gpio.h index 573755eadb..63aff8bb48 100644 --- a/firmware/controllers/system/efi_gpio.h +++ b/firmware/controllers/system/efi_gpio.h @@ -74,7 +74,7 @@ public: brain_pin_e brainPin; #endif /* EFI_GPIO_HARDWARE */ - int8_t currentLogicValue = INITIAL_PIN_STATE; + int8_t currentLogicValue = 0; /** * we track current pin status so that we do not touch the actual hardware if we want to write new pin bit * which is same as current pin value. This maybe helps in case of status leds, but maybe it's a total over-engineering diff --git a/firmware/hw_layer/io_pins.h b/firmware/hw_layer/io_pins.h index 533d5be6af..3594da0f12 100644 --- a/firmware/hw_layer/io_pins.h +++ b/firmware/hw_layer/io_pins.h @@ -10,7 +10,6 @@ #include "global.h" -#define INITIAL_PIN_STATE -1 #define GPIO_NULL NULL // mode >= 0 is always true since that's an unsigned diff --git a/unit_tests/tests/test_start_stop.cpp b/unit_tests/tests/test_start_stop.cpp index 7b271e1d77..5c9f7855b0 100644 --- a/unit_tests/tests/test_start_stop.cpp +++ b/unit_tests/tests/test_start_stop.cpp @@ -10,6 +10,8 @@ TEST(start, startStop) { WITH_ENGINE_TEST_HELPER(BMW_M73_PROTEUS); + eth.smartMoveTimeForwardSeconds(1); // '0' time has special meaning for implementation so let's move forward + // this is a pull-up, so 'true' on start-up setMockState(engineConfiguration->startStopButtonPin, true); @@ -33,8 +35,6 @@ TEST(start, startStop) { eth.smartMoveTimeForwardSeconds(5); slowStartStopButtonCallback(PASS_ENGINE_PARAMETER_SIGNATURE); - // todo: FIX THIS, starter wire should go off on timeout! - ASSERT_TRUE(efiReadPin(engineConfiguration->starterControlPin)); - - + // starter is now OFF due to timeout + ASSERT_FALSE(efiReadPin(engineConfiguration->starterControlPin)); }