From 5250b177c29de118e2b2ec1fa92681d0e64daa0b Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Sat, 10 Dec 2022 14:55:51 -0800 Subject: [PATCH] Allow scheduling_s to come from a pool (#4841) * injection events use scheduling pool * knock and prime * bench test * dropped this: * * extract action before execute * comment * init allocated timers * metrics * dropped this: , * guard * injection events use scheduling pool * knock and prime * bench test * dropped this: * * extract action before execute * comment * init allocated timers * metrics * dropped this: , * guard Co-authored-by: Matthew Kennedy --- firmware/console/binary/output_channels.txt | 4 +- firmware/controllers/bench_test.cpp | 7 +-- .../controllers/engine_cycle/fuel_schedule.h | 3 -- .../engine_cycle/knock_controller.cpp | 4 +- .../engine_cycle/main_trigger_callback.cpp | 6 +-- .../engine_cycle/prime_injection.cpp | 4 +- .../engine_cycle/prime_injection.h | 3 -- .../controllers/system/timer/event_queue.cpp | 48 +++++++++++++++++++ .../controllers/system/timer/event_queue.h | 8 +++- firmware/controllers/system/timer/scheduler.h | 1 + .../system/timer/signal_executor_sleep.cpp | 36 ++++++++++++-- .../trigger/test_injection_scheduling.cpp | 12 ++--- 12 files changed, 104 insertions(+), 32 deletions(-) diff --git a/firmware/console/binary/output_channels.txt b/firmware/console/binary/output_channels.txt index 5c541ca335..a9bcdd1afc 100644 --- a/firmware/console/binary/output_channels.txt +++ b/firmware/console/binary/output_channels.txt @@ -365,5 +365,7 @@ float mapFast uint16_t autoscale rawMaf2;;"V",{1/@@PACK_MULT_VOLTAGE@@}, 0, 0, 5, 3 uint16_t autoscale mafMeasured2;@@GAUGE_NAME_AIR_FLOW_MEASURED_2@@;"kg/h",{1/@@PACK_MULT_MASS_FLOW@@}, 0, 0, 0, 1 - uint8_t[138 iterate] unusedAtTheEnd;;"",1, 0, 0, 0, 0 + uint16_t schedulingUsedCount;;"",1,0,0,0,0 + + uint8_t[136 iterate] unusedAtTheEnd;;"",1, 0, 0, 0, 0 end_struct diff --git a/firmware/controllers/bench_test.cpp b/firmware/controllers/bench_test.cpp index 9c1ffd10c7..2dfe06def8 100644 --- a/firmware/controllers/bench_test.cpp +++ b/firmware/controllers/bench_test.cpp @@ -57,9 +57,6 @@ bool isRunningBenchTest(void) { return isRunningBench; } -static scheduling_s benchSchedStart; -static scheduling_s benchSchedEnd; - static void benchOn(OutputPin* output) { output->setValue(true); } @@ -105,8 +102,8 @@ static void runBench(brain_pin_e brainPin, OutputPin *output, float startDelayMs efitick_t endTime = startTime + US2NT(onTimeUs); // Schedule both events - engine->executor.scheduleByTimestampNt("bstart", &benchSchedStart, startTime, {benchOn, output}); - engine->executor.scheduleByTimestampNt("bend", &benchSchedEnd, endTime, {benchOff, output}); + engine->executor.scheduleByTimestampNt("bstart", nullptr, startTime, {benchOn, output}); + engine->executor.scheduleByTimestampNt("bend", nullptr, endTime, {benchOff, output}); // Wait one full cycle time for the event + delay to happen chThdSleepMicroseconds(onTimeUs + offTimeUs); diff --git a/firmware/controllers/engine_cycle/fuel_schedule.h b/firmware/controllers/engine_cycle/fuel_schedule.h index e13e4f6bb1..0767618f1c 100644 --- a/firmware/controllers/engine_cycle/fuel_schedule.h +++ b/firmware/controllers/engine_cycle/fuel_schedule.h @@ -38,9 +38,6 @@ public: float injectionStartAngle = 0; - scheduling_s signalTimerUp; - scheduling_s endOfInjectionEvent; - /** * we need atomic flag so that we do not schedule a new pair of up/down before previous down was executed. * diff --git a/firmware/controllers/engine_cycle/knock_controller.cpp b/firmware/controllers/engine_cycle/knock_controller.cpp index e3f38878bd..d56d144edd 100644 --- a/firmware/controllers/engine_cycle/knock_controller.cpp +++ b/firmware/controllers/engine_cycle/knock_controller.cpp @@ -155,13 +155,11 @@ static void startKnockSampling(Engine* engine) { onStartKnockSampling(cylinderNumberCopy, samplingSeconds, channel); } -static scheduling_s startSampling; - void Engine::onSparkFireKnockSense(uint8_t cylinderNumber, efitick_t nowNt) { cylinderNumberCopy = cylinderNumber; #if EFI_HIP_9011 || EFI_SOFTWARE_KNOCK - scheduleByAngle(&startSampling, nowNt, + scheduleByAngle(nullptr, nowNt, /*angle*/engineConfiguration->knockDetectionWindowStart, { startKnockSampling, engine }); #else UNUSED(nowNt); diff --git a/firmware/controllers/engine_cycle/main_trigger_callback.cpp b/firmware/controllers/engine_cycle/main_trigger_callback.cpp index 0c964b2c7d..1bb2060126 100644 --- a/firmware/controllers/engine_cycle/main_trigger_callback.cpp +++ b/firmware/controllers/engine_cycle/main_trigger_callback.cpp @@ -155,7 +155,7 @@ void InjectionEvent::onTriggerTooth(int rpm, efitick_t nowNt, float currentPhase } #endif /*EFI_PRINTF_FUEL_DETAILS */ - if (isScheduled) { +if (isScheduled) { #if EFI_PRINTF_FUEL_DETAILS if (printFuelDebug) { InjectorOutputPin *output = outputs[0]; @@ -183,9 +183,9 @@ void InjectionEvent::onTriggerTooth(int rpm, efitick_t nowNt, float currentPhase angleFromNow += getEngineState()->engineCycle; } - efitick_t startTime = scheduleByAngle(&signalTimerUp, nowNt, angleFromNow, startAction); + efitick_t startTime = scheduleByAngle(nullptr, nowNt, angleFromNow, startAction); efitick_t turnOffTime = startTime + US2NT((int)durationUs); - getExecutorInterface()->scheduleByTimestampNt("inj", &endOfInjectionEvent, turnOffTime, endAction); + getExecutorInterface()->scheduleByTimestampNt("inj", nullptr, turnOffTime, endAction); #if EFI_UNIT_TEST printf("scheduling injection angle=%.2f/delay=%.2f injectionDuration=%.2f\r\n", angleFromNow, NT2US(startTime - nowNt), injectionDuration); diff --git a/firmware/controllers/engine_cycle/prime_injection.cpp b/firmware/controllers/engine_cycle/prime_injection.cpp index 828a07c7e0..5e222bae82 100644 --- a/firmware/controllers/engine_cycle/prime_injection.cpp +++ b/firmware/controllers/engine_cycle/prime_injection.cpp @@ -60,7 +60,7 @@ void PrimeController::onIgnitionStateChanged(bool ignitionOn) { auto primeDelayMs = engineConfiguration->primingDelay * 1000; auto startTime = getTimeNowNt() + MS2NT(primeDelayMs); - getExecutorInterface()->scheduleByTimestampNt("prime", &m_start, startTime, { PrimeController::onPrimeStartAdapter, this }); + getExecutorInterface()->scheduleByTimestampNt("prime", nullptr, startTime, { PrimeController::onPrimeStartAdapter, this }); } else { efiPrintf("Skipped priming pulse since ignSwitchCounter = %d", ignSwitchCounter); } @@ -101,7 +101,7 @@ void PrimeController::onPrimeStart() { // Open all injectors, schedule closing later m_isPriming = true; startSimultaneousInjection(); - getExecutorInterface()->scheduleByTimestampNt("prime", &m_end, endTime, { onPrimeEndAdapter, this }); + getExecutorInterface()->scheduleByTimestampNt("prime", nullptr, endTime, { onPrimeEndAdapter, this }); } void PrimeController::onPrimeEnd() { diff --git a/firmware/controllers/engine_cycle/prime_injection.h b/firmware/controllers/engine_cycle/prime_injection.h index e5f584373c..1f0e53e6e0 100644 --- a/firmware/controllers/engine_cycle/prime_injection.h +++ b/firmware/controllers/engine_cycle/prime_injection.h @@ -24,9 +24,6 @@ public: } private: - scheduling_s m_start; - scheduling_s m_end; - bool m_isPriming = false; static void onPrimeStartAdapter(PrimeController* instance) { diff --git a/firmware/controllers/system/timer/event_queue.cpp b/firmware/controllers/system/timer/event_queue.cpp index ce0e647cf2..c7ee7477e1 100644 --- a/firmware/controllers/system/timer/event_queue.cpp +++ b/firmware/controllers/system/timer/event_queue.cpp @@ -21,6 +21,40 @@ extern int timeNowUs; extern bool verboseMode; #endif /* EFI_UNIT_TEST */ +EventQueue::EventQueue(efitick_t lateDelay) + : lateDelay(lateDelay) +{ + for (size_t i = 0; i < efi::size(m_pool); i++) { + tryReturnScheduling(&m_pool[i]); + } +} + +scheduling_s* EventQueue::getFreeScheduling() { + auto retVal = m_freelist; + + if (retVal) { + m_freelist = retVal->nextScheduling_s; + retVal->nextScheduling_s = nullptr; + +#if EFI_PROD_CODE + getTunerStudioOutputChannels()->schedulingUsedCount++; +#endif + } + + return retVal; +} + +void EventQueue::tryReturnScheduling(scheduling_s* sched) { + // Only return this scheduling to the free list if it's from the correct pool + if (sched >= &m_pool[0] && sched <= &m_pool[efi::size(m_pool) - 1]) { + sched->nextScheduling_s = m_freelist; + m_freelist = sched; + +#if EFI_PROD_CODE + getTunerStudioOutputChannels()->schedulingUsedCount--; +#endif + } +} /** * @return true if inserted into the head of the list @@ -28,6 +62,17 @@ extern bool verboseMode; bool EventQueue::insertTask(scheduling_s *scheduling, efitick_t timeX, action_s action) { ScopePerf perf(PE::EventQueueInsertTask); + if (!scheduling) { + scheduling = getFreeScheduling(); + + // If still null, the free list is empty and all schedulings in the pool have been expended. + if (!scheduling) { + // TODO: should we warn or error here? + + return false; + } + } + #if EFI_UNIT_TEST assertListIsSorted(); #endif /* EFI_UNIT_TEST */ @@ -210,6 +255,9 @@ bool EventQueue::executeOne(efitick_t now) { auto action = current->action; current->action = {}; + tryReturnScheduling(current); + current = nullptr; + #if EFI_UNIT_TEST printf("QUEUE: execute current=%d param=%d\r\n", (uintptr_t)current, (uintptr_t)action.getArgument()); #endif diff --git a/firmware/controllers/system/timer/event_queue.h b/firmware/controllers/system/timer/event_queue.h index eabff2b4d5..6017a2416c 100644 --- a/firmware/controllers/system/timer/event_queue.h +++ b/firmware/controllers/system/timer/event_queue.h @@ -44,7 +44,7 @@ public: // See comment in EventQueue::executeAll for info about lateDelay - it sets the // time gap between events for which we will wait instead of rescheduling the next // event in a group of events near one another. - EventQueue(efitick_t lateDelay = 0) : lateDelay(lateDelay) {} + explicit EventQueue(efitick_t lateDelay = 0); /** * O(size) - linear search in sorted linked list @@ -61,11 +61,17 @@ public: scheduling_s *getElementAtIndexForUnitText(int index); scheduling_s * getHead(); void assertListIsSorted() const; + + scheduling_s* getFreeScheduling(); + void tryReturnScheduling(scheduling_s* sched); private: /** * this list is sorted */ scheduling_s *head = nullptr; const efitick_t lateDelay; + + scheduling_s* m_freelist = nullptr; + scheduling_s m_pool[64]; }; diff --git a/firmware/controllers/system/timer/scheduler.h b/firmware/controllers/system/timer/scheduler.h index 47b35abe04..0a16b99d22 100644 --- a/firmware/controllers/system/timer/scheduler.h +++ b/firmware/controllers/system/timer/scheduler.h @@ -26,6 +26,7 @@ public: schfunc_t getCallback() const; void * getArgument() const; + // Actions with a callback set are truthy, all others are falsy operator bool() const { return callback != nullptr; } diff --git a/firmware/controllers/system/timer/signal_executor_sleep.cpp b/firmware/controllers/system/timer/signal_executor_sleep.cpp index 0c42ba30a9..d6b0349660 100644 --- a/firmware/controllers/system/timer/signal_executor_sleep.cpp +++ b/firmware/controllers/system/timer/signal_executor_sleep.cpp @@ -33,6 +33,12 @@ bool printSchedulerDebug = true; #if EFI_SIGNAL_EXECUTOR_SLEEP +struct CallbackContext +{ + scheduling_s* scheduling = nullptr; + bool shouldFree = false; +}; + void SleepExecutor::scheduleByTimestamp(const char *msg, scheduling_s *scheduling, efitimeus_t timeUs, action_s action) { scheduleForLater(msg, scheduling, timeUs - getTimeNowUs(), action); } @@ -41,18 +47,30 @@ void SleepExecutor::scheduleByTimestampNt(const char *msg, scheduling_s* schedul scheduleByTimestamp(msg, scheduling, NT2US(timeNt), action); } -static void timerCallback(scheduling_s *scheduling) { +static void timerCallback(CallbackContext* ctx) { #if EFI_PRINTF_FUEL_DETAILS if (printSchedulerDebug) { - if (scheduling->action.getCallback() == (schfunc_t)&turnInjectionPinLow) { - printf("executing cb=turnInjectionPinLow p=%d sch=%d now=%d\r\n", (int)scheduling->action.getArgument(), (int)scheduling, + if (ctx->scheduling->action.getCallback() == (schfunc_t)&turnInjectionPinLow) { + printf("executing cb=turnInjectionPinLow p=%d sch=%d now=%d\r\n", (int)ctx->scheduling->action.getArgument(), (int)scheduling, (int)getTimeNowUs()); } else { // printf("exec cb=%d p=%d\r\n", (int)scheduling->callback, (int)scheduling->param); } } #endif // EFI_PRINTF_FUEL_DETAILS - scheduling->action.execute(); + + // Grab the action but clear it in the event so we can reschedule from the action's execution + action_s action = ctx->scheduling->action; + ctx->scheduling->action = {}; + + // Clean up any memory we allocated + if (ctx->shouldFree) { + delete ctx->scheduling; + } + delete ctx; + + // Lastly, actually execute the action + action.execute(); } static void doScheduleForLater(scheduling_s *scheduling, int delayUs, action_s action) { @@ -67,6 +85,14 @@ static void doScheduleForLater(scheduling_s *scheduling, int delayUs, action_s a chibios_rt::CriticalSectionLocker csl; + auto ctx = new CallbackContext; + if (!scheduling) { + scheduling = new scheduling_s; + chVTObjectInit(&scheduling->timer); + ctx->shouldFree = true; + } + ctx->scheduling = scheduling; + scheduling->action = action; int isArmed = chVTIsArmedI(&scheduling->timer); if (isArmed) { @@ -84,7 +110,7 @@ static void doScheduleForLater(scheduling_s *scheduling, int delayUs, action_s a } #endif /* EFI_SIMULATOR */ - chVTSetI(&scheduling->timer, delaySt, (vtfunc_t)timerCallback, scheduling); + chVTSetI(&scheduling->timer, delaySt, (vtfunc_t)timerCallback, ctx); } void SleepExecutor::scheduleForLater(const char *msg, scheduling_s *scheduling, int delayUs, action_s action) { diff --git a/unit_tests/tests/trigger/test_injection_scheduling.cpp b/unit_tests/tests/trigger/test_injection_scheduling.cpp index 26228e9c51..68158e642b 100644 --- a/unit_tests/tests/trigger/test_injection_scheduling.cpp +++ b/unit_tests/tests/trigger/test_injection_scheduling.cpp @@ -33,9 +33,9 @@ TEST(injectionScheduling, InjectionIsScheduled) { // rising edge 5 degrees from now float nt5deg = USF2NT(engine->rpmCalculator.oneDegreeUs * 5); efitick_t startTime = nowNt + nt5deg; - EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), &event.signalTimerUp, startTime, _)); + EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), _, startTime, _)); // falling edge 20ms later - EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), &event.endOfInjectionEvent, startTime + MS2NT(20), _)); + EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), _, startTime + MS2NT(20), _)); } @@ -73,9 +73,9 @@ TEST(injectionScheduling, InjectionIsScheduledBeforeWraparound) { // rising edge 5 degrees from now float nt5deg = USF2NT(engine->rpmCalculator.oneDegreeUs * 5); efitick_t startTime = nowNt + nt5deg; - EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), &event.signalTimerUp, startTime, _)); + EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), _, startTime, _)); // falling edge 20ms later - EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), &event.endOfInjectionEvent, startTime + MS2NT(20), _)); + EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), _, startTime + MS2NT(20), _)); } // Event scheduled at 715 degrees @@ -112,9 +112,9 @@ TEST(injectionScheduling, InjectionIsScheduledAfterWraparound) { // rising edge 15 degrees from now float nt5deg = USF2NT(engine->rpmCalculator.oneDegreeUs * 15); efitick_t startTime = nowNt + nt5deg; - EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), &event.signalTimerUp, startTime, _)); + EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), _, startTime, _)); // falling edge 20ms later - EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), &event.endOfInjectionEvent, startTime + MS2NT(20), _)); + EXPECT_CALL(mockExec, scheduleByTimestampNt(testing::NotNull(), _, startTime + MS2NT(20), _)); } // Event scheduled at 5 degrees