Refactor the refactor refactor

esp-hal-embassy/build.rs

@@ -60,14 +60,14 @@ fn main() -> Result<(), Box<dyn Error>> {
     println!("cargo:rustc-check-cfg=cfg(single_queue)");
     println!("cargo:rustc-check-cfg=cfg(generic_timers)");
 
-    if cfg!(feature = "integrated-timers") {
+    if cfg!(feature = "generic-queue") {
+        println!("cargo:rustc-cfg=generic_timers");
+        println!("cargo:rustc-cfg=single_queue");
+    } else {
         println!("cargo:rustc-cfg=integrated_timers");
         if cfg!(feature = "single-queue") {
             println!("cargo:rustc-cfg=single_queue");
         }
-    } else {
-        println!("cargo:rustc-cfg=generic_timers");
-        println!("cargo:rustc-cfg=single_queue");
     }
 
     Ok(())

esp-hal-embassy/src/time_driver.rs

@@ -75,6 +75,9 @@ impl Alarm {
 }
 
 pub(super) struct EmbassyTimer {
+    #[cfg(single_queue)]
+    pub(crate) inner: crate::timer_queue::TimerQueue,
+
     alarms: [Alarm; MAX_SUPPORTED_ALARM_COUNT],
     available_timers: Locked<Option<&'static mut [Timer]>>,
 }
@@ -97,6 +100,8 @@ macro_rules! alarms {
 
 const MAX_SUPPORTED_ALARM_COUNT: usize = 7;
 embassy_time_driver::time_driver_impl!(static DRIVER: EmbassyTimer = EmbassyTimer {
+    #[cfg(single_queue)]
+    inner: crate::timer_queue::TimerQueue::new(Priority::max()),
     alarms: alarms!(0, 1, 2, 3, 4, 5, 6),
     available_timers: Locked::new(None),
 });
@@ -129,7 +134,7 @@ impl EmbassyTimer {
     }
 
     fn on_interrupt(&self, id: usize) {
-        let ctx = self.alarms[id].inner.with(|alarm| {
+        let _ctx = self.alarms[id].inner.with(|alarm| {
             if let AlarmState::Initialized(timer) = &mut alarm.state {
                 timer.clear_interrupt();
                 alarm.callback.get()
@@ -141,7 +146,14 @@ impl EmbassyTimer {
             }
         });
 
-        TIMER_QUEUE_DRIVER.handle_alarm(ctx);
+        #[cfg(all(integrated_timers, not(single_queue)))]
+        {
+            let executor = unsafe { &*_ctx.cast::<crate::executor::InnerExecutor>() };
+            executor.timer_queue.dispatch();
+        }
+
+        #[cfg(single_queue)]
+        self.inner.dispatch();
     }
 
     /// Returns `true` if the timer was armed, `false` if the timestamp is in
@@ -238,6 +250,22 @@ impl Driver for EmbassyTimer {
     fn now(&self) -> u64 {
         now().ticks()
     }
+
+    fn schedule_wake(&self, at: u64, waker: &core::task::Waker) {
+        #[cfg(not(single_queue))]
+        unsafe {
+            let task = embassy_executor::raw::task_from_waker(waker);
+            // FIXME: this is UB, use Exposed Provenance API (or something better) when
+            // available. Expose provenance in `InnerExecutor::init`, and use it here.
+            let executor = &*(task.executor().unwrap_unchecked()
+                as *const embassy_executor::raw::Executor)
+                .cast::<crate::executor::InnerExecutor>();
+            executor.timer_queue.schedule_wake(at, waker);
+        }
+
+        #[cfg(single_queue)]
+        self.inner.schedule_wake(at, waker);
+    }
 }
 
 #[cold]
@@ -249,31 +277,6 @@ fn not_enough_timers() -> ! {
     panic!("There are not enough timers to allocate a new alarm. Call esp_hal_embassy::init() with the correct number of timers, or consider either using the single-queue feature or disabling integrated-timers.");
 }
 
-pub(crate) struct TimerQueueDriver {
-    #[cfg(single_queue)]
-    pub(crate) inner: crate::timer_queue::TimerQueue,
-}
-
-impl TimerQueueDriver {
-    const fn new() -> Self {
-        Self {
-            #[cfg(single_queue)]
-            inner: crate::timer_queue::TimerQueue::new(Priority::max()),
-        }
-    }
-
-    fn handle_alarm(&self, _ctx: *const ()) {
-        #[cfg(all(integrated_timers, not(single_queue)))]
-        {
-            let executor = unsafe { &*_ctx.cast::<crate::executor::InnerExecutor>() };
-            executor.timer_queue.dispatch();
-        }
-
-        #[cfg(single_queue)]
-        self.inner.dispatch();
-    }
-}
-
 pub(crate) fn set_up_alarm(priority: Priority, _ctx: *mut ()) -> AlarmHandle {
     let alarm = unsafe {
         DRIVER
@@ -284,5 +287,3 @@ pub(crate) fn set_up_alarm(priority: Priority, _ctx: *mut ()) -> AlarmHandle {
     DRIVER.set_callback_ctx(alarm, _ctx);
     alarm
 }
-
-embassy_time_queue_driver::timer_queue_impl!(static TIMER_QUEUE_DRIVER: TimerQueueDriver = TimerQueueDriver::new());

esp-hal-embassy/src/timer_queue.rs

@@ -66,33 +66,11 @@ impl TimerQueue {
     }
 }
 
-impl embassy_time_queue_driver::TimerQueue for crate::time_driver::TimerQueueDriver {
-    fn schedule_wake(&'static self, at: u64, waker: &core::task::Waker) {
-        #[cfg(integrated_timers)]
-        let waker = embassy_executor::raw::task_from_waker(waker);
-
-        #[cfg(not(single_queue))]
-        unsafe {
-            // FIXME: this is UB, use Exposed Provenance API (or something better) when
-            // available. Expose provenance in `InnerExecutor::init`, and use it here.
-            let executor = &*(waker.executor().unwrap_unchecked()
-                as *const embassy_executor::raw::Executor)
-                .cast::<crate::executor::InnerExecutor>();
-            executor.timer_queue.schedule_wake(at, waker);
-        }
-
-        #[cfg(single_queue)]
-        self.inner.schedule_wake(at, waker);
-    }
-}
-
 #[cfg(integrated_timers)]
 mod adapter {
     use core::cell::RefCell;
 
-    use embassy_executor::raw;
-
-    type Q = embassy_time_queue_driver::queue_integrated::TimerQueue;
+    type Q = embassy_time_queue_driver::queue_integrated::Queue;
 
     /// A simple wrapper around a `Queue` to provide interior mutability.
     pub struct RefCellQueue {
@@ -109,7 +87,7 @@ mod adapter {
 
         /// Schedules a task to run at a specific time, and returns whether any
         /// changes were made.
-        pub fn schedule_wake(&self, at: u64, waker: raw::TaskRef) -> bool {
+        pub fn schedule_wake(&self, at: u64, waker: &core::task::Waker) -> bool {
             self.inner.borrow_mut().schedule_wake(at, waker)
         }
 
@@ -126,7 +104,7 @@ mod adapter {
     }
 
     impl super::TimerQueue {
-        pub fn schedule_wake(&self, at: u64, task: raw::TaskRef) {
+        pub fn schedule_wake(&self, at: u64, task: &core::task::Waker) {
             if self.inner.lock(|q| q.schedule_wake(at, task)) {
                 self.dispatch();
             }

qa-test/src/bin/embassy_executor_benchmark.rs

@@ -1,9 +1,9 @@
 //! Embassy executor benchmark, used to try out optimization ideas.
 
 //% CHIPS: esp32c2 esp32c3 esp32c6 esp32h2 esp32s2 esp32s3
-//% FEATURES: esp-hal-embassy/integrated-timers
-// FEATURES: esp-hal-embassy/integrated-timers esp-hal-embassy/single-queue
 // FEATURES:
+// FEATURES: esp-hal-embassy/single-queue
+//% FEATURES: esp-hal-embassy/generic-queue
 
 #![no_std]
 #![no_main]