diff --git a/util/src/timer.cpp b/util/src/timer.cpp index 9d3284c..982432a 100644 --- a/util/src/timer.cpp +++ b/util/src/timer.cpp @@ -37,18 +37,28 @@ bool Timer::hasElapsedUs(float const microseconds) const { return false; } - // If larger than 32 bits, timer has certainly expired - if (delta >= UINT32_MAX) { - return true; - } - constexpr float max_32_bit_fit_float{ 4294967295.f }; - if (microseconds >= max_32_bit_fit_float) { + // The fast path below truncates both sides to 32 bits, so it is only valid when the timeout + // fits in 32 bits AS TICKS - and US_TO_NT_MULTIPLIER can be as large as 168, shrinking the + // 32-bit tick horizon to as little as 25 seconds. Longer timeouts compare the full 64-bit + // delta, in double so the tick conversion keeps precision. This check must come before the + // "delta larger than 32 bits" shortcut: sitting after it, long timeouts were reported as + // elapsed the moment the delta crossed the horizon. + if (microseconds >= max_32_bit_fit_float / US_TO_NT_MULTIPLIER) { auto const ntDouble{ static_cast(microseconds) * US_TO_NT_MULTIPLIER }; + if (ntDouble >= static_cast(INT64_MAX)) { + // absurdly long timeout: the int64 cast below would be UB, and it cannot have elapsed + return false; + } return delta > static_cast(ntDouble); } + // The timeout fits in 32 bits of ticks, so a delta beyond 32 bits has certainly expired + if (delta >= UINT32_MAX) { + return true; + } + return static_cast(delta) > static_cast(USF2NT(microseconds)); } diff --git a/util/test/test_timer.cpp b/util/test/test_timer.cpp index 3befe8d..4123e04 100644 --- a/util/test/test_timer.cpp +++ b/util/test/test_timer.cpp @@ -33,3 +33,50 @@ TEST(util, timerResetStampedInTheFuture) { setTimeNowNt(0); } + +/** + * US_TO_NT_MULTIPLIER is 100 in this test build, so 32 bits of ticks is just under 43 seconds. + * On real ports the horizon ranges from ~25 seconds (kinetis/cypress, multiplier 168) to + * ~1073 seconds (stm32, multiplier 4). Timeouts longer than the horizon used to be reported + * as elapsed the moment the delta crossed it, because the "delta larger than 32 bits" shortcut + * ran before the long-timeout path. + */ +TEST(util, timerTimeoutLongerThan32BitsOfTicks) { + Timer timer; + timer.reset(0); + + // well inside the fast path: nothing surprising + setTimeNowNt(USF2NT((efitick_t)20'000'000)); // 20 seconds + EXPECT_FALSE(timer.hasElapsedSec(60)); + + // past the 32-bit tick horizon but before the requested timeout: NOT elapsed + setTimeNowNt(USF2NT((efitick_t)50'000'000)); // 50 seconds + EXPECT_FALSE(timer.hasElapsedSec(60)); + + setTimeNowNt(USF2NT((efitick_t)61'000'000)); // 61 seconds + EXPECT_TRUE(timer.hasElapsedSec(60)); + + setTimeNowNt(0); +} + +TEST(util, timerHourLongTimeoutStaysExact) { + Timer timer; + timer.reset(0); + + setTimeNowNt(USF2NT((efitick_t)3'599'000'000)); // 3599 seconds + EXPECT_FALSE(timer.hasElapsedSec(3600)); + + setTimeNowNt(USF2NT((efitick_t)3'601'000'000)); // 3601 seconds + EXPECT_TRUE(timer.hasElapsedSec(3600)); + + setTimeNowNt(0); +} + +TEST(util, brandNewTimerHasElapsedEvenForLongTimeouts) { + // "Brand new instances have most recent reset time far in the past" - that promise from the + // header must survive the long-timeout path too + Timer timer; + + setTimeNowNt(0); + EXPECT_TRUE(timer.hasElapsedSec(3600)); +}