diff --git a/firmware/controllers/actuators/alternator_controller.cpp b/firmware/controllers/actuators/alternator_controller.cpp index 01ade03bed..5c37058723 100644 --- a/firmware/controllers/actuators/alternator_controller.cpp +++ b/firmware/controllers/actuators/alternator_controller.cpp @@ -70,11 +70,14 @@ class AlternatorController : public PeriodicTimerController { // todo: migrate this to FSIO bool alternatorShouldBeEnabledAtCurrentRpm = GET_RPM() > engineConfiguration->cranking.rpm; - engine->isAlternatorControlEnabled = CONFIG(isAlternatorControlEnabled) && alternatorShouldBeEnabledAtCurrentRpm; - if (!engine->isAlternatorControlEnabled) { + if (!CONFIG(isAlternatorControlEnabled) || !alternatorShouldBeEnabledAtCurrentRpm) { // we need to avoid accumulating iTerm while engine is not running pidReset(); + + // Shut off output if not needed + alternatorControl.setSimplePwmDutyCycle(0); + return; } @@ -135,18 +138,6 @@ void setAltPFactor(float p) { showAltInfo(); } -static void applyAlternatorPinState(int stateIndex, PwmConfig *state) /* pwm_gen_callback */ { - efiAssertVoid(CUSTOM_ERR_6643, stateIndex < PWM_PHASE_MAX_COUNT, "invalid stateIndex"); - efiAssertVoid(CUSTOM_IDLE_WAVE_CNT, state->multiChannelStateSequence.waveCount == 1, "invalid idle waveCount"); - OutputPin *output = state->outputPins[0]; - int value = state->multiChannelStateSequence.getChannelState(/*channelIndex*/0, stateIndex); - /** - * 'engine->isAlternatorControlEnabled' would be false is RPM is too low - */ - if (!value || engine->isAlternatorControlEnabled) - output->setValue(value); -} - void setDefaultAlternatorParameters(DECLARE_CONFIG_PARAMETER_SIGNATURE) { engineConfiguration->alternatorOffAboveTps = 120; @@ -171,7 +162,7 @@ void initAlternatorCtrl(DECLARE_ENGINE_PARAMETER_SIGNATURE) { "Alternator control", &engine->executor, &enginePins.alternatorPin, - engineConfiguration->alternatorPwmFrequency, 0.1, (pwm_gen_callback*)applyAlternatorPinState); + engineConfiguration->alternatorPwmFrequency, 0); } instance.Start(); } diff --git a/firmware/controllers/algo/engine.h b/firmware/controllers/algo/engine.h index 8e21313022..5f05987793 100644 --- a/firmware/controllers/algo/engine.h +++ b/firmware/controllers/algo/engine.h @@ -185,10 +185,6 @@ public: * this is based on sensorChartMode and sensorSnifferRpmThreshold settings */ sensor_chart_e sensorChartMode = SC_OFF; - /** - * based on current RPM and isAlternatorControlEnabled setting - */ - bool isAlternatorControlEnabled = false; bool slowCallBackWasInvoked = false; diff --git a/firmware/controllers/system/timer/pwm_generator_logic.cpp b/firmware/controllers/system/timer/pwm_generator_logic.cpp index 7a011150d4..ab8554e28a 100644 --- a/firmware/controllers/system/timer/pwm_generator_logic.cpp +++ b/firmware/controllers/system/timer/pwm_generator_logic.cpp @@ -335,10 +335,9 @@ void PwmConfig::weComplexInit(const char *msg, ExecutorInterface *executor, } void startSimplePwm(SimplePwm *state, const char *msg, ExecutorInterface *executor, - OutputPin *output, float frequency, float dutyCycle, pwm_gen_callback *stateChangeCallback) { + OutputPin *output, float frequency, float dutyCycle) { efiAssertVoid(CUSTOM_ERR_PWM_STATE_ASSERT, state != NULL, "state"); efiAssertVoid(CUSTOM_ERR_PWM_DUTY_ASSERT, dutyCycle >= 0 && dutyCycle <= 1, "dutyCycle"); - efiAssertVoid(CUSTOM_ERR_PWM_CALLBACK_ASSERT, stateChangeCallback != NULL, "listener"); if (frequency < 1) { warning(CUSTOM_OBD_LOW_FREQUENCY, "low frequency %.2f", frequency); return; @@ -353,17 +352,17 @@ void startSimplePwm(SimplePwm *state, const char *msg, ExecutorInterface *execut state->outputPins[0] = output; state->setFrequency(frequency); - state->weComplexInit(msg, executor, 2, switchTimes, 1, pinStates, NULL, stateChangeCallback); + state->weComplexInit(msg, executor, 2, switchTimes, 1, pinStates, NULL, (pwm_gen_callback*)applyPinState); } void startSimplePwmExt(SimplePwm *state, const char *msg, ExecutorInterface *executor, brain_pin_e brainPin, OutputPin *output, float frequency, - float dutyCycle, pwm_gen_callback *stateChangeCallback) { + float dutyCycle) { output->initPin(msg, brainPin); - startSimplePwm(state, msg, executor, output, frequency, dutyCycle, stateChangeCallback); + startSimplePwm(state, msg, executor, output, frequency, dutyCycle); } void startSimplePwmHard(SimplePwm *state, const char *msg, diff --git a/firmware/controllers/system/timer/pwm_generator_logic.h b/firmware/controllers/system/timer/pwm_generator_logic.h index 1269a562a6..21cf222c78 100644 --- a/firmware/controllers/system/timer/pwm_generator_logic.h +++ b/firmware/controllers/system/timer/pwm_generator_logic.h @@ -145,7 +145,7 @@ void applyPinState(int stateIndex, PwmConfig* state) /* pwm_gen_callback */; void startSimplePwm(SimplePwm *state, const char *msg, ExecutorInterface *executor, OutputPin *output, - float frequency, float dutyCycle, pwm_gen_callback *stateChangeCallback = (pwm_gen_callback*)applyPinState); + float frequency, float dutyCycle); /** * initialize GPIO pin and start a one-channel software PWM driver. @@ -156,7 +156,7 @@ void startSimplePwmExt(SimplePwm *state, const char *msg, ExecutorInterface *executor, brain_pin_e brainPin, OutputPin *output, - float frequency, float dutyCycle, pwm_gen_callback *stateChangeCallback = (pwm_gen_callback*)applyPinState); + float frequency, float dutyCycle); void startSimplePwmHard(SimplePwm *state, const char *msg, ExecutorInterface *executor, diff --git a/firmware/hw_layer/sensors/cj125.cpp b/firmware/hw_layer/sensors/cj125.cpp index 88df9e0f55..f729a5c0c3 100644 --- a/firmware/hw_layer/sensors/cj125.cpp +++ b/firmware/hw_layer/sensors/cj125.cpp @@ -628,7 +628,7 @@ void initCJ125(DECLARE_ENGINE_PARAMETER_SIGNATURE) { if (!cjStartSpi(PASS_ENGINE_PARAMETER_SIGNATURE)) return; efiPrintf("cj125: Starting heater control"); - globalInstance.StartHeaterControl((pwm_gen_callback*)applyPinState PASS_ENGINE_PARAMETER_SUFFIX); + globalInstance.StartHeaterControl(PASS_ENGINE_PARAMETER_SIGNATURE); cjStart(PASS_ENGINE_PARAMETER_SIGNATURE); #ifdef CJ125_DEBUG diff --git a/firmware/hw_layer/sensors/cj125_logic.cpp b/firmware/hw_layer/sensors/cj125_logic.cpp index 8068c98399..1b0d59ffc6 100644 --- a/firmware/hw_layer/sensors/cj125_logic.cpp +++ b/firmware/hw_layer/sensors/cj125_logic.cpp @@ -41,12 +41,12 @@ bool CJ125::isWorkingState(void) const { return state != CJ125_ERROR && state != CJ125_INIT && state != CJ125_IDLE; } -void CJ125::StartHeaterControl(pwm_gen_callback *stateChangeCallback DECLARE_ENGINE_PARAMETER_SUFFIX) { +void CJ125::StartHeaterControl(DECLARE_ENGINE_PARAMETER_SIGNATURE) { // todo: use custom pin state method, turn pin off while not running startSimplePwmExt(&wboHeaterControl, "wboHeaterPin", &engine->executor, CONFIG(wboHeaterPin), - &wboHeaterPin, CJ125_HEATER_PWM_FREQ, 0.0f, stateChangeCallback); + &wboHeaterPin, CJ125_HEATER_PWM_FREQ, 0.0f); SetIdleHeater(PASS_ENGINE_PARAMETER_SIGNATURE); } diff --git a/firmware/hw_layer/sensors/cj125_logic.h b/firmware/hw_layer/sensors/cj125_logic.h index 29e0c9f6df..95cf97bfa7 100644 --- a/firmware/hw_layer/sensors/cj125_logic.h +++ b/firmware/hw_layer/sensors/cj125_logic.h @@ -99,7 +99,7 @@ public: bool isWorkingState(void) const; void SetHeater(float value DECLARE_ENGINE_PARAMETER_SUFFIX); void SetIdleHeater(DECLARE_ENGINE_PARAMETER_SIGNATURE); - void StartHeaterControl(pwm_gen_callback *stateChangeCallback DECLARE_ENGINE_PARAMETER_SUFFIX); + void StartHeaterControl(DECLARE_ENGINE_PARAMETER_SIGNATURE); bool cjIdentify(DECLARE_ENGINE_PARAMETER_SIGNATURE); void printDiag(); void calibrate(DECLARE_ENGINE_PARAMETER_SIGNATURE); diff --git a/unit_tests/tests/sensor/test_cj125.cpp b/unit_tests/tests/sensor/test_cj125.cpp index 76733fd62f..425d12d732 100644 --- a/unit_tests/tests/sensor/test_cj125.cpp +++ b/unit_tests/tests/sensor/test_cj125.cpp @@ -8,10 +8,6 @@ #include "cj125_logic.h" #include "engine_test_helper.h" -static void applyHeaterPinState(PwmConfig *state, int stateIndex) { - -} - class TestSpi : public Cj125SpiStream { public: MOCK_METHOD1(ReadRegister, uint8_t(uint8_t)); @@ -27,7 +23,7 @@ TEST(testCJ125, testInitialState) { WITH_ENGINE_TEST_HELPER(FORD_ASPIRE_1996); - cj.StartHeaterControl((pwm_gen_callback*)&applyHeaterPinState PASS_ENGINE_PARAMETER_SUFFIX); + cj.StartHeaterControl(PASS_ENGINE_PARAMETER_SIGNATURE); ASSERT_EQ(cj.heaterDuty, CJ125_HEATER_IDLE_RATE); TestSpi mock; diff --git a/unit_tests/tests/test_pwm_generator.cpp b/unit_tests/tests/test_pwm_generator.cpp index 886d015eb9..cd2f9d3ffe 100644 --- a/unit_tests/tests/test_pwm_generator.cpp +++ b/unit_tests/tests/test_pwm_generator.cpp @@ -15,15 +15,8 @@ extern int timeNowUs; static int expectedTimeOfNextEvent; -static int pinValue = -1; -static void testApplyPinState(int stateIndex, PwmConfig *state) /* pwm_gen_callback */ { - pinValue = state->multiChannelStateSequence.getChannelState(/*channelIndex*/0, stateIndex); - - printf("PWM_test: setPinValue=%d @ timeNow=%d\r\n", pinValue, timeNowUs); -} - -static void assertNextEvent(const char *msg, int expectedPinState, TestExecutor *executor) { +static void assertNextEvent(const char *msg, int expectedPinState, TestExecutor *executor, OutputPin& pin) { printf("PWM_test: Asserting event [%s]\r\n", msg); // only one action expected in queue ASSERT_EQ( 1, executor->size()) << "PWM_test: schedulingQueue size"; @@ -33,7 +26,7 @@ static void assertNextEvent(const char *msg, int expectedPinState, TestExecutor // execute pending actions and assert that only one action was executed ASSERT_NEAR(1, executor->executeAll(timeNowUs), 0) << msg << " executed"; - ASSERT_NEAR(expectedPinState, pinValue, 0) << msg << " pin state"; + ASSERT_NEAR(expectedPinState, pin.currentLogicValue, 0) << msg << " pin state"; // assert that we have one new action in queue ASSERT_EQ( 1, executor->size()) << "PWM_test: queue.size"; @@ -51,19 +44,18 @@ static void test100dutyCycle() { &executor, &pin, 1000 /* frequency */, - 1.0 /* duty cycle */, - (pwm_gen_callback*)&testApplyPinState); + 1.0 /* duty cycle */); expectedTimeOfNextEvent += 1000; assertEqualsM2("1@1000/100", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0); - assertNextEvent("exec@100", HIGH_VALUE, &executor); + assertNextEvent("exec@100", HIGH_VALUE, &executor, pin); expectedTimeOfNextEvent += 1000; - assertNextEvent("exec2@100", HIGH_VALUE, &executor); + assertNextEvent("exec2@100", HIGH_VALUE, &executor, pin); expectedTimeOfNextEvent += 1000; - assertNextEvent("exec3@100", HIGH_VALUE, &executor); + assertNextEvent("exec3@100", HIGH_VALUE, &executor, pin); } static void testSwitchToNanPeriod() { @@ -78,27 +70,26 @@ static void testSwitchToNanPeriod() { &executor, &pin, 1000 /* frequency */, - 0.60 /* duty cycle */, - (pwm_gen_callback*)&testApplyPinState); + 0.60 /* duty cycle */); expectedTimeOfNextEvent += 600; assertEqualsM2("1@1000/70", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0); - assertNextEvent("exec@70", LOW_VALUE, &executor); + assertNextEvent("exec@70", LOW_VALUE, &executor, pin); ASSERT_EQ( 600, timeNowUs) << "time1"; expectedTimeOfNextEvent += 400; - assertNextEvent("exec2@70", HIGH_VALUE, &executor); + assertNextEvent("exec2@70", HIGH_VALUE, &executor, pin); pwm.setFrequency(NAN); expectedTimeOfNextEvent += 600; assertEqualsM2("1@1000/NAN", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0); - assertNextEvent("exec2@NAN", LOW_VALUE, &executor); + assertNextEvent("exec2@NAN", LOW_VALUE, &executor, pin); expectedTimeOfNextEvent += MS2US(NAN_FREQUENCY_SLEEP_PERIOD_MS); assertEqualsM2("2@1000/NAN", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0); - assertNextEvent("exec3@NAN", LOW_VALUE, &executor); + assertNextEvent("exec3@NAN", LOW_VALUE, &executor, pin); } TEST(misc, testPwmGenerator) { @@ -115,14 +106,13 @@ TEST(misc, testPwmGenerator) { &executor, &pin, 1000 /* frequency */, - 0.80 /* duty cycle */, - (pwm_gen_callback*)&testApplyPinState); + 0.80 /* duty cycle */); expectedTimeOfNextEvent += 800; assertEqualsM2("1@1000/80", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0); - assertNextEvent("exec@0", LOW_VALUE, &executor); + assertNextEvent("exec@0", LOW_VALUE, &executor, pin); ASSERT_EQ( 800, timeNowUs) << "time1"; expectedTimeOfNextEvent += 200; @@ -132,33 +122,33 @@ TEST(misc, testPwmGenerator) { pwm.setSimplePwmDutyCycle(0); assertEqualsM2("2@1000/0", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0); - assertNextEvent("exec@1", LOW_VALUE, &executor); + assertNextEvent("exec@1", LOW_VALUE, &executor, pin); ASSERT_EQ( 1000, timeNowUs) << "time2"; expectedTimeOfNextEvent += 1000; assertEqualsM2("3@1000/0", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0); - assertNextEvent("exec@2", LOW_VALUE /* pin value */, &executor); + assertNextEvent("exec@2", LOW_VALUE /* pin value */, &executor, pin); ASSERT_EQ( 2000, timeNowUs) << "time3"; expectedTimeOfNextEvent += 1000; assertEqualsM2("4@1000/0", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0); - assertNextEvent("exec@3", LOW_VALUE /* pin value */, &executor); + assertNextEvent("exec@3", LOW_VALUE /* pin value */, &executor, pin); ASSERT_EQ( 3000, timeNowUs) << "time4"; expectedTimeOfNextEvent += 1000; assertEqualsM2("5@1000/0", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0); - assertNextEvent("exec@4", LOW_VALUE /* pin value */, &executor); + assertNextEvent("exec@4", LOW_VALUE /* pin value */, &executor, pin); expectedTimeOfNextEvent += 1000; assertEqualsM2("6@1000/0", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0); - assertNextEvent("exec@5", LOW_VALUE /* pin value */, &executor); + assertNextEvent("exec@5", LOW_VALUE /* pin value */, &executor, pin); expectedTimeOfNextEvent += 1000; ASSERT_EQ( 5000, timeNowUs) << "time4"; assertEqualsM2("7@1000/0", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0); - assertNextEvent("exec@6", LOW_VALUE /* pin value */, &executor); + assertNextEvent("exec@6", LOW_VALUE /* pin value */, &executor, pin); }