From 53608a87ac4b6c8c60b5508551d12f5ba76ca2f6 Mon Sep 17 00:00:00 2001 From: ivmarkov Date: Mon, 26 Sep 2022 13:46:15 +0300 Subject: Address feedback after code review --- embassy-time/src/queue_generic.rs | 474 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 474 insertions(+) create mode 100644 embassy-time/src/queue_generic.rs (limited to 'embassy-time/src/queue_generic.rs') diff --git a/embassy-time/src/queue_generic.rs b/embassy-time/src/queue_generic.rs new file mode 100644 index 000000000..1c4e5398b --- /dev/null +++ b/embassy-time/src/queue_generic.rs @@ -0,0 +1,474 @@ +use core::cell::RefCell; +use core::cmp::Ordering; +use core::task::Waker; + +use atomic_polyfill::{AtomicU64, Ordering as AtomicOrdering}; +use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex; +use embassy_sync::blocking_mutex::Mutex; +use heapless::sorted_linked_list::{LinkedIndexU8, Min, SortedLinkedList}; + +use crate::driver::{allocate_alarm, set_alarm, set_alarm_callback, AlarmHandle}; +use crate::queue::TimerQueue; +use crate::Instant; + +#[cfg(feature = "generic-queue-8")] +const QUEUE_SIZE: usize = 8; +#[cfg(feature = "generic-queue-16")] +const QUEUE_SIZE: usize = 16; +#[cfg(feature = "generic-queue-32")] +const QUEUE_SIZE: usize = 32; +#[cfg(feature = "generic-queue-64")] +const QUEUE_SIZE: usize = 32; +#[cfg(feature = "generic-queue-128")] +const QUEUE_SIZE: usize = 128; +#[cfg(not(any( + feature = "generic-queue-8", + feature = "generic-queue-16", + feature = "generic-queue-32", + feature = "generic-queue-64", + feature = "generic-queue-128" +)))] +const QUEUE_SIZE: usize = 64; + +#[derive(Debug)] +struct Timer { + at: Instant, + waker: Waker, +} + +impl PartialEq for Timer { + fn eq(&self, other: &Self) -> bool { + self.at == other.at + } +} + +impl Eq for Timer {} + +impl PartialOrd for Timer { + fn partial_cmp(&self, other: &Self) -> Option { + self.at.partial_cmp(&other.at) + } +} + +impl Ord for Timer { + fn cmp(&self, other: &Self) -> Ordering { + self.at.cmp(&other.at) + } +} + +struct InnerQueue { + queue: SortedLinkedList, + alarm: Option, + alarm_at: Instant, +} + +impl InnerQueue { + const fn new() -> Self { + Self { + queue: SortedLinkedList::new_u8(), + alarm: None, + alarm_at: Instant::MAX, + } + } + + fn schedule_wake(&mut self, at: Instant, waker: &Waker, alarm_schedule: &AtomicU64) { + self.queue + .find_mut(|timer| timer.waker.will_wake(waker)) + .map(|mut timer| { + timer.at = at; + timer.finish(); + }) + .unwrap_or_else(|| { + let mut timer = Timer { + waker: waker.clone(), + at, + }; + + loop { + match self.queue.push(timer) { + Ok(()) => break, + Err(e) => timer = e, + } + + self.queue.pop().unwrap().waker.wake(); + } + }); + + // Don't wait for the alarm callback to trigger and directly + // dispatch all timers that are already due + // + // Then update the alarm if necessary + self.dispatch(alarm_schedule); + } + + fn dispatch(&mut self, alarm_schedule: &AtomicU64) { + let now = Instant::now(); + + while self.queue.peek().filter(|timer| timer.at <= now).is_some() { + self.queue.pop().unwrap().waker.wake(); + } + + self.update_alarm(alarm_schedule); + } + + fn update_alarm(&mut self, alarm_schedule: &AtomicU64) { + if let Some(timer) = self.queue.peek() { + let new_at = timer.at; + + if self.alarm_at != new_at { + self.alarm_at = new_at; + alarm_schedule.store(new_at.as_ticks(), AtomicOrdering::SeqCst); + } + } else { + self.alarm_at = Instant::MAX; + alarm_schedule.store(Instant::MAX.as_ticks(), AtomicOrdering::SeqCst); + } + } + + fn handle_alarm(&mut self, alarm_schedule: &AtomicU64) { + self.alarm_at = Instant::MAX; + + self.dispatch(alarm_schedule); + } +} + +struct Queue { + inner: Mutex>, + alarm_schedule: AtomicU64, +} + +impl Queue { + const fn new() -> Self { + Self { + inner: Mutex::new(RefCell::new(InnerQueue::new())), + alarm_schedule: AtomicU64::new(u64::MAX), + } + } + + fn schedule_wake(&'static self, at: Instant, waker: &Waker) { + self.inner.lock(|inner| { + let mut inner = inner.borrow_mut(); + + if inner.alarm.is_none() { + let handle = unsafe { allocate_alarm() }.unwrap(); + inner.alarm = Some(handle); + + set_alarm_callback(handle, Self::handle_alarm_callback, self as *const _ as _); + } + + inner.schedule_wake(at, waker, &self.alarm_schedule) + }); + + self.update_alarm(); + } + + fn update_alarm(&self) { + // Need to set the alarm when we are *not* holding the mutex on the inner queue + // because mutexes are not re-entrant, which is a problem because `set_alarm` might immediately + // call us back if the timestamp is in the past. + let alarm_at = self.alarm_schedule.swap(u64::MAX, AtomicOrdering::SeqCst); + + if alarm_at < u64::MAX { + set_alarm(self.inner.lock(|inner| inner.borrow().alarm.unwrap()), alarm_at); + } + } + + fn handle_alarm(&self) { + self.inner + .lock(|inner| inner.borrow_mut().handle_alarm(&self.alarm_schedule)); + + self.update_alarm(); + } + + fn handle_alarm_callback(ctx: *mut ()) { + unsafe { (ctx as *const Self).as_ref().unwrap() }.handle_alarm(); + } +} + +impl TimerQueue for Queue { + fn schedule_wake(&'static self, at: Instant, waker: &Waker) { + Queue::schedule_wake(self, at, waker); + } +} + +crate::timer_queue_impl!(static QUEUE: Queue = Queue::new()); + +#[cfg(test)] +mod tests { + use core::cell::Cell; + use core::sync::atomic::Ordering; + use core::task::{RawWaker, RawWakerVTable, Waker}; + use std::rc::Rc; + use std::sync::Mutex; + + use serial_test::serial; + + use super::InnerQueue; + use crate::driver::{AlarmHandle, Driver}; + use crate::queue_generic::QUEUE; + use crate::Instant; + + struct InnerTestDriver { + now: u64, + alarm: u64, + callback: fn(*mut ()), + ctx: *mut (), + } + + impl InnerTestDriver { + const fn new() -> Self { + Self { + now: 0, + alarm: u64::MAX, + callback: Self::noop, + ctx: core::ptr::null_mut(), + } + } + + fn noop(_ctx: *mut ()) {} + } + + unsafe impl Send for InnerTestDriver {} + + struct TestDriver(Mutex); + + impl TestDriver { + const fn new() -> Self { + Self(Mutex::new(InnerTestDriver::new())) + } + + fn reset(&self) { + *self.0.lock().unwrap() = InnerTestDriver::new(); + } + + fn set_now(&self, now: u64) { + let notify = { + let mut inner = self.0.lock().unwrap(); + + if inner.now < now { + inner.now = now; + + if inner.alarm <= now { + inner.alarm = u64::MAX; + + Some((inner.callback, inner.ctx)) + } else { + None + } + } else { + panic!("Going back in time?"); + } + }; + + if let Some((callback, ctx)) = notify { + (callback)(ctx); + } + } + } + + impl Driver for TestDriver { + fn now(&self) -> u64 { + self.0.lock().unwrap().now + } + + unsafe fn allocate_alarm(&self) -> Option { + Some(AlarmHandle::new(0)) + } + + fn set_alarm_callback(&self, _alarm: AlarmHandle, callback: fn(*mut ()), ctx: *mut ()) { + let mut inner = self.0.lock().unwrap(); + + inner.callback = callback; + inner.ctx = ctx; + } + + fn set_alarm(&self, _alarm: AlarmHandle, timestamp: u64) { + let notify = { + let mut inner = self.0.lock().unwrap(); + + if timestamp <= inner.now { + Some((inner.callback, inner.ctx)) + } else { + inner.alarm = timestamp; + None + } + }; + + if let Some((callback, ctx)) = notify { + (callback)(ctx); + } + } + } + + struct TestWaker { + pub awoken: Rc>, + pub waker: Waker, + } + + impl TestWaker { + fn new() -> Self { + let flag = Rc::new(Cell::new(false)); + + const VTABLE: RawWakerVTable = RawWakerVTable::new( + |data: *const ()| { + unsafe { + Rc::increment_strong_count(data as *const Cell); + } + + RawWaker::new(data as _, &VTABLE) + }, + |data: *const ()| unsafe { + let data = data as *const Cell; + data.as_ref().unwrap().set(true); + Rc::decrement_strong_count(data); + }, + |data: *const ()| unsafe { + (data as *const Cell).as_ref().unwrap().set(true); + }, + |data: *const ()| unsafe { + Rc::decrement_strong_count(data); + }, + ); + + let raw = RawWaker::new(Rc::into_raw(flag.clone()) as _, &VTABLE); + + Self { + awoken: flag.clone(), + waker: unsafe { Waker::from_raw(raw) }, + } + } + } + + crate::time_driver_impl!(static DRIVER: TestDriver = TestDriver::new()); + + fn setup() { + DRIVER.reset(); + + QUEUE.alarm_schedule.store(u64::MAX, Ordering::SeqCst); + QUEUE.inner.lock(|inner| { + *inner.borrow_mut() = InnerQueue::new(); + }); + } + + fn queue_len() -> usize { + QUEUE.inner.lock(|inner| inner.borrow().queue.iter().count()) + } + + #[test] + #[serial] + fn test_schedule() { + setup(); + + assert_eq!(queue_len(), 0); + + let waker = TestWaker::new(); + + QUEUE.schedule_wake(Instant::from_secs(1), &waker.waker); + + assert!(!waker.awoken.get()); + assert_eq!(queue_len(), 1); + } + + #[test] + #[serial] + fn test_schedule_same() { + setup(); + + let waker = TestWaker::new(); + + QUEUE.schedule_wake(Instant::from_secs(1), &waker.waker); + + assert_eq!(queue_len(), 1); + + QUEUE.schedule_wake(Instant::from_secs(1), &waker.waker); + + assert_eq!(queue_len(), 1); + + QUEUE.schedule_wake(Instant::from_secs(100), &waker.waker); + + assert_eq!(queue_len(), 1); + + let waker2 = TestWaker::new(); + + QUEUE.schedule_wake(Instant::from_secs(100), &waker2.waker); + + assert_eq!(queue_len(), 2); + } + + #[test] + #[serial] + fn test_trigger() { + setup(); + + let waker = TestWaker::new(); + + QUEUE.schedule_wake(Instant::from_secs(100), &waker.waker); + + assert!(!waker.awoken.get()); + + DRIVER.set_now(Instant::from_secs(99).as_ticks()); + + assert!(!waker.awoken.get()); + + assert_eq!(queue_len(), 1); + + DRIVER.set_now(Instant::from_secs(100).as_ticks()); + + assert!(waker.awoken.get()); + + assert_eq!(queue_len(), 0); + } + + #[test] + #[serial] + fn test_immediate_trigger() { + setup(); + + let waker = TestWaker::new(); + + QUEUE.schedule_wake(Instant::from_secs(100), &waker.waker); + + DRIVER.set_now(Instant::from_secs(50).as_ticks()); + + let waker2 = TestWaker::new(); + + QUEUE.schedule_wake(Instant::from_secs(40), &waker2.waker); + + assert!(!waker.awoken.get()); + assert!(waker2.awoken.get()); + assert_eq!(queue_len(), 1); + } + + #[test] + #[serial] + fn test_queue_overflow() { + setup(); + + for i in 1..super::QUEUE_SIZE { + let waker = TestWaker::new(); + + QUEUE.schedule_wake(Instant::from_secs(310), &waker.waker); + + assert_eq!(queue_len(), i); + assert!(!waker.awoken.get()); + } + + let first_waker = TestWaker::new(); + + QUEUE.schedule_wake(Instant::from_secs(300), &first_waker.waker); + + assert_eq!(queue_len(), super::QUEUE_SIZE); + assert!(!first_waker.awoken.get()); + + let second_waker = TestWaker::new(); + + QUEUE.schedule_wake(Instant::from_secs(305), &second_waker.waker); + + assert_eq!(queue_len(), super::QUEUE_SIZE); + assert!(first_waker.awoken.get()); + + QUEUE.schedule_wake(Instant::from_secs(320), &TestWaker::new().waker); + assert_eq!(queue_len(), super::QUEUE_SIZE); + assert!(second_waker.awoken.get()); + } +} -- cgit From 4d5550070fe5e80ff2296a71239c568c774b9ceb Mon Sep 17 00:00:00 2001 From: ivmarkov Date: Mon, 24 Oct 2022 09:17:43 +0300 Subject: Change time Driver contract to never fire the alarm synchronously --- embassy-time/src/queue_generic.rs | 77 ++++++++++++++------------------------- 1 file changed, 28 insertions(+), 49 deletions(-) (limited to 'embassy-time/src/queue_generic.rs') diff --git a/embassy-time/src/queue_generic.rs b/embassy-time/src/queue_generic.rs index 1c4e5398b..83f734848 100644 --- a/embassy-time/src/queue_generic.rs +++ b/embassy-time/src/queue_generic.rs @@ -2,7 +2,6 @@ use core::cell::RefCell; use core::cmp::Ordering; use core::task::Waker; -use atomic_polyfill::{AtomicU64, Ordering as AtomicOrdering}; use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex; use embassy_sync::blocking_mutex::Mutex; use heapless::sorted_linked_list::{LinkedIndexU8, Min, SortedLinkedList}; @@ -71,7 +70,7 @@ impl InnerQueue { } } - fn schedule_wake(&mut self, at: Instant, waker: &Waker, alarm_schedule: &AtomicU64) { + fn schedule_wake(&mut self, at: Instant, waker: &Waker) { self.queue .find_mut(|timer| timer.waker.will_wake(waker)) .map(|mut timer| { @@ -98,50 +97,54 @@ impl InnerQueue { // dispatch all timers that are already due // // Then update the alarm if necessary - self.dispatch(alarm_schedule); + self.dispatch(); } - fn dispatch(&mut self, alarm_schedule: &AtomicU64) { - let now = Instant::now(); + fn dispatch(&mut self) { + loop { + let now = Instant::now(); - while self.queue.peek().filter(|timer| timer.at <= now).is_some() { - self.queue.pop().unwrap().waker.wake(); - } + while self.queue.peek().filter(|timer| timer.at <= now).is_some() { + self.queue.pop().unwrap().waker.wake(); + } - self.update_alarm(alarm_schedule); + if self.update_alarm() { + break; + } + } } - fn update_alarm(&mut self, alarm_schedule: &AtomicU64) { + fn update_alarm(&mut self) -> bool { if let Some(timer) = self.queue.peek() { let new_at = timer.at; if self.alarm_at != new_at { self.alarm_at = new_at; - alarm_schedule.store(new_at.as_ticks(), AtomicOrdering::SeqCst); + + return set_alarm(self.alarm.unwrap(), self.alarm_at.as_ticks()); } } else { self.alarm_at = Instant::MAX; - alarm_schedule.store(Instant::MAX.as_ticks(), AtomicOrdering::SeqCst); } + + true } - fn handle_alarm(&mut self, alarm_schedule: &AtomicU64) { + fn handle_alarm(&mut self) { self.alarm_at = Instant::MAX; - self.dispatch(alarm_schedule); + self.dispatch(); } } struct Queue { inner: Mutex>, - alarm_schedule: AtomicU64, } impl Queue { const fn new() -> Self { Self { inner: Mutex::new(RefCell::new(InnerQueue::new())), - alarm_schedule: AtomicU64::new(u64::MAX), } } @@ -156,28 +159,12 @@ impl Queue { set_alarm_callback(handle, Self::handle_alarm_callback, self as *const _ as _); } - inner.schedule_wake(at, waker, &self.alarm_schedule) + inner.schedule_wake(at, waker) }); - - self.update_alarm(); - } - - fn update_alarm(&self) { - // Need to set the alarm when we are *not* holding the mutex on the inner queue - // because mutexes are not re-entrant, which is a problem because `set_alarm` might immediately - // call us back if the timestamp is in the past. - let alarm_at = self.alarm_schedule.swap(u64::MAX, AtomicOrdering::SeqCst); - - if alarm_at < u64::MAX { - set_alarm(self.inner.lock(|inner| inner.borrow().alarm.unwrap()), alarm_at); - } } fn handle_alarm(&self) { - self.inner - .lock(|inner| inner.borrow_mut().handle_alarm(&self.alarm_schedule)); - - self.update_alarm(); + self.inner.lock(|inner| inner.borrow_mut().handle_alarm()); } fn handle_alarm_callback(ctx: *mut ()) { @@ -196,7 +183,6 @@ crate::timer_queue_impl!(static QUEUE: Queue = Queue::new()); #[cfg(test)] mod tests { use core::cell::Cell; - use core::sync::atomic::Ordering; use core::task::{RawWaker, RawWakerVTable, Waker}; use std::rc::Rc; use std::sync::Mutex; @@ -282,20 +268,14 @@ mod tests { inner.ctx = ctx; } - fn set_alarm(&self, _alarm: AlarmHandle, timestamp: u64) { - let notify = { - let mut inner = self.0.lock().unwrap(); - - if timestamp <= inner.now { - Some((inner.callback, inner.ctx)) - } else { - inner.alarm = timestamp; - None - } - }; + fn set_alarm(&self, _alarm: AlarmHandle, timestamp: u64) -> bool { + let mut inner = self.0.lock().unwrap(); - if let Some((callback, ctx)) = notify { - (callback)(ctx); + if timestamp <= inner.now { + false + } else { + inner.alarm = timestamp; + true } } } @@ -344,7 +324,6 @@ mod tests { fn setup() { DRIVER.reset(); - QUEUE.alarm_schedule.store(u64::MAX, Ordering::SeqCst); QUEUE.inner.lock(|inner| { *inner.borrow_mut() = InnerQueue::new(); }); -- cgit From ac6995f9e656a724d92590e722ac0c25f417893b Mon Sep 17 00:00:00 2001 From: ivmarkov Date: Wed, 26 Oct 2022 17:48:22 +0300 Subject: Fix a bug identified during code review --- embassy-time/src/queue_generic.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'embassy-time/src/queue_generic.rs') diff --git a/embassy-time/src/queue_generic.rs b/embassy-time/src/queue_generic.rs index 83f734848..6769d6a58 100644 --- a/embassy-time/src/queue_generic.rs +++ b/embassy-time/src/queue_generic.rs @@ -1,5 +1,5 @@ use core::cell::RefCell; -use core::cmp::Ordering; +use core::cmp::{min, Ordering}; use core::task::Waker; use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex; @@ -74,7 +74,7 @@ impl InnerQueue { self.queue .find_mut(|timer| timer.waker.will_wake(waker)) .map(|mut timer| { - timer.at = at; + timer.at = min(timer.at, at); timer.finish(); }) .unwrap_or_else(|| { -- cgit From 4976cbbe6040d5e147e7c42bd29b72d6223b05b0 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 26 Oct 2022 20:02:58 +0200 Subject: time/generic-queue: ensure queue goes in .bss instead of .data --- embassy-time/src/queue_generic.rs | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) (limited to 'embassy-time/src/queue_generic.rs') diff --git a/embassy-time/src/queue_generic.rs b/embassy-time/src/queue_generic.rs index 6769d6a58..8a355b327 100644 --- a/embassy-time/src/queue_generic.rs +++ b/embassy-time/src/queue_generic.rs @@ -57,19 +57,11 @@ impl Ord for Timer { struct InnerQueue { queue: SortedLinkedList, - alarm: Option, + alarm: AlarmHandle, alarm_at: Instant, } impl InnerQueue { - const fn new() -> Self { - Self { - queue: SortedLinkedList::new_u8(), - alarm: None, - alarm_at: Instant::MAX, - } - } - fn schedule_wake(&mut self, at: Instant, waker: &Waker) { self.queue .find_mut(|timer| timer.waker.will_wake(waker)) @@ -121,7 +113,7 @@ impl InnerQueue { if self.alarm_at != new_at { self.alarm_at = new_at; - return set_alarm(self.alarm.unwrap(), self.alarm_at.as_ticks()); + return set_alarm(self.alarm, self.alarm_at.as_ticks()); } } else { self.alarm_at = Instant::MAX; @@ -138,13 +130,13 @@ impl InnerQueue { } struct Queue { - inner: Mutex>, + inner: Mutex>>, } impl Queue { const fn new() -> Self { Self { - inner: Mutex::new(RefCell::new(InnerQueue::new())), + inner: Mutex::new(RefCell::new(None)), } } @@ -152,19 +144,25 @@ impl Queue { self.inner.lock(|inner| { let mut inner = inner.borrow_mut(); - if inner.alarm.is_none() { - let handle = unsafe { allocate_alarm() }.unwrap(); - inner.alarm = Some(handle); - - set_alarm_callback(handle, Self::handle_alarm_callback, self as *const _ as _); - } + if inner.is_none() {} - inner.schedule_wake(at, waker) + inner + .get_or_insert_with(|| { + let handle = unsafe { allocate_alarm() }.unwrap(); + set_alarm_callback(handle, Self::handle_alarm_callback, self as *const _ as _); + InnerQueue { + queue: SortedLinkedList::new_u8(), + alarm: handle, + alarm_at: Instant::MAX, + } + }) + .schedule_wake(at, waker) }); } fn handle_alarm(&self) { - self.inner.lock(|inner| inner.borrow_mut().handle_alarm()); + self.inner + .lock(|inner| inner.borrow_mut().as_mut().unwrap().handle_alarm()); } fn handle_alarm_callback(ctx: *mut ()) { -- cgit From f9da6271cea7035b2c9f27cfe479aa81889168d1 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 26 Oct 2022 21:00:50 +0200 Subject: time/generic_queue: use Vec instead of SortedLinkedList --- embassy-time/src/queue_generic.rs | 46 +++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 24 deletions(-) (limited to 'embassy-time/src/queue_generic.rs') diff --git a/embassy-time/src/queue_generic.rs b/embassy-time/src/queue_generic.rs index 8a355b327..20ae7e6cc 100644 --- a/embassy-time/src/queue_generic.rs +++ b/embassy-time/src/queue_generic.rs @@ -4,7 +4,7 @@ use core::task::Waker; use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex; use embassy_sync::blocking_mutex::Mutex; -use heapless::sorted_linked_list::{LinkedIndexU8, Min, SortedLinkedList}; +use heapless::Vec; use crate::driver::{allocate_alarm, set_alarm, set_alarm_callback, AlarmHandle}; use crate::queue::TimerQueue; @@ -56,18 +56,17 @@ impl Ord for Timer { } struct InnerQueue { - queue: SortedLinkedList, + queue: Vec, alarm: AlarmHandle, - alarm_at: Instant, } impl InnerQueue { fn schedule_wake(&mut self, at: Instant, waker: &Waker) { self.queue - .find_mut(|timer| timer.waker.will_wake(waker)) + .iter_mut() + .find(|timer| timer.waker.will_wake(waker)) .map(|mut timer| { timer.at = min(timer.at, at); - timer.finish(); }) .unwrap_or_else(|| { let mut timer = Timer { @@ -96,35 +95,35 @@ impl InnerQueue { loop { let now = Instant::now(); - while self.queue.peek().filter(|timer| timer.at <= now).is_some() { - self.queue.pop().unwrap().waker.wake(); + let mut next_alarm = Instant::MAX; + + let mut i = 0; + while i < self.queue.len() { + let timer = &self.queue[i]; + if timer.at <= now { + let timer = self.queue.swap_remove(i); + timer.waker.wake(); + } else { + next_alarm = min(next_alarm, timer.at); + i += 1; + } } - if self.update_alarm() { + if self.update_alarm(next_alarm) { break; } } } - fn update_alarm(&mut self) -> bool { - if let Some(timer) = self.queue.peek() { - let new_at = timer.at; - - if self.alarm_at != new_at { - self.alarm_at = new_at; - - return set_alarm(self.alarm, self.alarm_at.as_ticks()); - } + fn update_alarm(&mut self, next_alarm: Instant) -> bool { + if next_alarm == Instant::MAX { + true } else { - self.alarm_at = Instant::MAX; + set_alarm(self.alarm, next_alarm.as_ticks()) } - - true } fn handle_alarm(&mut self) { - self.alarm_at = Instant::MAX; - self.dispatch(); } } @@ -151,9 +150,8 @@ impl Queue { let handle = unsafe { allocate_alarm() }.unwrap(); set_alarm_callback(handle, Self::handle_alarm_callback, self as *const _ as _); InnerQueue { - queue: SortedLinkedList::new_u8(), + queue: Vec::new(), alarm: handle, - alarm_at: Instant::MAX, } }) .schedule_wake(at, waker) -- cgit From e7ff759f1ca78a5b53c1ea95c24a6317227dc8b0 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 13 Apr 2023 23:50:41 +0200 Subject: time: remove dependency on embassy-sync. --- embassy-time/src/queue_generic.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'embassy-time/src/queue_generic.rs') diff --git a/embassy-time/src/queue_generic.rs b/embassy-time/src/queue_generic.rs index 20ae7e6cc..0f67d9dbb 100644 --- a/embassy-time/src/queue_generic.rs +++ b/embassy-time/src/queue_generic.rs @@ -2,8 +2,7 @@ use core::cell::RefCell; use core::cmp::{min, Ordering}; use core::task::Waker; -use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex; -use embassy_sync::blocking_mutex::Mutex; +use critical_section::Mutex; use heapless::Vec; use crate::driver::{allocate_alarm, set_alarm, set_alarm_callback, AlarmHandle}; @@ -129,7 +128,7 @@ impl InnerQueue { } struct Queue { - inner: Mutex>>, + inner: Mutex>>, } impl Queue { @@ -140,8 +139,8 @@ impl Queue { } fn schedule_wake(&'static self, at: Instant, waker: &Waker) { - self.inner.lock(|inner| { - let mut inner = inner.borrow_mut(); + critical_section::with(|cs| { + let mut inner = self.inner.borrow_ref_mut(cs); if inner.is_none() {} @@ -159,8 +158,7 @@ impl Queue { } fn handle_alarm(&self) { - self.inner - .lock(|inner| inner.borrow_mut().as_mut().unwrap().handle_alarm()); + critical_section::with(|cs| self.inner.borrow_ref_mut(cs).as_mut().unwrap().handle_alarm()) } fn handle_alarm_callback(ctx: *mut ()) { -- cgit From df56f901de7aaf5fe77421a3087f64af7b7fc961 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Fri, 19 May 2023 17:38:57 +0200 Subject: time: fix unused mut. --- embassy-time/src/queue_generic.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'embassy-time/src/queue_generic.rs') diff --git a/embassy-time/src/queue_generic.rs b/embassy-time/src/queue_generic.rs index 0f67d9dbb..64a8af4bc 100644 --- a/embassy-time/src/queue_generic.rs +++ b/embassy-time/src/queue_generic.rs @@ -64,7 +64,7 @@ impl InnerQueue { self.queue .iter_mut() .find(|timer| timer.waker.will_wake(waker)) - .map(|mut timer| { + .map(|timer| { timer.at = min(timer.at, at); }) .unwrap_or_else(|| { -- cgit From 46961cfdf72a3b5d54b241a41d9f2496c6dc6229 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 29 May 2023 19:46:28 +0200 Subject: Fix tests. --- embassy-time/src/queue_generic.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'embassy-time/src/queue_generic.rs') diff --git a/embassy-time/src/queue_generic.rs b/embassy-time/src/queue_generic.rs index 64a8af4bc..4795eb2f3 100644 --- a/embassy-time/src/queue_generic.rs +++ b/embassy-time/src/queue_generic.rs @@ -183,7 +183,6 @@ mod tests { use serial_test::serial; - use super::InnerQueue; use crate::driver::{AlarmHandle, Driver}; use crate::queue_generic::QUEUE; use crate::Instant; @@ -317,14 +316,18 @@ mod tests { fn setup() { DRIVER.reset(); - - QUEUE.inner.lock(|inner| { - *inner.borrow_mut() = InnerQueue::new(); - }); + critical_section::with(|cs| *QUEUE.inner.borrow_ref_mut(cs) = None); } fn queue_len() -> usize { - QUEUE.inner.lock(|inner| inner.borrow().queue.iter().count()) + critical_section::with(|cs| { + QUEUE + .inner + .borrow_ref(cs) + .as_ref() + .map(|inner| inner.queue.iter().count()) + .unwrap_or(0) + }) } #[test] -- cgit From 40d25da793bd18528f3555d0a9205986018d01f0 Mon Sep 17 00:00:00 2001 From: cumthugo Date: Tue, 4 Jul 2023 21:13:31 +0800 Subject: time: fix queue size --- embassy-time/src/queue_generic.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'embassy-time/src/queue_generic.rs') diff --git a/embassy-time/src/queue_generic.rs b/embassy-time/src/queue_generic.rs index 4795eb2f3..77947ab29 100644 --- a/embassy-time/src/queue_generic.rs +++ b/embassy-time/src/queue_generic.rs @@ -16,7 +16,7 @@ const QUEUE_SIZE: usize = 16; #[cfg(feature = "generic-queue-32")] const QUEUE_SIZE: usize = 32; #[cfg(feature = "generic-queue-64")] -const QUEUE_SIZE: usize = 32; +const QUEUE_SIZE: usize = 64; #[cfg(feature = "generic-queue-128")] const QUEUE_SIZE: usize = 128; #[cfg(not(any( -- cgit