From 5a5495aac43d75610735f2ca80fb6c8e8f31ed71 Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Tue, 26 Nov 2024 23:54:21 +0100 Subject: Refactor integrated-timers --- embassy-time-queue-driver/src/lib.rs | 136 ++++++++++++++++++++++++++++++++++- 1 file changed, 134 insertions(+), 2 deletions(-) (limited to 'embassy-time-queue-driver/src/lib.rs') diff --git a/embassy-time-queue-driver/src/lib.rs b/embassy-time-queue-driver/src/lib.rs index 50736e8c7..c5e989854 100644 --- a/embassy-time-queue-driver/src/lib.rs +++ b/embassy-time-queue-driver/src/lib.rs @@ -6,7 +6,29 @@ //! //! - Define a struct `MyTimerQueue` //! - Implement [`TimerQueue`] for it -//! - Register it as the global timer queue with [`timer_queue_impl`](crate::timer_queue_impl). +//! - Register it as the global timer queue with [`timer_queue_impl`]. +//! - Ensure that you process the timer queue when `schedule_wake` is due. This usually involves +//! waking expired tasks, finding the next expiration time and setting an alarm. +//! +//! If a single global timer queue is sufficient for you, you can use the +//! [`GlobalTimerQueue`] type, which is a wrapper around a global timer queue +//! protected by a critical section. +//! +//! ``` +//! use embassy_time_queue_driver::GlobalTimerQueue; +//! embassy_time_queue_driver::timer_queue_impl!( +//! static TIMER_QUEUE_DRIVER: GlobalTimerQueue +//! = GlobalTimerQueue::new(|next_expiration| todo!("Set an alarm")) +//! ); +//! ``` +//! +//! You can also use the `queue_generic` or the `embassy_executor::raw::timer_queue` modules to +//! implement your own timer queue. These modules contain queue implementations which you can wrap +//! and tailor to your needs. +//! +//! If you are providing an embassy-executor implementation besides a timer queue, you can choose to +//! expose the `integrated-timers` feature in your implementation. This feature stores timer items +//! in the tasks themselves, so you don't need a fixed-size queue or dynamic memory allocation. //! //! ## Example //! @@ -14,7 +36,7 @@ //! use core::task::Waker; //! //! use embassy_time::Instant; -//! use embassy_time::queue::{TimerQueue}; +//! use embassy_time::queue::TimerQueue; //! //! struct MyTimerQueue{}; // not public! //! @@ -26,11 +48,18 @@ //! //! embassy_time_queue_driver::timer_queue_impl!(static QUEUE: MyTimerQueue = MyTimerQueue{}); //! ``` + +pub mod queue_generic; + +use core::cell::RefCell; use core::task::Waker; +use critical_section::Mutex; + /// Timer queue pub trait TimerQueue { /// Schedules a waker in the queue to be awoken at moment `at`. + /// /// If this moment is in the past, the waker might be awoken immediately. fn schedule_wake(&'static self, at: u64, waker: &Waker); } @@ -58,3 +87,106 @@ macro_rules! timer_queue_impl { } }; } + +#[cfg(feature = "integrated-timers")] +type InnerQueue = embassy_executor::raw::timer_queue::TimerQueue; + +#[cfg(not(feature = "integrated-timers"))] +type InnerQueue = queue_generic::Queue; + +/// A timer queue implementation that can be used as a global timer queue. +/// +/// This implementation is not thread-safe, and should be protected by a mutex of some sort. +pub struct GenericTimerQueue bool> { + queue: InnerQueue, + set_alarm: F, +} + +impl bool> GenericTimerQueue { + /// Creates a new timer queue. + /// + /// `set_alarm` is a function that should set the next alarm time. The function should + /// return `true` if the alarm was set, and `false` if the alarm was in the past. + pub const fn new(set_alarm: F) -> Self { + Self { + queue: InnerQueue::new(), + set_alarm, + } + } + + /// Schedules a task to run at a specific time, and returns whether any changes were made. + pub fn schedule_wake(&mut self, at: u64, waker: &core::task::Waker) { + #[cfg(feature = "integrated-timers")] + let waker = embassy_executor::raw::task_from_waker(waker); + + if self.queue.schedule_wake(at, waker) { + self.dispatch() + } + } + + /// Dequeues expired timers and returns the next alarm time. + pub fn next_expiration(&mut self, now: u64) -> u64 { + self.queue.next_expiration(now) + } + + /// Handle the alarm. + /// + /// Call this function when the next alarm is due. + pub fn dispatch(&mut self) { + let mut next_expiration = self.next_expiration(embassy_time_driver::now()); + + while !(self.set_alarm)(next_expiration) { + // next_expiration is in the past, dequeue and find a new expiration + next_expiration = self.next_expiration(next_expiration); + } + } +} + +/// A [`GenericTimerQueue`] protected by a critical section. Directly useable as a [`TimerQueue`]. +pub struct GlobalTimerQueue { + inner: Mutex bool>>>, +} + +impl GlobalTimerQueue { + /// Creates a new timer queue. + /// + /// `set_alarm` is a function that should set the next alarm time. The function should + /// return `true` if the alarm was set, and `false` if the alarm was in the past. + pub const fn new(set_alarm: fn(u64) -> bool) -> Self { + Self { + inner: Mutex::new(RefCell::new(GenericTimerQueue::new(set_alarm))), + } + } + + /// Schedules a task to run at a specific time, and returns whether any changes were made. + pub fn schedule_wake(&self, at: u64, waker: &core::task::Waker) { + critical_section::with(|cs| { + let mut inner = self.inner.borrow_ref_mut(cs); + inner.schedule_wake(at, waker); + }); + } + + /// Dequeues expired timers and returns the next alarm time. + pub fn next_expiration(&self, now: u64) -> u64 { + critical_section::with(|cs| { + let mut inner = self.inner.borrow_ref_mut(cs); + inner.next_expiration(now) + }) + } + + /// Handle the alarm. + /// + /// Call this function when the next alarm is due. + pub fn dispatch(&self) { + critical_section::with(|cs| { + let mut inner = self.inner.borrow_ref_mut(cs); + inner.dispatch() + }) + } +} + +impl TimerQueue for GlobalTimerQueue { + fn schedule_wake(&'static self, at: u64, waker: &Waker) { + GlobalTimerQueue::schedule_wake(self, at, waker) + } +} -- cgit From d45ea43892198484b5f6dcea4c351dc11d226cc4 Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Sun, 8 Dec 2024 23:21:53 +0100 Subject: Move integrated timer queue into time-queue-driver --- embassy-time-queue-driver/src/lib.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'embassy-time-queue-driver/src/lib.rs') diff --git a/embassy-time-queue-driver/src/lib.rs b/embassy-time-queue-driver/src/lib.rs index c5e989854..0c78921ed 100644 --- a/embassy-time-queue-driver/src/lib.rs +++ b/embassy-time-queue-driver/src/lib.rs @@ -22,9 +22,9 @@ //! ); //! ``` //! -//! You can also use the `queue_generic` or the `embassy_executor::raw::timer_queue` modules to -//! implement your own timer queue. These modules contain queue implementations which you can wrap -//! and tailor to your needs. +//! You can also use the `queue_generic` or the `queue_integrated` modules to implement your own +//! timer queue. These modules contain queue implementations which you can wrap and tailor to +//! your needs. //! //! If you are providing an embassy-executor implementation besides a timer queue, you can choose to //! expose the `integrated-timers` feature in your implementation. This feature stores timer items @@ -49,7 +49,10 @@ //! embassy_time_queue_driver::timer_queue_impl!(static QUEUE: MyTimerQueue = MyTimerQueue{}); //! ``` +#[cfg(not(feature = "integrated-timers"))] pub mod queue_generic; +#[cfg(feature = "integrated-timers")] +pub mod queue_integrated; use core::cell::RefCell; use core::task::Waker; @@ -89,7 +92,7 @@ macro_rules! timer_queue_impl { } #[cfg(feature = "integrated-timers")] -type InnerQueue = embassy_executor::raw::timer_queue::TimerQueue; +type InnerQueue = queue_integrated::TimerQueue; #[cfg(not(feature = "integrated-timers"))] type InnerQueue = queue_generic::Queue; -- cgit From ec96395d084d5edc8be25ddaea8547e2ebd447a6 Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Mon, 9 Dec 2024 08:43:57 +0100 Subject: Prevent task from respawning while in the timer queue --- embassy-time-queue-driver/src/lib.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'embassy-time-queue-driver/src/lib.rs') diff --git a/embassy-time-queue-driver/src/lib.rs b/embassy-time-queue-driver/src/lib.rs index 0c78921ed..2d5fd449a 100644 --- a/embassy-time-queue-driver/src/lib.rs +++ b/embassy-time-queue-driver/src/lib.rs @@ -73,6 +73,20 @@ extern "Rust" { /// Schedule the given waker to be woken at `at`. pub fn schedule_wake(at: u64, waker: &Waker) { + #[cfg(feature = "integrated-timers")] + { + use embassy_executor::raw::task_from_waker; + use embassy_executor::raw::timer_queue::TimerEnqueueOperation; + // The very first thing we must do, before we even access the timer queue, is to + // mark the task a TIMER_QUEUED. This ensures that the task that is being scheduled + // can not be respawn while we are accessing the timer queue. + let task = task_from_waker(waker); + if unsafe { task.timer_enqueue() } == TimerEnqueueOperation::Ignore { + // We are not allowed to enqueue the task in the timer queue. This is because the + // task is not spawned, and so it makes no sense to schedule it. + return; + } + } unsafe { _embassy_time_schedule_wake(at, waker) } } -- cgit From b268b1795fed58544c166c41842ce0d66328aa3e Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Sun, 8 Dec 2024 23:27:32 +0100 Subject: Merge time-driver and time-queue-driver traits, make HALs own and handle the queue. --- embassy-time-queue-driver/src/lib.rs | 140 +++-------------------------------- 1 file changed, 10 insertions(+), 130 deletions(-) (limited to 'embassy-time-queue-driver/src/lib.rs') diff --git a/embassy-time-queue-driver/src/lib.rs b/embassy-time-queue-driver/src/lib.rs index 2d5fd449a..ed490a0ef 100644 --- a/embassy-time-queue-driver/src/lib.rs +++ b/embassy-time-queue-driver/src/lib.rs @@ -49,23 +49,18 @@ //! embassy_time_queue_driver::timer_queue_impl!(static QUEUE: MyTimerQueue = MyTimerQueue{}); //! ``` +use core::task::Waker; + #[cfg(not(feature = "integrated-timers"))] pub mod queue_generic; #[cfg(feature = "integrated-timers")] pub mod queue_integrated; -use core::cell::RefCell; -use core::task::Waker; - -use critical_section::Mutex; +#[cfg(feature = "integrated-timers")] +pub use queue_integrated::Queue; -/// Timer queue -pub trait TimerQueue { - /// Schedules a waker in the queue to be awoken at moment `at`. - /// - /// If this moment is in the past, the waker might be awoken immediately. - fn schedule_wake(&'static self, at: u64, waker: &Waker); -} +#[cfg(not(feature = "integrated-timers"))] +pub use queue_generic::Queue; extern "Rust" { fn _embassy_time_schedule_wake(at: u64, waker: &Waker); @@ -73,7 +68,10 @@ extern "Rust" { /// Schedule the given waker to be woken at `at`. pub fn schedule_wake(at: u64, waker: &Waker) { - #[cfg(feature = "integrated-timers")] + // This function is not implemented in embassy-time-driver because it needs access to executor + // internals. The function updates task state, then delegates to the implementation provided + // by the time driver. + #[cfg(not(feature = "_generic-queue"))] { use embassy_executor::raw::task_from_waker; use embassy_executor::raw::timer_queue::TimerEnqueueOperation; @@ -89,121 +87,3 @@ pub fn schedule_wake(at: u64, waker: &Waker) { } unsafe { _embassy_time_schedule_wake(at, waker) } } - -/// Set the TimerQueue implementation. -/// -/// See the module documentation for an example. -#[macro_export] -macro_rules! timer_queue_impl { - (static $name:ident: $t: ty = $val:expr) => { - static $name: $t = $val; - - #[no_mangle] - fn _embassy_time_schedule_wake(at: u64, waker: &core::task::Waker) { - <$t as $crate::TimerQueue>::schedule_wake(&$name, at, waker); - } - }; -} - -#[cfg(feature = "integrated-timers")] -type InnerQueue = queue_integrated::TimerQueue; - -#[cfg(not(feature = "integrated-timers"))] -type InnerQueue = queue_generic::Queue; - -/// A timer queue implementation that can be used as a global timer queue. -/// -/// This implementation is not thread-safe, and should be protected by a mutex of some sort. -pub struct GenericTimerQueue bool> { - queue: InnerQueue, - set_alarm: F, -} - -impl bool> GenericTimerQueue { - /// Creates a new timer queue. - /// - /// `set_alarm` is a function that should set the next alarm time. The function should - /// return `true` if the alarm was set, and `false` if the alarm was in the past. - pub const fn new(set_alarm: F) -> Self { - Self { - queue: InnerQueue::new(), - set_alarm, - } - } - - /// Schedules a task to run at a specific time, and returns whether any changes were made. - pub fn schedule_wake(&mut self, at: u64, waker: &core::task::Waker) { - #[cfg(feature = "integrated-timers")] - let waker = embassy_executor::raw::task_from_waker(waker); - - if self.queue.schedule_wake(at, waker) { - self.dispatch() - } - } - - /// Dequeues expired timers and returns the next alarm time. - pub fn next_expiration(&mut self, now: u64) -> u64 { - self.queue.next_expiration(now) - } - - /// Handle the alarm. - /// - /// Call this function when the next alarm is due. - pub fn dispatch(&mut self) { - let mut next_expiration = self.next_expiration(embassy_time_driver::now()); - - while !(self.set_alarm)(next_expiration) { - // next_expiration is in the past, dequeue and find a new expiration - next_expiration = self.next_expiration(next_expiration); - } - } -} - -/// A [`GenericTimerQueue`] protected by a critical section. Directly useable as a [`TimerQueue`]. -pub struct GlobalTimerQueue { - inner: Mutex bool>>>, -} - -impl GlobalTimerQueue { - /// Creates a new timer queue. - /// - /// `set_alarm` is a function that should set the next alarm time. The function should - /// return `true` if the alarm was set, and `false` if the alarm was in the past. - pub const fn new(set_alarm: fn(u64) -> bool) -> Self { - Self { - inner: Mutex::new(RefCell::new(GenericTimerQueue::new(set_alarm))), - } - } - - /// Schedules a task to run at a specific time, and returns whether any changes were made. - pub fn schedule_wake(&self, at: u64, waker: &core::task::Waker) { - critical_section::with(|cs| { - let mut inner = self.inner.borrow_ref_mut(cs); - inner.schedule_wake(at, waker); - }); - } - - /// Dequeues expired timers and returns the next alarm time. - pub fn next_expiration(&self, now: u64) -> u64 { - critical_section::with(|cs| { - let mut inner = self.inner.borrow_ref_mut(cs); - inner.next_expiration(now) - }) - } - - /// Handle the alarm. - /// - /// Call this function when the next alarm is due. - pub fn dispatch(&self) { - critical_section::with(|cs| { - let mut inner = self.inner.borrow_ref_mut(cs); - inner.dispatch() - }) - } -} - -impl TimerQueue for GlobalTimerQueue { - fn schedule_wake(&'static self, at: u64, waker: &Waker) { - GlobalTimerQueue::schedule_wake(self, at, waker) - } -} -- cgit From 2f2e2c6031a1abaecdac5ed2febe109e647fe6fd Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 9 Dec 2024 00:28:14 +0100 Subject: Make `integrated-timers` the default, remove Cargo feature. --- embassy-time-queue-driver/src/lib.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'embassy-time-queue-driver/src/lib.rs') diff --git a/embassy-time-queue-driver/src/lib.rs b/embassy-time-queue-driver/src/lib.rs index ed490a0ef..46dd646ca 100644 --- a/embassy-time-queue-driver/src/lib.rs +++ b/embassy-time-queue-driver/src/lib.rs @@ -51,16 +51,15 @@ use core::task::Waker; -#[cfg(not(feature = "integrated-timers"))] +#[cfg(feature = "_generic-queue")] pub mod queue_generic; -#[cfg(feature = "integrated-timers")] +#[cfg(not(feature = "_generic-queue"))] pub mod queue_integrated; -#[cfg(feature = "integrated-timers")] -pub use queue_integrated::Queue; - -#[cfg(not(feature = "integrated-timers"))] +#[cfg(feature = "_generic-queue")] pub use queue_generic::Queue; +#[cfg(not(feature = "_generic-queue"))] +pub use queue_integrated::Queue; extern "Rust" { fn _embassy_time_schedule_wake(at: u64, waker: &Waker); -- cgit From 0492dba5368e7cb22ede2d41d26d4d0431ba2252 Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Sun, 15 Dec 2024 19:24:49 +0100 Subject: Update documentation and changelogs --- embassy-time-queue-driver/src/lib.rs | 49 ++++-------------------------------- 1 file changed, 5 insertions(+), 44 deletions(-) (limited to 'embassy-time-queue-driver/src/lib.rs') diff --git a/embassy-time-queue-driver/src/lib.rs b/embassy-time-queue-driver/src/lib.rs index 46dd646ca..d8b01df3b 100644 --- a/embassy-time-queue-driver/src/lib.rs +++ b/embassy-time-queue-driver/src/lib.rs @@ -2,52 +2,13 @@ #![doc = include_str!("../README.md")] #![warn(missing_docs)] -//! ## Implementing a timer queue +//! This crate is an implementation detail of `embassy-time-driver`. //! -//! - Define a struct `MyTimerQueue` -//! - Implement [`TimerQueue`] for it -//! - Register it as the global timer queue with [`timer_queue_impl`]. -//! - Ensure that you process the timer queue when `schedule_wake` is due. This usually involves -//! waking expired tasks, finding the next expiration time and setting an alarm. +//! As a HAL user, you should only depend on this crate if your application does not use +//! `embassy-executor` and your HAL does not configure a generic queue by itself. //! -//! If a single global timer queue is sufficient for you, you can use the -//! [`GlobalTimerQueue`] type, which is a wrapper around a global timer queue -//! protected by a critical section. -//! -//! ``` -//! use embassy_time_queue_driver::GlobalTimerQueue; -//! embassy_time_queue_driver::timer_queue_impl!( -//! static TIMER_QUEUE_DRIVER: GlobalTimerQueue -//! = GlobalTimerQueue::new(|next_expiration| todo!("Set an alarm")) -//! ); -//! ``` -//! -//! You can also use the `queue_generic` or the `queue_integrated` modules to implement your own -//! timer queue. These modules contain queue implementations which you can wrap and tailor to -//! your needs. -//! -//! If you are providing an embassy-executor implementation besides a timer queue, you can choose to -//! expose the `integrated-timers` feature in your implementation. This feature stores timer items -//! in the tasks themselves, so you don't need a fixed-size queue or dynamic memory allocation. -//! -//! ## Example -//! -//! ``` -//! use core::task::Waker; -//! -//! use embassy_time::Instant; -//! use embassy_time::queue::TimerQueue; -//! -//! struct MyTimerQueue{}; // not public! -//! -//! impl TimerQueue for MyTimerQueue { -//! fn schedule_wake(&'static self, at: u64, waker: &Waker) { -//! todo!() -//! } -//! } -//! -//! embassy_time_queue_driver::timer_queue_impl!(static QUEUE: MyTimerQueue = MyTimerQueue{}); -//! ``` +//! As a HAL implementer, you need to depend on this crate if you want to implement a time driver, +//! but how you should do so is documented in [`embassy_time_driver`]. use core::task::Waker; -- cgit From 4df4ffbbd49729cde38a3a4a73cdafd208372a53 Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Sun, 15 Dec 2024 20:28:12 +0100 Subject: Remove time-driver dependency --- embassy-time-queue-driver/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'embassy-time-queue-driver/src/lib.rs') diff --git a/embassy-time-queue-driver/src/lib.rs b/embassy-time-queue-driver/src/lib.rs index d8b01df3b..97c81a124 100644 --- a/embassy-time-queue-driver/src/lib.rs +++ b/embassy-time-queue-driver/src/lib.rs @@ -8,7 +8,7 @@ //! `embassy-executor` and your HAL does not configure a generic queue by itself. //! //! As a HAL implementer, you need to depend on this crate if you want to implement a time driver, -//! but how you should do so is documented in [`embassy_time_driver`]. +//! but how you should do so is documented in `embassy-time-driver`. use core::task::Waker; -- cgit