From 95a7a9a8e147aead67394dce792955cd04ae5fa8 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Tue, 7 Jan 2020 15:10:31 -0800 Subject: [PATCH] Strongly typed action constructor (#1087) * strong typing * maybe we needed that one --- firmware/controllers/engine_cycle/aux_valves.cpp | 10 +++++----- .../engine_cycle/main_trigger_callback.cpp | 12 ++++++------ firmware/controllers/engine_cycle/rpm_calculator.cpp | 2 +- firmware/controllers/engine_cycle/spark_logic.cpp | 4 ++-- .../controllers/system/timer/pwm_generator_logic.cpp | 2 +- firmware/controllers/system/timer/scheduler.h | 5 +++++ firmware/hw_layer/servo.cpp | 2 +- unit_tests/tests/test_signal_executor.cpp | 6 +++--- 8 files changed, 24 insertions(+), 19 deletions(-) diff --git a/firmware/controllers/engine_cycle/aux_valves.cpp b/firmware/controllers/engine_cycle/aux_valves.cpp index 868cb8b029..615cc9761e 100644 --- a/firmware/controllers/engine_cycle/aux_valves.cpp +++ b/firmware/controllers/engine_cycle/aux_valves.cpp @@ -34,7 +34,7 @@ void plainPinTurnOn(AuxActor *current) { scheduleOrQueue(¤t->open, TRIGGER_EVENT_UNDEFINED, current->extra + engine->engineState.auxValveStart, - { (schfunc_t)plainPinTurnOn, current } + { plainPinTurnOn, current } PASS_ENGINE_PARAMETER_SUFFIX ); @@ -45,7 +45,7 @@ void plainPinTurnOn(AuxActor *current) { scheduleOrQueue(¤t->close, TRIGGER_EVENT_UNDEFINED, current->extra + engine->engineState.auxValveEnd, - { (schfunc_t)plainPinTurnOff, output } + { plainPinTurnOff, output } PASS_ENGINE_PARAMETER_SUFFIX ); } @@ -103,12 +103,12 @@ static void auxValveTriggerCallback(trigger_event_e ckpSignalType, fixAngle(onTime, "onTime", CUSTOM_ERR_6556); scheduleByAngle(onEvent, onTime, - (schfunc_t) &plainPinTurnOn, output PASS_ENGINE_PARAMETER_SUFFIX); + &plainPinTurnOn, output PASS_ENGINE_PARAMETER_SUFFIX); angle_t offTime = extra + engine->engineState.auxValveEnd; fixAngle(offTime, "offTime", CUSTOM_ERR_6557); scheduleByAngle(offEvent, offTime, - (schfunc_t) &plainPinTurnOff, output PASS_ENGINE_PARAMETER_SUFFIX); + &plainPinTurnOff, output PASS_ENGINE_PARAMETER_SUFFIX); if (isOverlap) { enginePins.debugTriggerSync.setValue(0); } @@ -149,7 +149,7 @@ void initAuxValves(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX) { scheduleOrQueue(&actor->open, TRIGGER_EVENT_UNDEFINED, actor->extra + engine->engineState.auxValveStart, - { (schfunc_t)plainPinTurnOn, actor } + { plainPinTurnOn, actor } PASS_ENGINE_PARAMETER_SUFFIX ); } diff --git a/firmware/controllers/engine_cycle/main_trigger_callback.cpp b/firmware/controllers/engine_cycle/main_trigger_callback.cpp index f77849d5c7..a7981cdb70 100644 --- a/firmware/controllers/engine_cycle/main_trigger_callback.cpp +++ b/firmware/controllers/engine_cycle/main_trigger_callback.cpp @@ -192,7 +192,7 @@ static void sescheduleByTimestamp(scheduling_s *scheduling, efitimeus_t time, ac // scheduleMsg(&sharedLogger, "schX %s %x %d", prefix, scheduling, time); // scheduleMsg(&sharedLogger, "schX %s", param->name); - const char *direction = callback == (schfunc_t) &seTurnPinHigh ? "up" : "down"; + const char *direction = callback == &seTurnPinHigh ? "up" : "down"; printf("seScheduleByTime %s %s %d sch=%d\r\n", direction, param->name, (int)time, (int)scheduling); #endif /* FUEL_MATH_EXTREME_LOGGING || EFI_UNIT_TEST */ @@ -278,9 +278,9 @@ static ALWAYS_INLINE void handleFuelInjectionEvent(int injEventIndex, InjectionE // todo: sequential need this logic as well, just do not forget to clear flag event->isScheduled = true; scheduling_s * sDown = &event->endOfInjectionEvent; - engine->executor.scheduleForLater(sUp, (int) injectionStartDelayUs, { (schfunc_t) &startSimultaniousInjection, engine }); + engine->executor.scheduleForLater(sUp, (int) injectionStartDelayUs, { &startSimultaniousInjection, engine }); engine->executor.scheduleForLater(sDown, (int) injectionStartDelayUs + durationUs, - { (schfunc_t) &endSimultaniousInjection, event }); + { &endSimultaniousInjection, event }); } else { #if EFI_UNIT_TEST @@ -327,10 +327,10 @@ static ALWAYS_INLINE void handleFuelInjectionEvent(int injEventIndex, InjectionE printf("please cancel %s %d %d\r\n", output->name, (int)getTimeNowUs(), output->overlappingCounter); #endif /* EFI_UNIT_TEST || EFI_SIMULATOR */ } else { - sescheduleByTimestamp(sUp, turnOnTime, { (schfunc_t) &seTurnPinHigh, event } PASS_ENGINE_PARAMETER_SUFFIX); + sescheduleByTimestamp(sUp, turnOnTime, { &seTurnPinHigh, event } PASS_ENGINE_PARAMETER_SUFFIX); } efitimeus_t turnOffTime = nowUs + (int) (injectionStartDelayUs + durationUs); - sescheduleByTimestamp(sDown, turnOffTime, { (schfunc_t) &seTurnPinLow, event } PASS_ENGINE_PARAMETER_SUFFIX); + sescheduleByTimestamp(sDown, turnOffTime, { &seTurnPinLow, event } PASS_ENGINE_PARAMETER_SUFFIX); } } @@ -561,7 +561,7 @@ void startPrimeInjectionPulse(DECLARE_ENGINE_PARAMETER_SIGNATURE) { if (pulseLength > 0) { startSimultaniousInjection(engine); efitimeus_t turnOffDelayUs = (efitimeus_t)efiRound(MS2US(pulseLength), 1.0f); - engine->executor.scheduleForLater(sDown, turnOffDelayUs, { (schfunc_t) &endSimultaniousInjectionOnlyTogglePins, engine }); + engine->executor.scheduleForLater(sDown, turnOffDelayUs, { &endSimultaniousInjectionOnlyTogglePins, engine }); } } #if EFI_PROD_CODE diff --git a/firmware/controllers/engine_cycle/rpm_calculator.cpp b/firmware/controllers/engine_cycle/rpm_calculator.cpp index f43e266cd6..3ae50b0eba 100644 --- a/firmware/controllers/engine_cycle/rpm_calculator.cpp +++ b/firmware/controllers/engine_cycle/rpm_calculator.cpp @@ -323,7 +323,7 @@ static void tdcMarkCallback(trigger_event_e ckpSignalType, // todo: use tooth event-based scheduling, not just time-based scheduling if (isValidRpm(rpm)) { scheduleByAngle(&tdcScheduler[revIndex2], tdcPosition(), - { (schfunc_t) onTdcCallback, engine } PASS_ENGINE_PARAMETER_SUFFIX); + { onTdcCallback, engine } PASS_ENGINE_PARAMETER_SUFFIX); } } } diff --git a/firmware/controllers/engine_cycle/spark_logic.cpp b/firmware/controllers/engine_cycle/spark_logic.cpp index 5437ac8cfc..4197bbce09 100644 --- a/firmware/controllers/engine_cycle/spark_logic.cpp +++ b/firmware/controllers/engine_cycle/spark_logic.cpp @@ -321,7 +321,7 @@ static ALWAYS_INLINE void handleSparkEvent(bool limitedSpark, uint32_t trgEventI * This way we make sure that coil dwell started while spark was enabled would fire and not burn * the coil. */ - engine->executor.scheduleForLater(sUp, chargeDelayUs, { (schfunc_t) &turnSparkPinHigh, iEvent }); + engine->executor.scheduleForLater(sUp, chargeDelayUs, { &turnSparkPinHigh, iEvent }); } /** * Spark event is often happening during a later trigger event timeframe @@ -332,7 +332,7 @@ static ALWAYS_INLINE void handleSparkEvent(bool limitedSpark, uint32_t trgEventI assertAngleRange(sparkAngle, "findAngle#a5", CUSTOM_ERR_6549); - bool scheduled = scheduleOrQueue(&iEvent->sparkEvent, trgEventIndex, sparkAngle, { (schfunc_t)fireSparkAndPrepareNextSchedule, iEvent } PASS_ENGINE_PARAMETER_SUFFIX); + bool scheduled = scheduleOrQueue(&iEvent->sparkEvent, trgEventIndex, sparkAngle, { fireSparkAndPrepareNextSchedule, iEvent } PASS_ENGINE_PARAMETER_SUFFIX); if (scheduled) { #if SPARK_EXTREME_LOGGING diff --git a/firmware/controllers/system/timer/pwm_generator_logic.cpp b/firmware/controllers/system/timer/pwm_generator_logic.cpp index 7397697ea7..45823fbb94 100644 --- a/firmware/controllers/system/timer/pwm_generator_logic.cpp +++ b/firmware/controllers/system/timer/pwm_generator_logic.cpp @@ -262,7 +262,7 @@ static void timerCallback(PwmConfig *state) { return; } - state->executor->scheduleByTimestamp(&state->scheduling, switchTimeUs, { (schfunc_t) timerCallback, state }); + state->executor->scheduleByTimestamp(&state->scheduling, switchTimeUs, { timerCallback, state }); state->dbgNestingLevel--; } diff --git a/firmware/controllers/system/timer/scheduler.h b/firmware/controllers/system/timer/scheduler.h index f9ba4b1837..c23a8e2b90 100644 --- a/firmware/controllers/system/timer/scheduler.h +++ b/firmware/controllers/system/timer/scheduler.h @@ -18,6 +18,11 @@ public: action_s(schfunc_t callback) : action_s(callback, nullptr) { } action_s(schfunc_t callback, void *param) : callback(callback), param(param) { } + // Allow any function that takes a single pointer parameter, so long as param is also of the same pointer type. + // This constructor means you shouldn't ever have to cast to schfunc_t on your own. + template + action_s(void (*callback)(TArg*), TArg* param) : callback((schfunc_t)callback), param(param) { } + void execute(); schfunc_t getCallback() const; void * getArgument() const; diff --git a/firmware/hw_layer/servo.cpp b/firmware/hw_layer/servo.cpp index d9bc04edfb..b8788226ca 100644 --- a/firmware/hw_layer/servo.cpp +++ b/firmware/hw_layer/servo.cpp @@ -61,7 +61,7 @@ static msg_t seThread(void *arg) { float durationMs = 0 + position * 0.02f; - engine->executor.scheduleForLater(&servoTurnSignalOff, (int)MS2US(durationMs), { (schfunc_t) &servoTachPinLow, pin }); + engine->executor.scheduleForLater(&servoTurnSignalOff, (int)MS2US(durationMs), { &servoTachPinLow, pin }); chThdSleepMilliseconds(19); diff --git a/unit_tests/tests/test_signal_executor.cpp b/unit_tests/tests/test_signal_executor.cpp index 9415e44449..f735b5d504 100644 --- a/unit_tests/tests/test_signal_executor.cpp +++ b/unit_tests/tests/test_signal_executor.cpp @@ -34,7 +34,7 @@ static void complexCallback(TestPwm *testPwm) { callbackCounter++; eq.insertTask(&testPwm->s, complexTestNow + testPwm->period, - { (schfunc_t) complexCallback, testPwm }); + { complexCallback, testPwm }); } static void testSignalExecutor2(void) { @@ -47,8 +47,8 @@ static void testSignalExecutor2(void) { complexTestNow = 0; callbackCounter = 0; - eq.insertTask(&p1.s, 0, { (schfunc_t) complexCallback, &p1 }); - eq.insertTask(&p2.s, 0, { (schfunc_t) complexCallback, &p2 }); + eq.insertTask(&p1.s, 0, { complexCallback, &p1 }); + eq.insertTask(&p2.s, 0, { complexCallback, &p2 }); eq.executeAll(complexTestNow); ASSERT_EQ( 2, callbackCounter) << "callbackCounter #1"; ASSERT_EQ(2, eq.size());