pw_chrono_freertos: Ensure timer periods are always > 0 ticks
Updates the FreeRTOS backend for the SystemTimer to always pass
a non-zero timer period to the native FreeRTOS API. Note that this
does not impact the resulting expiry deadline.
Also updates the facade unit tests to cover this corner case.
Tested: Validated on all known timer backends.
Change-Id: I94b313724cade0d886769e5aaedab44ceb5d846a
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/57020
Reviewed-by: Keir Mierle <keir@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
Pigweed-Auto-Submit: Ewout van Bekkum <ewout@google.com>
diff --git a/pw_chrono/system_timer_facade_test.cc b/pw_chrono/system_timer_facade_test.cc
index 88cfd26..d819e70 100644
--- a/pw_chrono/system_timer_facade_test.cc
+++ b/pw_chrono/system_timer_facade_test.cc
@@ -123,6 +123,11 @@
uut.expected_deadline = SystemClock::now() + kRoundedArbitraryShortDuration;
uut.timer().InvokeAt(uut.expected_deadline);
uut.callback_ran_notification.acquire();
+
+ // Ensure scheduling it in the past causes it to execute immediately.
+ uut.expected_deadline = SystemClock::now() - SystemClock::duration(1);
+ uut.timer().InvokeAt(uut.expected_deadline);
+ uut.callback_ran_notification.acquire();
}
TEST(SystemTimer, InvokeAfter) {
@@ -149,6 +154,11 @@
SystemClock::TimePointAfterAtLeast(kRoundedArbitraryShortDuration);
uut.timer().InvokeAfter(kRoundedArbitraryShortDuration);
uut.callback_ran_notification.acquire();
+
+ // Ensure scheduling it immediately works.
+ uut.expected_min_deadline = SystemClock::now();
+ uut.timer().InvokeAfter(SystemClock::duration(0));
+ uut.callback_ran_notification.acquire();
}
TEST(SystemTimer, CancelFromCallback) {
diff --git a/pw_chrono_freertos/system_timer.cc b/pw_chrono_freertos/system_timer.cc
index 0ab4d2b..e646f32 100644
--- a/pw_chrono_freertos/system_timer.cc
+++ b/pw_chrono_freertos/system_timer.cc
@@ -72,6 +72,8 @@
}
// We haven't met the deadline yet, reschedule as far out as possible.
+ // Note that this must be > SystemClock::duration::zero() based on the
+ // conditional above.
const SystemClock::duration period =
std::min(pw::chrono::freertos::kMaxTimeout, time_until_deadline);
PW_CHECK_UINT_EQ(
@@ -84,7 +86,9 @@
"Timer command queue overflowed");
}
-constexpr TickType_t kInvalidPeriod = 1;
+// FreeRTOS requires a timer to have a non-zero period.
+constexpr SystemClock::duration kMinTimerPeriod = SystemClock::duration(1);
+constexpr TickType_t kInvalidPeriod = kMinTimerPeriod.count();
constexpr UBaseType_t kOneShotMode = pdFALSE; // Do not use auto reload.
} // namespace
@@ -154,10 +158,8 @@
// clamped and it may be rescheduled internally.
const SystemClock::duration time_until_deadline =
timestamp - SystemClock::now();
- const SystemClock::duration period =
- std::clamp(SystemClock::duration::zero(),
- time_until_deadline,
- pw::chrono::freertos::kMaxTimeout);
+ const SystemClock::duration period = std::clamp(
+ kMinTimerPeriod, time_until_deadline, pw::chrono::freertos::kMaxTimeout);
PW_CHECK_UINT_EQ(
xTimerChangePeriod(