diff --git a/firmware/controllers/system/timer/event_queue.cpp b/firmware/controllers/system/timer/event_queue.cpp index ad59c547c0..cd2ec71ce4 100644 --- a/firmware/controllers/system/timer/event_queue.cpp +++ b/firmware/controllers/system/timer/event_queue.cpp @@ -82,8 +82,7 @@ bool EventQueue::insertTask(scheduling_s *scheduling, efitime_t timeX, action_s * This method is always invoked under a lock * @return Get the timestamp of the soonest pending action, skipping all the actions in the past */ -efitime_t EventQueue::getNextEventTime(efitime_t nowX) const { - +expected EventQueue::getNextEventTime(efitime_t nowX) const { if (head != NULL) { if (head->momentX <= nowX) { /** @@ -99,7 +98,8 @@ efitime_t EventQueue::getNextEventTime(efitime_t nowX) const { return head->momentX; } } - return EMPTY_QUEUE; + + return unexpected; } /** diff --git a/firmware/controllers/system/timer/event_queue.h b/firmware/controllers/system/timer/event_queue.h index 88c1c7657e..24d4a7298f 100644 --- a/firmware/controllers/system/timer/event_queue.h +++ b/firmware/controllers/system/timer/event_queue.h @@ -7,14 +7,10 @@ #include "scheduler.h" #include "utlist.h" +#include "expected.h" #pragma once -/** - * this is a large value which is expected to be larger than any real time - */ -#define EMPTY_QUEUE 0x0FFFFFFFFFFFFFFFLL - #define QUEUE_LENGTH_LIMIT 1000 // templates do not accept field names so we use a macro here @@ -58,7 +54,7 @@ public: int executeAll(efitime_t now); bool executeOne(efitime_t now); - efitime_t getNextEventTime(efitime_t nowUs) const; + expected getNextEventTime(efitime_t nowUs) const; void clear(void); int size(void) const; scheduling_s *getElementAtIndexForUnitText(int index); diff --git a/firmware/controllers/system/timer/single_timer_executor.cpp b/firmware/controllers/system/timer/single_timer_executor.cpp index e4fd913d9e..d324720fef 100644 --- a/firmware/controllers/system/timer/single_timer_executor.cpp +++ b/firmware/controllers/system/timer/single_timer_executor.cpp @@ -35,11 +35,6 @@ #include "engine.h" EXTERN_ENGINE; -/** - * these fields are global in order to facilitate debugging - */ -static efitime_t nextEventTimeNt = 0; - uint32_t hwSetTimerDuration; void globalTimerCallback() { @@ -154,12 +149,16 @@ void SingleTimerExecutor::scheduleTimerCallback() { * Let's grab fresh time value */ efitick_t nowNt = getTimeNowNt(); - nextEventTimeNt = queue.getNextEventTime(nowNt); - efiAssertVoid(CUSTOM_ERR_6625, nextEventTimeNt > nowNt, "setTimer constraint"); - if (nextEventTimeNt == EMPTY_QUEUE) - return; // no pending events in the queue + expected nextEventTimeNt = queue.getNextEventTime(nowNt); + + if (!nextEventTimeNt) { + return; // no pending events in the queue + } + + efiAssertVoid(CUSTOM_ERR_6625, nextEventTimeNt.Value > nowNt, "setTimer constraint"); + + int32_t hwAlarmTime = NT2US((int32_t)nextEventTimeNt.Value - (int32_t)nowNt); - int32_t hwAlarmTime = NT2US((int32_t)nextEventTimeNt - (int32_t)nowNt); setHardwareUsTimer(hwAlarmTime == 0 ? 1 : hwAlarmTime); } diff --git a/firmware/hw_layer/microsecond_timer.cpp b/firmware/hw_layer/microsecond_timer.cpp index 26d90ffcb5..8a4a5bfc54 100644 --- a/firmware/hw_layer/microsecond_timer.cpp +++ b/firmware/hw_layer/microsecond_timer.cpp @@ -65,7 +65,9 @@ static volatile bool hwStarted = false; */ void setHardwareUsTimer(int32_t deltaTimeUs) { efiAssertVoid(OBD_PCM_Processor_Fault, hwStarted, "HW.started"); + setHwTimerCounter++; + /** * #259 BUG error: not positive deltaTimeUs * Once in a while we night get an interrupt where we do not expect it @@ -74,8 +76,12 @@ void setHardwareUsTimer(int32_t deltaTimeUs) { timerFreezeCounter++; warning(CUSTOM_OBD_LOCAL_FREEZE, "local freeze cnt=%d", timerFreezeCounter); } - if (deltaTimeUs < 2) - deltaTimeUs = 2; // for some reason '1' does not really work + + // We need the timer to fire after we return - 1 doesn't work as it may actually schedule in the past + if (deltaTimeUs < 2) { + deltaTimeUs = 2; + } + efiAssertVoid(CUSTOM_DELTA_NOT_POSITIVE, deltaTimeUs > 0, "not positive deltaTimeUs"); if (deltaTimeUs >= TOO_FAR_INTO_FUTURE_US) { // we are trying to set callback for too far into the future. This does not look right at all @@ -83,16 +89,21 @@ void setHardwareUsTimer(int32_t deltaTimeUs) { return; } + // If already set, reset the timer if (GPTDEVICE.state == GPT_ONESHOT) { gptStopTimerI(&GPTDEVICE); } + if (GPTDEVICE.state != GPT_READY) { firmwareError(CUSTOM_HW_TIMER, "HW timer state %d/%d", GPTDEVICE.state, setHwTimerCounter); return; } + if (hasFirmwareError()) { return; } + + // Start the timer gptStartOneShotI(&GPTDEVICE, deltaTimeUs); lastSetTimerTimeNt = getTimeNowNt(); @@ -103,8 +114,7 @@ void setHardwareUsTimer(int32_t deltaTimeUs) { void globalTimerCallback(); -static void hwTimerCallback(GPTDriver *gptp) { - (void)gptp; +static void hwTimerCallback(GPTDriver*) { timerCallbackCounter++; isTimerPending = false; @@ -148,8 +158,7 @@ static constexpr GPTConfig gpt5cfg = { 1000000, /* 1 MHz timer clock.*/ static scheduling_s watchDogBuddy; -static void watchDogBuddyCallback(void *arg) { - (void)arg; +static void watchDogBuddyCallback(void*) { /** * the purpose of this periodic activity is to make watchdogControllerInstance * watchdog happy by ensuring that we have scheduler activity even in case of very broken configuration @@ -161,9 +170,7 @@ static void watchDogBuddyCallback(void *arg) { static volatile bool testSchedulingHappened = false; static efitimems_t testSchedulingStart; -static void timerValidationCallback(void *arg) { - (void)arg; - +static void timerValidationCallback(void*) { testSchedulingHappened = true; efitimems_t actualTimeSinceScheduling = (currentTimeMillis() - testSchedulingStart); diff --git a/unit_tests/tests/test_signal_executor.cpp b/unit_tests/tests/test_signal_executor.cpp index 212f62ecba..565c2abb85 100644 --- a/unit_tests/tests/test_signal_executor.cpp +++ b/unit_tests/tests/test_signal_executor.cpp @@ -6,7 +6,6 @@ */ #include "global.h" -#include #include "test_signal_executor.h" #include "io_pins.h" @@ -91,7 +90,7 @@ TEST(misc, testSignalExecutor) { print("*************************************** testSignalExecutor\r\n"); EventQueue eq; - ASSERT_EQ(EMPTY_QUEUE, eq.getNextEventTime(0)); + ASSERT_EQ(eq.getNextEventTime(0), unexpected); scheduling_s s1; scheduling_s s2; scheduling_s s3; @@ -131,7 +130,7 @@ TEST(misc, testSignalExecutor) { callbackCounter = 0; eq.insertTask(&s1, 10, callback); - ASSERT_EQ(10, eq.getNextEventTime(0)); + ASSERT_EQ(10, eq.getNextEventTime(0).value_or(-1)); eq.executeAll(1); ASSERT_EQ( 0, callbackCounter) << "callbacks not expected"; @@ -139,14 +138,14 @@ TEST(misc, testSignalExecutor) { eq.executeAll(11); ASSERT_EQ(1, callbackCounter); - ASSERT_EQ(EMPTY_QUEUE, eq.getNextEventTime(0)); + ASSERT_EQ(eq.getNextEventTime(0), unexpected); eq.insertTask(&s1, 10, callback); eq.insertTask(&s2, 13, callback); - ASSERT_EQ(10, eq.getNextEventTime(0)); + ASSERT_EQ(10, eq.getNextEventTime(0).value_or(-1)); eq.executeAll(1); - ASSERT_EQ(10, eq.getNextEventTime(0)); + ASSERT_EQ(10, eq.getNextEventTime(0).value_or(-1)); eq.executeAll(100); ASSERT_EQ(0, eq.size());