aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--examples/teensy4_blinky/Cargo.lock2
-rw-r--r--rtic-monotonics/CHANGELOG.md4
-rw-r--r--rtic-monotonics/Cargo.toml4
-rw-r--r--rtic-monotonics/src/imxrt.rs24
-rw-r--r--rtic-monotonics/src/nrf/rtc.rs23
-rw-r--r--rtic-monotonics/src/nrf/timer.rs24
-rw-r--r--rtic-monotonics/src/rp2040.rs22
-rw-r--r--rtic-monotonics/src/stm32.rs24
-rw-r--r--rtic-monotonics/src/systick.rs33
-rw-r--r--rtic-sync/Cargo.toml6
-rw-r--r--rtic-sync/src/arbiter.rs8
-rw-r--r--rtic-time/CHANGELOG.md1
-rw-r--r--rtic-time/Cargo.toml4
-rw-r--r--rtic-time/src/lib.rs22
-rw-r--r--rtic-time/src/monotonic.rs153
-rw-r--r--rtic-time/tests/delay_precision_subtick.rs313
-rw-r--r--rtic-time/tests/timer_queue.rs18
-rw-r--r--rtic/CHANGELOG.md4
-rw-r--r--rtic/ci/expected/async-timeout.run10
19 files changed, 559 insertions, 140 deletions
diff --git a/examples/teensy4_blinky/Cargo.lock b/examples/teensy4_blinky/Cargo.lock
index daec54f7..408bfe7d 100644
--- a/examples/teensy4_blinky/Cargo.lock
+++ b/examples/teensy4_blinky/Cargo.lock
@@ -440,7 +440,7 @@ dependencies = [
[[package]]
name = "rtic-monotonics"
-version = "1.2.1"
+version = "1.3.0"
dependencies = [
"atomic-polyfill",
"cfg-if",
diff --git a/rtic-monotonics/CHANGELOG.md b/rtic-monotonics/CHANGELOG.md
index d030bb02..4dea3fc5 100644
--- a/rtic-monotonics/CHANGELOG.md
+++ b/rtic-monotonics/CHANGELOG.md
@@ -7,6 +7,10 @@ For each category, *Added*, *Changed*, *Fixed* add new entries at the top!
## Unreleased
+### Fixed
+
+- **Soundness fix:** Monotonics did not wait long enough in `Duration` based delays.
+
## v1.3.0 - 2023-11-08
### Added
diff --git a/rtic-monotonics/Cargo.toml b/rtic-monotonics/Cargo.toml
index 441e772f..4d9dc0b8 100644
--- a/rtic-monotonics/Cargo.toml
+++ b/rtic-monotonics/Cargo.toml
@@ -28,8 +28,8 @@ rustdoc-flags = ["--cfg", "docsrs"]
[dependencies]
rtic-time = { version = "1.0.0", path = "../rtic-time" }
-embedded-hal = { version = "1.0.0-rc.1" }
-embedded-hal-async = { version = "1.0.0-rc.1", optional = true }
+embedded-hal = { version = "1.0.0-rc.2" }
+embedded-hal-async = { version = "1.0.0-rc.2", optional = true }
fugit = { version = "0.3.6" }
atomic-polyfill = "1"
cfg-if = "1.0.0"
diff --git a/rtic-monotonics/src/imxrt.rs b/rtic-monotonics/src/imxrt.rs
index 39448291..97ba73f8 100644
--- a/rtic-monotonics/src/imxrt.rs
+++ b/rtic-monotonics/src/imxrt.rs
@@ -31,7 +31,7 @@
use crate::{Monotonic, TimeoutError, TimerQueue};
use atomic_polyfill::{compiler_fence, AtomicU32, Ordering};
-pub use fugit::{self, ExtU64};
+pub use fugit::{self, ExtU64, ExtU64Ceil};
use imxrt_ral as ral;
@@ -216,31 +216,17 @@ macro_rules! make_timer {
}
}
- #[cfg(feature = "embedded-hal-async")]
- impl embedded_hal_async::delay::DelayUs for $mono_name {
- #[inline]
- async fn delay_us(&mut self, us: u32) {
- Self::delay((us as u64).micros()).await;
- }
-
- #[inline]
- async fn delay_ms(&mut self, ms: u32) {
- Self::delay((ms as u64).millis()).await;
- }
- }
+ rtic_time::embedded_hal_delay_impl_fugit64!($mono_name);
- impl embedded_hal::delay::DelayUs for $mono_name {
- fn delay_us(&mut self, us: u32) {
- let done = Self::now() + (us as u64).micros();
- while Self::now() < done {}
- }
- }
+ #[cfg(feature = "embedded-hal-async")]
+ rtic_time::embedded_hal_async_delay_impl_fugit64!($mono_name);
impl Monotonic for $mono_name {
type Instant = fugit::TimerInstantU64<TIMER_HZ>;
type Duration = fugit::TimerDurationU64<TIMER_HZ>;
const ZERO: Self::Instant = Self::Instant::from_ticks(0);
+ const TICK_PERIOD: Self::Duration = Self::Duration::from_ticks(1);
fn now() -> Self::Instant {
let gpt = unsafe{ $timer::instance() };
diff --git a/rtic-monotonics/src/nrf/rtc.rs b/rtic-monotonics/src/nrf/rtc.rs
index 94913071..1f4e34f5 100644
--- a/rtic-monotonics/src/nrf/rtc.rs
+++ b/rtic-monotonics/src/nrf/rtc.rs
@@ -43,7 +43,7 @@ use nrf9160_pac::{self as pac, Interrupt, RTC0_NS as RTC0, RTC1_NS as RTC1};
use crate::{Monotonic, TimeoutError, TimerQueue};
use atomic_polyfill::{AtomicU32, Ordering};
use core::future::Future;
-pub use fugit::{self, ExtU64};
+pub use fugit::{self, ExtU64, ExtU64Ceil};
#[doc(hidden)]
#[macro_export]
@@ -167,28 +167,15 @@ macro_rules! make_rtc {
}
}
- #[cfg(feature = "embedded-hal-async")]
- impl embedded_hal_async::delay::DelayUs for $mono_name {
- #[inline]
- async fn delay_us(&mut self, us: u32) {
- Self::delay((us as u64).micros()).await;
- }
- #[inline]
- async fn delay_ms(&mut self, ms: u32) {
- Self::delay((ms as u64).millis()).await;
- }
- }
+ rtic_time::embedded_hal_delay_impl_fugit64!($mono_name);
- impl embedded_hal::delay::DelayUs for $mono_name {
- fn delay_us(&mut self, us: u32) {
- let done = Self::now() + u64::from(us).micros();
- while Self::now() < done {}
- }
- }
+ #[cfg(feature = "embedded-hal-async")]
+ rtic_time::embedded_hal_async_delay_impl_fugit64!($mono_name);
impl Monotonic for $mono_name {
const ZERO: Self::Instant = Self::Instant::from_ticks(0);
+ const TICK_PERIOD: Self::Duration = Self::Duration::from_ticks(1);
type Instant = fugit::TimerInstantU64<32_768>;
type Duration = fugit::TimerDurationU64<32_768>;
diff --git a/rtic-monotonics/src/nrf/timer.rs b/rtic-monotonics/src/nrf/timer.rs
index 589cc6fd..0488ca9b 100644
--- a/rtic-monotonics/src/nrf/timer.rs
+++ b/rtic-monotonics/src/nrf/timer.rs
@@ -29,7 +29,7 @@
use crate::{Monotonic, TimeoutError, TimerQueue};
use atomic_polyfill::{AtomicU32, Ordering};
use core::future::Future;
-pub use fugit::{self, ExtU64};
+pub use fugit::{self, ExtU64, ExtU64Ceil};
#[cfg(feature = "nrf52810")]
use nrf52810_pac::{self as pac, Interrupt, TIMER0, TIMER1, TIMER2};
@@ -203,28 +203,14 @@ macro_rules! make_timer {
}
}
- #[cfg(feature = "embedded-hal-async")]
- impl embedded_hal_async::delay::DelayUs for $mono_name {
- #[inline]
- async fn delay_us(&mut self, us: u32) {
- Self::delay((us as u64).micros()).await;
- }
-
- #[inline]
- async fn delay_ms(&mut self, ms: u32) {
- Self::delay((ms as u64).millis()).await;
- }
- }
+ rtic_time::embedded_hal_delay_impl_fugit64!($mono_name);
- impl embedded_hal::delay::DelayUs for $mono_name {
- fn delay_us(&mut self, us: u32) {
- let done = Self::now() + (us as u64).micros();
- while Self::now() < done {}
- }
- }
+ #[cfg(feature = "embedded-hal-async")]
+ rtic_time::embedded_hal_async_delay_impl_fugit64!($mono_name);
impl Monotonic for $mono_name {
const ZERO: Self::Instant = Self::Instant::from_ticks(0);
+ const TICK_PERIOD: Self::Duration = Self::Duration::from_ticks(1);
type Instant = fugit::TimerInstantU64<1_000_000>;
type Duration = fugit::TimerDurationU64<1_000_000>;
diff --git a/rtic-monotonics/src/rp2040.rs b/rtic-monotonics/src/rp2040.rs
index 130c7d3e..998b5325 100644
--- a/rtic-monotonics/src/rp2040.rs
+++ b/rtic-monotonics/src/rp2040.rs
@@ -28,7 +28,7 @@ use super::Monotonic;
pub use super::{TimeoutError, TimerQueue};
use core::future::Future;
-pub use fugit::{self, ExtU64};
+pub use fugit::{self, ExtU64, ExtU64Ceil};
use rp2040_pac::{timer, Interrupt, NVIC, RESETS, TIMER};
/// Timer implementing [`Monotonic`] which runs at 1 MHz.
@@ -104,6 +104,7 @@ impl Monotonic for Timer {
type Duration = fugit::TimerDurationU64<1_000_000>;
const ZERO: Self::Instant = Self::Instant::from_ticks(0);
+ const TICK_PERIOD: Self::Duration = Self::Duration::from_ticks(1);
fn now() -> Self::Instant {
let timer = Self::timer();
@@ -151,23 +152,10 @@ impl Monotonic for Timer {
fn disable_timer() {}
}
-#[cfg(feature = "embedded-hal-async")]
-impl embedded_hal_async::delay::DelayUs for Timer {
- async fn delay_us(&mut self, us: u32) {
- Self::delay((us as u64).micros()).await;
- }
-
- async fn delay_ms(&mut self, ms: u32) {
- Self::delay((ms as u64).millis()).await;
- }
-}
+rtic_time::embedded_hal_delay_impl_fugit64!(Timer);
-impl embedded_hal::delay::DelayUs for Timer {
- fn delay_us(&mut self, us: u32) {
- let done = Self::now() + u64::from(us).micros();
- while Self::now() < done {}
- }
-}
+#[cfg(feature = "embedded-hal-async")]
+rtic_time::embedded_hal_async_delay_impl_fugit64!(Timer);
/// Register the Timer interrupt for the monotonic.
#[macro_export]
diff --git a/rtic-monotonics/src/stm32.rs b/rtic-monotonics/src/stm32.rs
index 2676a346..254f1302 100644
--- a/rtic-monotonics/src/stm32.rs
+++ b/rtic-monotonics/src/stm32.rs
@@ -36,7 +36,7 @@
use crate::{Monotonic, TimeoutError, TimerQueue};
use atomic_polyfill::{compiler_fence, AtomicU64, Ordering};
-pub use fugit::{self, ExtU64};
+pub use fugit::{self, ExtU64, ExtU64Ceil};
use stm32_metapac as pac;
mod _generated {
@@ -218,31 +218,17 @@ macro_rules! make_timer {
}
}
- #[cfg(feature = "embedded-hal-async")]
- impl embedded_hal_async::delay::DelayUs for $mono_name {
- #[inline]
- async fn delay_us(&mut self, us: u32) {
- Self::delay((us as u64).micros()).await;
- }
-
- #[inline]
- async fn delay_ms(&mut self, ms: u32) {
- Self::delay((ms as u64).millis()).await;
- }
- }
+ rtic_time::embedded_hal_delay_impl_fugit64!($mono_name);
- impl embedded_hal::delay::DelayUs for $mono_name {
- fn delay_us(&mut self, us: u32) {
- let done = Self::now() + (us as u64).micros();
- while Self::now() < done {}
- }
- }
+ #[cfg(feature = "embedded-hal-async")]
+ rtic_time::embedded_hal_async_delay_impl_fugit64!($mono_name);
impl Monotonic for $mono_name {
type Instant = fugit::TimerInstantU64<TIMER_HZ>;
type Duration = fugit::TimerDurationU64<TIMER_HZ>;
const ZERO: Self::Instant = Self::Instant::from_ticks(0);
+ const TICK_PERIOD: Self::Duration = Self::Duration::from_ticks(1);
fn now() -> Self::Instant {
// Credits to the `time-driver` of `embassy-stm32`.
diff --git a/rtic-monotonics/src/systick.rs b/rtic-monotonics/src/systick.rs
index 265ca9a0..9bd056ce 100644
--- a/rtic-monotonics/src/systick.rs
+++ b/rtic-monotonics/src/systick.rs
@@ -40,11 +40,11 @@ use cortex_m::peripheral::SYST;
pub use fugit;
cfg_if::cfg_if! {
if #[cfg(feature = "systick-64bit")] {
- pub use fugit::ExtU64;
+ pub use fugit::{ExtU64, ExtU64Ceil};
use atomic_polyfill::AtomicU64;
static SYSTICK_CNT: AtomicU64 = AtomicU64::new(0);
} else {
- pub use fugit::ExtU32;
+ pub use fugit::{ExtU32, ExtU32Ceil};
use atomic_polyfill::AtomicU32;
static SYSTICK_CNT: AtomicU32 = AtomicU32::new(0);
}
@@ -156,6 +156,7 @@ impl Monotonic for Systick {
}
const ZERO: Self::Instant = Self::Instant::from_ticks(0);
+ const TICK_PERIOD: Self::Duration = Self::Duration::from_ticks(1);
fn now() -> Self::Instant {
if Self::systick().has_wrapped() {
@@ -188,27 +189,17 @@ impl Monotonic for Systick {
fn disable_timer() {}
}
-#[cfg(feature = "embedded-hal-async")]
-impl embedded_hal_async::delay::DelayUs for Systick {
- async fn delay_us(&mut self, us: u32) {
- #[cfg(feature = "systick-64bit")]
- let us = u64::from(us);
- Self::delay(us.micros()).await;
- }
+cfg_if::cfg_if! {
+ if #[cfg(feature = "systick-64bit")] {
+ rtic_time::embedded_hal_delay_impl_fugit64!(Systick);
- async fn delay_ms(&mut self, ms: u32) {
- #[cfg(feature = "systick-64bit")]
- let ms = u64::from(ms);
- Self::delay(ms.millis()).await;
- }
-}
+ #[cfg(feature = "embedded-hal-async")]
+ rtic_time::embedded_hal_async_delay_impl_fugit64!(Systick);
+ } else {
+ rtic_time::embedded_hal_delay_impl_fugit32!(Systick);
-impl embedded_hal::delay::DelayUs for Systick {
- fn delay_us(&mut self, us: u32) {
- #[cfg(feature = "systick-64bit")]
- let us = u64::from(us);
- let done = Self::now() + us.micros();
- while Self::now() < done {}
+ #[cfg(feature = "embedded-hal-async")]
+ rtic_time::embedded_hal_async_delay_impl_fugit32!(Systick);
}
}
diff --git a/rtic-sync/Cargo.toml b/rtic-sync/Cargo.toml
index 39f62dc8..8172e262 100644
--- a/rtic-sync/Cargo.toml
+++ b/rtic-sync/Cargo.toml
@@ -21,9 +21,9 @@ heapless = "0.7"
critical-section = "1"
rtic-common = { version = "1.0.0", path = "../rtic-common" }
portable-atomic = { version = "1", default-features = false }
-embedded-hal = { version = "1.0.0-rc.1", optional = true }
-embedded-hal-async = { version = "1.0.0-rc.1", optional = true }
-embedded-hal-bus = { version = "0.1.0-rc.1", optional = true, features = ["async"] }
+embedded-hal = { version = "1.0.0-rc.2", optional = true }
+embedded-hal-async = { version = "1.0.0-rc.2", optional = true }
+embedded-hal-bus = { version = "0.1.0-rc.2", optional = true, features = ["async"] }
[dev-dependencies]
tokio = { version = "1", features = ["rt", "macros", "time"] }
diff --git a/rtic-sync/src/arbiter.rs b/rtic-sync/src/arbiter.rs
index 99d41748..f0dbc4cd 100644
--- a/rtic-sync/src/arbiter.rs
+++ b/rtic-sync/src/arbiter.rs
@@ -197,7 +197,7 @@ pub mod spi {
use super::Arbiter;
use embedded_hal::digital::OutputPin;
use embedded_hal_async::{
- delay::DelayUs,
+ delay::DelayNs,
spi::{ErrorType, Operation, SpiBus, SpiDevice},
};
use embedded_hal_bus::spi::DeviceError;
@@ -229,7 +229,7 @@ pub mod spi {
Word: Copy + 'static,
BUS: SpiBus<Word>,
CS: OutputPin,
- D: DelayUs,
+ D: DelayNs,
{
async fn transaction(
&mut self,
@@ -246,10 +246,10 @@ pub mod spi {
Operation::Write(buf) => bus.write(buf).await,
Operation::Transfer(read, write) => bus.transfer(read, write).await,
Operation::TransferInPlace(buf) => bus.transfer_in_place(buf).await,
- Operation::DelayUs(us) => match bus.flush().await {
+ Operation::DelayNs(ns) => match bus.flush().await {
Err(e) => Err(e),
Ok(()) => {
- self.delay.delay_us(*us).await;
+ self.delay.delay_ns(*ns).await;
Ok(())
}
},
diff --git a/rtic-time/CHANGELOG.md b/rtic-time/CHANGELOG.md
index 2e7bebc3..cf312ac1 100644
--- a/rtic-time/CHANGELOG.md
+++ b/rtic-time/CHANGELOG.md
@@ -15,6 +15,7 @@ For each category, *Added*, *Changed*, *Fixed* add new entries at the top!
### Fixed
+- **Soundness fix:** `TimerQueue` did not wait long enough in `Duration` based delays. Fixing this sadly required adding a `const TICK_PERIOD` to the `Monotonic` trait, which requires updating all existing implementations.
- If the queue was non-empty and a new instant was added that was earlier than `head`, then the queue would no pend the monotonic handler. This would cause the new `head` to be dequeued at the wrong time.
## [v1.0.0] - 2023-05-31
diff --git a/rtic-time/Cargo.toml b/rtic-time/Cargo.toml
index ef7635e3..a8379e4a 100644
--- a/rtic-time/Cargo.toml
+++ b/rtic-time/Cargo.toml
@@ -22,5 +22,9 @@ futures-util = { version = "0.3.25", default-features = false }
rtic-common = { version = "1.0.0", path = "../rtic-common" }
[dev-dependencies]
+embedded-hal = { version = "1.0.0-rc.2" }
+embedded-hal-async = { version = "1.0.0-rc.2" }
+fugit = "0.3.7"
parking_lot = "0.12"
cassette = "0.2"
+cooked-waker = "5.0.0"
diff --git a/rtic-time/src/lib.rs b/rtic-time/src/lib.rs
index 0c3e3495..4c89d52c 100644
--- a/rtic-time/src/lib.rs
+++ b/rtic-time/src/lib.rs
@@ -181,22 +181,36 @@ impl<Mono: Monotonic> TimerQueue<Mono> {
}
}
- /// Timeout after a specific duration.
+ /// Timeout after at least a specific duration.
#[inline]
pub async fn timeout_after<F: Future>(
&self,
duration: Mono::Duration,
future: F,
) -> Result<F::Output, TimeoutError> {
- self.timeout_at(Mono::now() + duration, future).await
+ let now = Mono::now();
+ let mut timeout = now + duration;
+ if now != timeout {
+ timeout = timeout + Mono::TICK_PERIOD;
+ }
+
+ // Wait for one period longer, because by definition timers have an uncertainty
+ // of one period, so waiting for 'at least' needs to compensate for that.
+ self.timeout_at(timeout, future).await
}
- /// Delay for some duration of time.
+ /// Delay for at least some duration of time.
#[inline]
pub async fn delay(&self, duration: Mono::Duration) {
let now = Mono::now();
+ let mut timeout = now + duration;
+ if now != timeout {
+ timeout = timeout + Mono::TICK_PERIOD;
+ }
- self.delay_until(now + duration).await;
+ // Wait for one period longer, because by definition timers have an uncertainty
+ // of one period, so waiting for 'at least' needs to compensate for that.
+ self.delay_until(timeout).await;
}
/// Delay to some specific time instant.
diff --git a/rtic-time/src/monotonic.rs b/rtic-time/src/monotonic.rs
index ad79bf20..0c0d6f35 100644
--- a/rtic-time/src/monotonic.rs
+++ b/rtic-time/src/monotonic.rs
@@ -10,6 +10,9 @@ pub trait Monotonic {
/// The time at time zero.
const ZERO: Self::Instant;
+ /// The duration between two timer ticks.
+ const TICK_PERIOD: Self::Duration;
+
/// The type for instant, defining an instant in time.
///
/// **Note:** In all APIs in RTIC that use instants from this monotonic, this type will be used.
@@ -65,3 +68,153 @@ pub trait Monotonic {
/// NOTE: This may be called more than once.
fn disable_timer() {}
}
+
+/// Creates impl blocks for `embedded_hal::delay::DelayUs`,
+/// based on `fugit::ExtU64Ceil`.
+#[macro_export]
+macro_rules! embedded_hal_delay_impl_fugit64 {
+ ($t:ty) => {
+ impl ::embedded_hal::delay::DelayNs for $t {
+ fn delay_ns(&mut self, ns: u32) {
+ use ::fugit::ExtU64Ceil;
+
+ let now = Self::now();
+ let mut done = now + u64::from(ns).nanos_at_least();
+ if now != done {
+ // Compensate for sub-tick uncertainty
+ done += Self::TICK_PERIOD;
+ }
+
+ while Self::now() < done {}
+ }
+
+ fn delay_us(&mut self, us: u32) {
+ use ::fugit::ExtU64Ceil;
+
+ let now = Self::now();
+ let mut done = now + u64::from(us).micros_at_least();
+ if now != done {
+ // Compensate for sub-tick uncertainty
+ done += Self::TICK_PERIOD;
+ }
+
+ while Self::now() < done {}
+ }
+
+ fn delay_ms(&mut self, ms: u32) {
+ use ::fugit::ExtU64Ceil;
+
+ let now = Self::now();
+ let mut done = now + u64::from(ms).millis_at_least();
+ if now != done {
+ // Compensate for sub-tick uncertainty
+ done += Self::TICK_PERIOD;
+ }
+
+ while Self::now() < done {}
+ }
+ }
+ };
+}
+
+/// Creates impl blocks for `embedded_hal_async::delay::DelayUs`,
+/// based on `fugit::ExtU64Ceil`.
+#[macro_export]
+macro_rules! embedded_hal_async_delay_impl_fugit64 {
+ ($t:ty) => {
+ impl ::embedded_hal_async::delay::DelayNs for $t {
+ #[inline]
+ async fn delay_ns(&mut self, ns: u32) {
+ use ::fugit::ExtU64Ceil;
+ Self::delay(u64::from(ns).nanos_at_least()).await;
+ }
+
+ #[inline]
+ async fn delay_us(&mut self, us: u32) {
+ use ::fugit::ExtU64Ceil;
+ Self::delay(u64::from(us).micros_at_least()).await;
+ }
+
+ #[inline]
+ async fn delay_ms(&mut self, ms: u32) {
+ use ::fugit::ExtU64Ceil;
+ Self::delay(u64::from(ms).millis_at_least()).await;
+ }
+ }
+ };
+}
+
+/// Creates impl blocks for `embedded_hal::delay::DelayUs`,
+/// based on `fugit::ExtU32Ceil`.
+#[macro_export]
+macro_rules! embedded_hal_delay_impl_fugit32 {
+ ($t:ty) => {
+ impl ::embedded_hal::delay::DelayNs for $t {
+ fn delay_ns(&mut self, ns: u32) {
+ use ::fugit::ExtU32Ceil;
+
+ let now = Self::now();
+ let mut done = now + ns.nanos_at_least();
+ if now != done {
+ // Compensate for sub-tick uncertainty
+ done += Self::TICK_PERIOD;
+ }
+
+ while Self::now() < done {}
+ }
+
+ fn delay_us(&mut self, us: u32) {
+ use ::fugit::ExtU32Ceil;
+
+ let now = Self::now();
+ let mut done = now + us.micros_at_least();
+ if now != done {
+ // Compensate for sub-tick uncertainty
+ done += Self::TICK_PERIOD;
+ }
+
+ while Self::now() < done {}
+ }
+
+ fn delay_ms(&mut self, ms: u32) {
+ use ::fugit::ExtU32Ceil;
+
+ let now = Self::now();
+ let mut done = now + ms.millis_at_least();
+ if now != done {
+ // Compensate for sub-tick uncertainty
+ done += Self::TICK_PERIOD;
+ }
+
+ while Self::now() < done {}
+ }
+ }
+ };
+}
+
+/// Creates impl blocks for `embedded_hal_async::delay::DelayUs`,
+/// based on `fugit::ExtU32Ceil`.
+#[macro_export]
+macro_rules! embedded_hal_async_delay_impl_fugit32 {
+ ($t:ty) => {
+ impl ::embedded_hal_async::delay::DelayNs for $t {
+ #[inline]
+ async fn delay_ns(&mut self, ns: u32) {
+ use ::fugit::ExtU32Ceil;
+ Self::delay(ns.nanos_at_least()).await;
+ }
+
+ #[inline]
+ async fn delay_us(&mut self, us: u32) {
+ use ::fugit::ExtU32Ceil;
+ Self::delay(us.micros_at_least()).await;
+ }
+
+ #[inline]
+ async fn delay_ms(&mut self, ms: u32) {
+ use ::fugit::ExtU32Ceil;
+ Self::delay(ms.millis_at_least()).await;
+ }
+ }
+ };
+}
diff --git a/rtic-time/tests/delay_precision_subtick.rs b/rtic-time/tests/delay_precision_subtick.rs
new file mode 100644
index 00000000..4db889c8
--- /dev/null
+++ b/rtic-time/tests/delay_precision_subtick.rs
@@ -0,0 +1,313 @@
+//! A test that verifies the sub-tick correctness of the [`TimerQueue`]'s `delay` functionality.
+//!
+//! To run this test, you need to activate the `critical-section/std` feature.
+
+use std::{
+ fmt::Debug,
+ future::Future,
+ pin::Pin,
+ sync::{
+ atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering},
+ Arc,
+ },
+ task::Context,
+ thread::sleep,
+ time::Duration,
+};
+
+use ::fugit::ExtU64Ceil;
+use cooked_waker::{IntoWaker, WakeRef};
+use parking_lot::Mutex;
+use rtic_time::{Monotonic, TimeoutError, TimerQueue};
+
+const SUBTICKS_PER_TICK: u32 = 10;
+struct SubtickTestTimer;
+static TIMER_QUEUE: TimerQueue<SubtickTestTimer> = TimerQueue::new();
+static NOW_SUBTICKS: AtomicU64 = AtomicU64::new(0);
+static COMPARE_TICKS: Mutex<Option<u64>> = Mutex::new(None);
+
+impl Monotonic for SubtickTestTimer {
+ const ZERO: Self::Instant = Self::Instant::from_ticks(0);
+ const TICK_PERIOD: Self::Duration = Self::Duration::from_ticks(1);
+
+ type Instant = fugit::Instant<u64, SUBTICKS_PER_TICK, 1000>;
+ type Duration = fugit::Duration<u64, SUBTICKS_PER_TICK, 1000>;
+
+ fn now() -> Self::Instant {
+ Self::Instant::from_ticks(
+ NOW_SUBTICKS.load(Ordering::Relaxed) / u64::from(SUBTICKS_PER_TICK),
+ )
+ }
+
+ fn set_compare(instant: Self::Instant) {
+ *COMPARE_TICKS.lock() = Some(instant.ticks());
+ }
+
+ fn clear_compare_flag() {}
+
+ fn pend_interrupt() {
+ unsafe {
+ Self::__tq().on_monotonic_interrupt();
+ }
+ }
+}
+
+impl SubtickTestTimer {
+ pub fn init() {
+ Self::__tq().initialize(Self)
+ }
+
+ pub fn tick() -> u64 {
+ let now = NOW_SUBTICKS.fetch_add(1, Ordering::Relaxed) + 1;
+ let ticks = now / u64::from(SUBTICKS_PER_TICK);
+ let subticks = now % u64::from(SUBTICKS_PER_TICK);
+
+ let compare = COMPARE_TICKS.lock();
+
+ // println!(
+ // "ticks: {ticks}, subticks: {subticks}, compare: {:?}",
+ // *compare
+ // );
+ if subticks == 0 && Some(ticks) == *compare {
+ unsafe {
+ Self::__tq().on_monotonic_interrupt();
+ }
+ }
+
+ subticks
+ }
+
+ pub fn forward_to_subtick(subtick: u64) {
+ assert!(subtick < u64::from(SUBTICKS_PER_TICK));
+ while Self::tick() != subtick {}
+ }
+
+ pub fn now_subticks() -> u64 {
+ NOW_SUBTICKS.load(Ordering::Relaxed)
+ }
+
+ fn __tq() -> &'static TimerQueue<Self> {
+ &TIMER_QUEUE
+ }
+
+ /// Delay for some duration of time.
+ #[inline]
+ pub async fn delay(duration: <Self as Monotonic>::Duration) {
+ Self::__tq().delay(duration).await;
+ }
+
+ /// Timeout after a specific duration.
+ #[inline]
+ pub async fn timeout_after<F: core::future::Future>(
+ duration: <Self as Monotonic>::Duration,
+ future: F,
+ ) -> Result<F::Output, TimeoutError> {
+ Self::__tq().timeout_after(duration, future).await
+ }
+}
+
+rtic_time::embedded_hal_delay_impl_fugit64!(SubtickTestTimer);
+rtic_time::embedded_hal_async_delay_impl_fugit64!(SubtickTestTimer);
+
+// A simple struct that counts the number of times it is awoken. Can't
+// be awoken by value (because that would discard the counter), so we
+// must instead wrap it in an Arc.
+#[derive(Debug, Default)]
+struct WakeCounter {
+ count: AtomicUsize,
+}
+
+impl WakeCounter {
+ fn get(&self) -> usize {
+ self.count.load(Ordering::SeqCst)
+ }
+}
+
+impl WakeRef for WakeCounter {
+ fn wake_by_ref(&self) {
+ let _prev = self.count.fetch_add(1, Ordering::SeqCst);
+ }
+}
+
+struct OnDrop<F: FnOnce()>(Option<F>);
+impl<F: FnOnce()> OnDrop<F> {
+ pub fn new(f: F) -> Self {
+ Self(Some(f))
+ }
+}
+impl<F: FnOnce()> Drop for OnDrop<F> {
+ fn drop(&mut self) {
+ (self.0.take().unwrap())();
+ }
+}
+
+macro_rules! subtick_test {
+ (@run $start:expr, $actual_duration:expr, $delay_fn:expr) => {{
+ // forward clock to $start
+ SubtickTestTimer::forward_to_subtick($start);
+
+ // call wait function
+ let delay_fn = $delay_fn;
+ let mut future = std::pin::pin!(delay_fn);
+
+ let wakecounter = Arc::new(WakeCounter::default());
+ let waker = Arc::clone(&wakecounter).into_waker();
+ let mut context = Context::from_waker(&waker);
+
+ let mut finished_after: Option<u64> = None;
+ for i in 0..10 * u64::from(SUBTICKS_PER_TICK) {
+ if Future::poll(Pin::new(&mut future), &mut context).is_ready() {
+ if finished_after.is_none() {
+ finished_after = Some(i);
+ }
+ break;
+ };
+
+ assert_eq!(wakecounter.get(), 0);
+ SubtickTestTimer::tick();
+ }
+
+ let expected_wakeups = {
+ if $actual_duration == 0 {
+ 0
+ } else {
+ 1
+ }
+ };
+ assert_eq!(wakecounter.get(), expected_wakeups);
+
+ // Tick again to test that we don't get a second wake
+ SubtickTestTimer::tick();
+ assert_eq!(wakecounter.get(), expected_wakeups);
+
+ assert_eq!(
+ Some($actual_duration),
+ finished_after,
+ "Expected to wait {} ticks, but waited {:?} ticks.",
+ $actual_duration,
+ finished_after,
+ );
+ }};
+
+ (@run_blocking $start:expr, $actual_duration:expr, $delay_fn:expr) => {{
+ // forward clock to $start
+ SubtickTestTimer::forward_to_subtick($start);
+
+ let t_start = SubtickTestTimer::now_subticks();
+
+ let finished = AtomicBool::new(false);
+ std::thread::scope(|s|{
+ s.spawn(||{
+ let _finished_guard = OnDrop::new(|| finished.store(true, Ordering::Relaxed));
+ ($delay_fn)();
+ });
+ s.spawn(||{
+ sleep(Duration::from_millis(10));
+ while !finished.load(Ordering::Relaxed) {
+ SubtickTestTimer::tick();
+ sleep(Duration::from_millis(10));
+ }
+ });
+ });
+
+ let t_end = SubtickTestTimer::now_subticks();
+ let measured_duration = t_end - t_start;
+ assert_eq!(
+ $actual_duration,
+ measured_duration,
+ "Expected to wait {} ticks, but waited {:?} ticks.",
+ $actual_duration,
+ measured_duration,
+ );
+ }};
+
+
+
+
+ ($start:expr, $min_duration:expr, $actual_duration:expr) => {{
+ subtick_test!(@run $start, $actual_duration, async {
+ let mut timer = SubtickTestTimer;
+ embedded_hal_async::delay::DelayNs::delay_ms(&mut timer, $min_duration).await;
+ });
+ subtick_test!(@run $start, $actual_duration, async {
+ let mut timer = SubtickTestTimer;
+ embedded_hal_async::delay::DelayNs::delay_us(&mut timer, 1_000 * $min_duration).await;
+ });
+ subtick_test!(@run $start, $actual_duration, async {
+ let mut timer = SubtickTestTimer;
+ embedded_hal_async::delay::DelayNs::delay_ns(&mut timer, 1_000_000 * $min_duration).await;
+ });
+ subtick_test!(@run $start, $actual_duration, async {
+ SubtickTestTimer::delay($min_duration.millis_at_least()).await;
+ });
+ subtick_test!(@run $start, $actual_duration, async {
+ let _ = SubtickTestTimer::timeout_after($min_duration.millis_at_least(), std::future::pending::<()>()).await;
+ });
+
+ // Those are slow and unreliable; enable them when needed.
+ const ENABLE_BLOCKING_TESTS: bool = false;
+ if ENABLE_BLOCKING_TESTS {
+ subtick_test!(@run_blocking $start, $actual_duration, || {
+ let mut timer = SubtickTestTimer;
+ embedded_hal::delay::DelayNs::delay_ms(&mut timer, $min_duration);
+ });
+ subtick_test!(@run_blocking $start, $actual_duration, || {
+ let mut timer = SubtickTestTimer;
+ embedded_hal::delay::DelayNs::delay_us(&mut timer, 1_000 * $min_duration);
+ });
+ subtick_test!(@run_blocking $start, $actual_duration, || {
+ let mut timer = SubtickTestTimer;
+ embedded_hal::delay::DelayNs::delay_ns(&mut timer, 1_000_000 * $min_duration);
+ });
+ }
+ }};
+}
+
+#[test]
+fn timer_queue_subtick_precision() {
+ SubtickTestTimer::init();
+
+ // subtick_test!(a, b, c) tests the following thing:
+ //
+ // If we start at subtick a and we need to wait b subticks,
+ // then we will actually wait c subticks.
+ // The important part is that c is never smaller than b,
+ // in all cases, as that would violate the contract of
+ // embedded-hal's DelayUs.
+
+ subtick_test!(0, 0, 0);
+ subtick_test!(0, 1, 20);
+ subtick_test!(0, 10, 20);
+ subtick_test!(0, 11, 30);
+ subtick_test!(0, 12, 30);
+
+ subtick_test!(1, 0, 0);
+ subtick_test!(1, 1, 19);
+ subtick_test!(1, 10, 19);
+ subtick_test!(1, 11, 29);
+ subtick_test!(1, 12, 29);
+
+ subtick_test!(2, 0, 0);
+ subtick_test!(2, 1, 18);
+ subtick_test!(2, 10, 18);
+ subtick_test!(2, 11, 28);
+ subtick_test!(2, 12, 28);
+
+ subtick_test!(3, 0, 0);
+ subtick_test!(3, 1, 17);
+ subtick_test!(3, 10, 17);
+ subtick_test!(3, 11, 27);
+ subtick_test!(3, 12, 27);
+
+ subtick_test!(8, 0, 0);
+ subtick_test!(8, 1, 12);
+ subtick_test!(8, 10, 12);
+ subtick_test!(8, 11, 22);
+ subtick_test!(8, 12, 22);
+
+ subtick_test!(9, 0, 0);
+ subtick_test!(9, 1, 11);
+ subtick_test!(9, 10, 11);
+ subtick_test!(9, 11, 21);
+ subtick_test!(9, 12, 21);
+}
diff --git a/rtic-time/tests/timer_queue.rs b/rtic-time/tests/timer_queue.rs
index 9ad71757..8bae3853 100644
--- a/rtic-time/tests/timer_queue.rs
+++ b/rtic-time/tests/timer_queue.rs
@@ -17,7 +17,7 @@ static NOW: Mutex<Option<Instant>> = Mutex::new(None);
pub struct Duration(u64);
impl Duration {
- pub fn from_ticks(millis: u64) -> Self {
+ pub const fn from_ticks(millis: u64) -> Self {
Self(millis)
}
@@ -161,6 +161,7 @@ impl TestMono {
impl Monotonic for TestMono {
const ZERO: Self::Instant = Instant::ZERO;
+ const TICK_PERIOD: Self::Duration = Duration::from_ticks(1);
type Instant = Instant;
@@ -210,7 +211,8 @@ fn timer_queue() {
let elapsed = start.elapsed().as_ticks();
println!("{total_millis} ticks delay reached after {elapsed} ticks");
- if elapsed != total_millis {
+ // Expect a delay of one longer, to compensate for timer uncertainty
+ if elapsed != total_millis + 1 {
panic!(
"{total_millis} ticks delay was not on time ({elapsed} ticks passed instead)"
);
@@ -263,25 +265,25 @@ fn timer_queue() {
if Instant::now() == 0.into() {
// First, we want to be waiting for our 300 tick delay
- assert_eq!(TestMono::compare(), Some(300.into()));
+ assert_eq!(TestMono::compare(), Some(301.into()));
}
if Instant::now() == 100.into() {
// After 100 ticks, we enqueue a new delay that is supposed to last
// until the 200-tick-mark
- assert_eq!(TestMono::compare(), Some(200.into()));
+ assert_eq!(TestMono::compare(), Some(201.into()));
}
- if Instant::now() == 200.into() {
+ if Instant::now() == 201.into() {
// After 200 ticks, we dequeue the 200-tick-mark delay and
// requeue the 300 tick delay
- assert_eq!(TestMono::compare(), Some(300.into()));
+ assert_eq!(TestMono::compare(), Some(301.into()));
}
- if Instant::now() == 300.into() {
+ if Instant::now() == 301.into() {
// After 300 ticks, we dequeue the 300-tick-mark delay and
// go to the 400 tick delay that is already enqueued
- assert_eq!(TestMono::compare(), Some(400.into()));
+ assert_eq!(TestMono::compare(), Some(401.into()));
}
}
diff --git a/rtic/CHANGELOG.md b/rtic/CHANGELOG.md
index db90aa3e..815b1bb0 100644
--- a/rtic/CHANGELOG.md
+++ b/rtic/CHANGELOG.md
@@ -8,10 +8,14 @@ For each category, *Added*, *Changed*, *Fixed* add new entries at the top!
## [Unreleased]
### Added
+
- Unstable support for ESP32-C3
### Fixed
+- **Soundness fix:** Monotonics did not wait long enough in `Duration` based delays.
+ This is not directly a change for `rtic`, but required bumping the minimal version of `rtic-monotonics`.
+
### Changed
## [v2.0.1] - 2023-07-25
diff --git a/rtic/ci/expected/async-timeout.run b/rtic/ci/expected/async-timeout.run
index dea8e5d2..6b4c0898 100644
--- a/rtic/ci/expected/async-timeout.run
+++ b/rtic/ci/expected/async-timeout.run
@@ -5,12 +5,12 @@ the hal takes a duration of Duration { ticks: 45 }
hal returned 5
the hal takes a duration of Duration { ticks: 45 }
hal returned 5
-now is Instant { ticks: 210 }, timeout at Instant { ticks: 260 }
+now is Instant { ticks: 213 }, timeout at Instant { ticks: 263 }
the hal takes a duration of Duration { ticks: 35 }
-hal returned 5 at time Instant { ticks: 245 }
-now is Instant { ticks: 310 }, timeout at Instant { ticks: 360 }
+hal returned 5 at time Instant { ticks: 249 }
+now is Instant { ticks: 313 }, timeout at Instant { ticks: 363 }
the hal takes a duration of Duration { ticks: 45 }
-hal returned 5 at time Instant { ticks: 355 }
-now is Instant { ticks: 410 }, timeout at Instant { ticks: 460 }
+hal returned 5 at time Instant { ticks: 359 }
+now is Instant { ticks: 413 }, timeout at Instant { ticks: 463 }
the hal takes a duration of Duration { ticks: 55 }
timeout
generated by cgit v1.2.3 (git 2.39.1) at 2025-07-22 18:00:04 +0000