diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index 29a0d4d9..174e3928 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -109,10 +109,11 @@ impl<'hw> Timers<'hw> { struct ServiceInner<'hw> { clock_state: Mutex>>, - // TODO [POWER_SOURCE] signal this whenever the power source changes - power_source_signal: Signal, + wake_signal: Signal, timers: Timers<'hw>, + // Keep both timers' updates and publication of their combined wake level atomic. + timer_update_lock: Mutex, capabilities: TimeAlarmDeviceCapabilities, } @@ -126,18 +127,21 @@ impl<'hw> ServiceInner<'hw> { dc_expiration_storage: &'hw mut dyn NvramStorage<'hw, u32>, dc_policy_storage: &'hw mut dyn NvramStorage<'hw, u32>, ) -> Self { + let wake_signal = Signal::new(); + wake_signal.signal(false); Self { clock_state: Mutex::new(RefCell::new(ClockState { datetime_clock: backing_clock, tz_data: TimeZoneData::new(tz_storage), })), - power_source_signal: Signal::new(), + wake_signal, timers: Timers::new( ac_expiration_storage, ac_policy_storage, dc_expiration_storage, dc_policy_storage, ), + timer_update_lock: Mutex::new(()), capabilities: { // TODO [CONFIG] We could consider making some of these user-configurable, e.g. if we want to support devices that don't have a battery let mut caps = TimeAlarmDeviceCapabilities(0); @@ -191,7 +195,10 @@ impl<'hw> ServiceInner<'hw> { /// Clear the current wake status. Analogous to ACPI TAD's _CWS method. fn clear_wake_status(&self, timer_id: AcpiTimerId) { - self.timers.get_timer(timer_id).clear_wake_status(); + self.timer_update_lock.lock(|_| { + self.timers.get_timer(timer_id).clear_wake_status(); + self.publish_wake_signal(); + }); } /// Configures behavior when the timer expires while the system is on the other power source. Analogous to ACPI TAD's _STP method. @@ -226,10 +233,14 @@ impl<'hw> ServiceInner<'hw> { } }; - self.timers - .get_timer(timer_id) - .set_expiration_time(&self.clock_state, new_expiration_time)?; - Ok(()) + self.timer_update_lock.lock(|_| { + let result = self + .timers + .get_timer(timer_id) + .set_expiration_time(&self.clock_state, new_expiration_time); + self.publish_wake_signal(); + result + }) } /// Query the expiry time for the given timer. Analogous to ACPI TAD's _TIV method. @@ -251,39 +262,44 @@ impl<'hw> ServiceInner<'hw> { } } - async fn handle_power_source_updates(&'hw self) -> ! { - loop { - let new_power_source = self.power_source_signal.wait().await; - info!("[Time/Alarm] Power source changed to {:?}", new_power_source); - + fn set_power_source(&self, new_power_source: AcpiTimerId) { + self.timer_update_lock.lock(|_| { self.timers .get_timer(new_power_source.get_other_timer_id()) .set_active(&self.clock_state, false); self.timers .get_timer(new_power_source) .set_active(&self.clock_state, true); - } + }); + } + + // Called with timer_update_lock held, after releasing individual TimerState borrows. + fn publish_wake_signal(&self) { + self.wake_signal.signal( + self.timers.ac_timer.get_wake_status().timer_triggered_wake() + || self.timers.dc_timer.get_wake_status().timer_triggered_wake(), + ); } async fn handle_timer(&'hw self, timer_id: AcpiTimerId) -> ! { let timer = self.timers.get_timer(timer_id); loop { - timer.wait_until_wake(&self.clock_state).await; - self.timers - .get_timer(timer_id.get_other_timer_id()) - .set_timer_wake_policy(&self.clock_state, AlarmExpiredWakePolicy::NEVER) - .unwrap_or_else(|e| { - warn!( - "[Time/Alarm] Failed to update wake policy on timer expiry - this should never happen: {:?}", - e - ); - }); - - warn!( - "[Time/Alarm] Timer {:?} expired and would trigger a wake now, but the power service is not yet implemented so will currently do nothing", - timer_id - ); - // TODO [COMMS] We can't currently trigger a wake because the power service isn't implemented yet - when it is, we need to notify it here + timer.wait_for_expiry().await; + self.timer_update_lock.lock(|_| { + if timer.process_expired_timer(&self.clock_state) { + self.timers + .get_timer(timer_id.get_other_timer_id()) + .set_timer_wake_policy(&self.clock_state, AlarmExpiredWakePolicy::NEVER) + .unwrap_or_else(|e| { + warn!( + "[Time/Alarm] Failed to update wake policy on timer expiry - this should never happen: {:?}", + e + ); + }); + self.publish_wake_signal(); + info!("[Time/Alarm] Timer {:?} requested a wake", timer_id); + } + }); } } } @@ -303,8 +319,7 @@ impl<'hw> odp_service_common::runnable_service::ServiceRunner<'hw> for Runner<'h /// Run the service. async fn run(self) -> embedded_services::Never { loop { - embassy_futures::select::select3( - self.service.handle_power_source_updates(), + embassy_futures::select::select( self.service.handle_timer(AcpiTimerId::AcPower), self.service.handle_timer(AcpiTimerId::DcPower), ) @@ -375,7 +390,21 @@ impl<'hw> odp_service_common::runnable_service::Service<'hw> for Service<'hw> { } impl<'hw> Service<'hw> { + /// Selects the active power source synchronously, preserving every transition. + /// Boards with source information should call this before running the runner. + pub fn set_power_source(&self, power_source: AcpiTimerId) { + self.inner.set_power_source(power_source); + } + + /// Waits for the latest wake-request level, initially false, for one board consumer. + /// The level is the OR of both triggered-wake latches; pending updates coalesce. + /// Consuming a level neither clears status nor acknowledges a physical host resume. + pub async fn wait_for_wake_signal(&self) -> bool { + self.inner.wake_signal.wait().await + } + /// Initializes an instance of the time-alarm service. + /// AC is initially active; use [`Self::set_power_source`] to select another source. pub async fn new( service_storage: &'hw mut Resources<'hw>, backing_clock: &'hw mut dyn DatetimeClock, @@ -394,8 +423,6 @@ impl<'hw> Service<'hw> { dc_policy_storage, )); - // TODO [POWER_SOURCE] we need to subscribe to messages that tell us if we're on AC or DC power so we can decide which alarms to trigger, but those notifications are not yet implemented - revisit when they are. - // TODO [POWER_SOURCE] if it's possible to learn which power source is active at init time, we should set that one active rather than defaulting to the AC timer. service.timers.ac_timer.start(&service.clock_state, true)?; service.timers.dc_timer.start(&service.clock_state, false)?; diff --git a/time-alarm-service/src/timer.rs b/time-alarm-service/src/timer.rs index 27034e5e..4bc80c26 100644 --- a/time-alarm-service/src/timer.rs +++ b/time-alarm-service/src/timer.rs @@ -6,6 +6,9 @@ use embedded_mcu_hal::nvram::NvramStorage; use embedded_mcu_hal::time::{Datetime, DatetimeClockError}; use embedded_services::{GlobalRawMutex, error}; +#[cfg(test)] +mod tests; + /// Represents where in the timer lifecycle the current timer is #[derive(Copy, Clone, Debug, PartialEq)] enum WakeState { @@ -274,7 +277,7 @@ impl<'hw> Timer<'hw> { }); } - pub(crate) async fn wait_until_wake(&self, clock_state: &Mutex>>) { + pub(crate) async fn wait_for_expiry(&self) { loop { let mut wait_duration: Option = self.timer_signal.wait().await; 'waiting_for_timer: loop { @@ -286,11 +289,7 @@ impl<'hw> Timer<'hw> { ) .await { - Either::First(()) => { - if self.process_expired_timer(clock_state) { - return; - } - } + Either::First(()) => return, Either::Second(new_wait_duration) => { wait_duration = new_wait_duration; } @@ -307,8 +306,12 @@ impl<'hw> Timer<'hw> { /// Handles state changes for when the timer expires (figuring out what to do based on the current power source, etc). /// Returns true if the timer's expiry indicates that a wake event should be signaled to the host. - fn process_expired_timer(&self, clock_state: &Mutex>>) -> bool { + pub(crate) fn process_expired_timer(&self, clock_state: &Mutex>>) -> bool { self.timer_state.lock(|timer_state| { + // A pending command supersedes the deadline that just completed. + if self.timer_signal.signaled() { + return false; + } let mut timer_state = timer_state.borrow_mut(); match timer_state.wake_state { diff --git a/time-alarm-service/src/timer/tests.rs b/time-alarm-service/src/timer/tests.rs new file mode 100644 index 00000000..8b758131 --- /dev/null +++ b/time-alarm-service/src/timer/tests.rs @@ -0,0 +1,361 @@ +#![allow(clippy::unwrap_used)] + +use core::future::Future; +use core::pin::{Pin, pin}; +use core::task::{Context, Poll, Waker}; +use std::sync::Barrier; + +use embassy_futures::block_on; +use embedded_mcu_hal::time::Datetime; +use odp_service_common::runnable_service::ServiceRunner; + +use crate::mock::{MockDatetimeClock, MockNvramStorage}; +use crate::{AcpiTimerId, AlarmExpiredWakePolicy, AlarmTimerSeconds, Runner, Service, TimeAlarmService}; + +const SOURCES: [AcpiTimerId; 2] = [AcpiTimerId::AcPower, AcpiTimerId::DcPower]; + +fn with_service(test: impl FnOnce(Service<'_>, Runner<'_>)) { + let mut clock = MockDatetimeClock::Paused { + frozen_time: Datetime::from_unix_timestamp(1_700_000_000), + }; + let mut tz = MockNvramStorage::new(0); + let mut ac_expiry = MockNvramStorage::new(u32::MAX); + let mut ac_policy = MockNvramStorage::new(u32::MAX); + let mut dc_expiry = MockNvramStorage::new(u32::MAX); + let mut dc_policy = MockNvramStorage::new(u32::MAX); + let mut resources = Default::default(); + let (service, runner) = block_on(Service::new( + &mut resources, + &mut clock, + &mut tz, + &mut ac_expiry, + &mut ac_policy, + &mut dc_expiry, + &mut dc_policy, + )) + .unwrap(); + test(service, runner); +} + +fn poll_once(future: Pin<&mut F>) -> Poll { + future.poll(&mut Context::from_waker(Waker::noop())) +} + +fn poll_runner(mut runner: Pin<&mut impl Future>) { + // Embassy timers yield once even for a zero-second deadline. + assert!(poll_once(runner.as_mut()).is_pending()); + assert!(poll_once(runner).is_pending()); +} + +fn wake_level(service: Service<'_>) -> Poll { + poll_once(pin!(service.wait_for_wake_signal())) +} + +fn advance_clock(service: Service<'_>, seconds: u64) { + let mut timestamp = service.get_real_time().unwrap(); + timestamp.datetime = Datetime::from_unix_timestamp(timestamp.datetime.unix_timestamp() + seconds); + service.set_real_time(timestamp).unwrap(); +} + +fn deliver_deadline(service: Service<'_>, timer_id: AcpiTimerId, runner: Pin<&mut impl Future>) { + // Deliver an elapsed deadline after advancing the controlled RTC, without a wall-clock sleep. + service.inner.timers.get_timer(timer_id).timer_signal.signal(Some(0)); + poll_runner(runner); +} + +fn latch_both(service: Service<'_>, mut runner: Pin<&mut impl Future>) { + for source in SOURCES { + service.set_power_source(source); + service.set_timer_value(source, AlarmTimerSeconds(0)).unwrap(); + poll_runner(runner.as_mut()); + assert!(service.get_wake_status(source).timer_triggered_wake()); + } +} + +#[test] +fn initial_level_is_false_and_default_source_is_ac() { + with_service(|service, runner| { + assert_eq!(wake_level(service), Poll::Ready(false)); + assert_eq!(wake_level(service), Poll::Pending); + service + .set_timer_value(AcpiTimerId::AcPower, AlarmTimerSeconds(0)) + .unwrap(); + let mut runner = pin!(runner.run()); + poll_runner(runner.as_mut()); + assert_eq!(wake_level(service), Poll::Ready(true)); + assert_eq!(wake_level(service), Poll::Pending); + assert!(service.get_wake_status(AcpiTimerId::AcPower).timer_triggered_wake()); + }); +} + +#[test] +fn either_source_can_be_selected_before_running() { + for source in SOURCES { + with_service(|service, runner| { + service.set_power_source(source); + for timer_id in SOURCES { + service + .set_expired_timer_policy(timer_id, AlarmExpiredWakePolicy(10)) + .unwrap(); + } + service.set_timer_value(source, AlarmTimerSeconds(0)).unwrap(); + assert_eq!(wake_level(service), Poll::Ready(false)); + let mut runner = pin!(runner.run()); + poll_runner(runner.as_mut()); + assert_eq!(wake_level(service), Poll::Ready(true)); + let status = service.get_wake_status(source); + assert!(status.timer_expired()); + assert!(status.timer_triggered_wake()); + assert_eq!(service.get_timer_value(source).unwrap(), AlarmTimerSeconds::DISABLED); + for timer_id in SOURCES { + assert_eq!( + service.get_expired_timer_policy(timer_id), + AlarmExpiredWakePolicy::NEVER + ); + } + }); + } +} + +#[test] +fn inactive_expiry_waits_for_matching_source() { + for source in SOURCES { + with_service(|service, runner| { + service.set_power_source(source.get_other_timer_id()); + service + .set_expired_timer_policy(source, AlarmExpiredWakePolicy::INSTANTLY) + .unwrap(); + service.set_timer_value(source, AlarmTimerSeconds(0)).unwrap(); + assert_eq!(wake_level(service), Poll::Ready(false)); + let mut runner = pin!(runner.run()); + poll_runner(runner.as_mut()); + assert!(service.get_wake_status(source).timer_expired()); + assert!(!service.get_wake_status(source).timer_triggered_wake()); + assert_eq!(wake_level(service), Poll::Pending); + service.set_power_source(source); + poll_runner(runner.as_mut()); + assert_eq!(wake_level(service), Poll::Ready(true)); + }); + } +} + +#[test] +fn never_orphans_only_an_inactive_expiry() { + for source in SOURCES { + with_service(|service, runner| { + service.set_power_source(source.get_other_timer_id()); + service.set_timer_value(source, AlarmTimerSeconds(0)).unwrap(); + assert_eq!(wake_level(service), Poll::Ready(false)); + let mut runner = pin!(runner.run()); + poll_runner(runner.as_mut()); + assert!(service.get_wake_status(source).timer_expired()); + assert!(!service.get_wake_status(source).timer_triggered_wake()); + service.set_power_source(source); + advance_clock(service, 100); + poll_runner(runner.as_mut()); + assert_eq!(wake_level(service), Poll::Pending); + service + .set_expired_timer_policy(source, AlarmExpiredWakePolicy::INSTANTLY) + .unwrap(); + poll_runner(runner.as_mut()); + assert_eq!(wake_level(service), Poll::Pending); + + service + .set_expired_timer_policy(source, AlarmExpiredWakePolicy::NEVER) + .unwrap(); + service.set_timer_value(source, AlarmTimerSeconds(0)).unwrap(); + poll_runner(runner.as_mut()); + assert_eq!(wake_level(service), Poll::Ready(true)); + }); + } +} + +#[test] +fn policy_delay_preserves_every_unpolled_source_transition() { + for source in SOURCES { + with_service(|service, runner| { + let other = source.get_other_timer_id(); + service.set_power_source(other); + service + .set_expired_timer_policy(source, AlarmExpiredWakePolicy(10)) + .unwrap(); + service.set_timer_value(source, AlarmTimerSeconds(0)).unwrap(); + assert_eq!(wake_level(service), Poll::Ready(false)); + let mut runner = pin!(runner.run()); + poll_runner(runner.as_mut()); + + service.set_power_source(source); + advance_clock(service, 3); + service.set_power_source(other); + advance_clock(service, 100); + service.set_power_source(source); + advance_clock(service, 2); + service.set_power_source(other); + advance_clock(service, 100); + service.set_power_source(source); + let signal = &service.inner.timers.get_timer(source).timer_signal; + assert_eq!(signal.try_take(), Some(Some(5))); + signal.signal(Some(5)); + poll_runner(runner.as_mut()); + assert_eq!(wake_level(service), Poll::Pending); + assert!(!service.get_wake_status(source).timer_triggered_wake()); + + advance_clock(service, 5); + deliver_deadline(service, source, runner.as_mut()); + assert_eq!(wake_level(service), Poll::Ready(true)); + }); + } +} + +#[test] +fn stale_expiry_does_not_skip_a_restarted_policy_delay() { + with_service(|service, runner| { + let source = AcpiTimerId::DcPower; + service + .set_expired_timer_policy(source, AlarmExpiredWakePolicy(10)) + .unwrap(); + service.set_timer_value(source, AlarmTimerSeconds(0)).unwrap(); + assert_eq!(wake_level(service), Poll::Ready(false)); + let mut runner = pin!(runner.run()); + poll_runner(runner.as_mut()); + service.set_power_source(source); + advance_clock(service, 3); + service.set_power_source(AcpiTimerId::AcPower); + advance_clock(service, 7); + service.set_power_source(source); + + // The old deadline completed before a source change, but has not processed it yet. + let timer = service.inner.timers.get_timer(source); + assert!(!timer.process_expired_timer(&service.inner.clock_state)); + assert_eq!(timer.timer_signal.try_take(), Some(Some(7))); + assert!(!service.get_wake_status(source).timer_triggered_wake()); + assert_eq!(wake_level(service), Poll::Pending); + advance_clock(service, 7); + deliver_deadline(service, source, runner.as_mut()); + assert_eq!(wake_level(service), Poll::Ready(true)); + }); +} + +#[test] +fn both_latches_are_ored_and_source_changes_do_not_acknowledge() { + for first in SOURCES { + with_service(|service, runner| { + let mut runner = pin!(runner.run()); + latch_both(service, runner.as_mut()); + assert_eq!(wake_level(service), Poll::Ready(true)); + for source in SOURCES { + service.set_power_source(source); + assert!(service.get_wake_status(source).timer_triggered_wake()); + } + assert_eq!(wake_level(service), Poll::Pending); + service.clear_wake_status(first); + assert_eq!(wake_level(service), Poll::Ready(true)); + service.clear_wake_status(first.get_other_timer_id()); + assert_eq!(wake_level(service), Poll::Ready(false)); + }); + } +} + +#[test] +fn clear_rearm_reprogram_and_disable_update_the_level() { + for source in SOURCES { + with_service(|service, runner| { + service.set_power_source(source); + let mut runner = pin!(runner.run()); + for _ in 0..2 { + service.set_timer_value(source, AlarmTimerSeconds(0)).unwrap(); + assert_eq!(wake_level(service), Poll::Ready(false)); + poll_runner(runner.as_mut()); + assert_eq!(wake_level(service), Poll::Ready(true)); + service.clear_wake_status(source); + assert_eq!(wake_level(service), Poll::Ready(false)); + assert_eq!(service.get_wake_status(source), Default::default()); + } + for new_value in [AlarmTimerSeconds(30), AlarmTimerSeconds::DISABLED] { + service.set_timer_value(source, AlarmTimerSeconds(0)).unwrap(); + poll_runner(runner.as_mut()); + assert_eq!(wake_level(service), Poll::Ready(true)); + service.set_timer_value(source, new_value).unwrap(); + assert_eq!(wake_level(service), Poll::Ready(false)); + assert_eq!(service.get_wake_status(source), Default::default()); + assert_eq!(service.get_timer_value(source).unwrap(), new_value); + poll_runner(runner.as_mut()); + assert_eq!(wake_level(service), Poll::Pending); + } + }); + } +} + +#[test] +fn reprogram_and_disable_preserve_the_other_latch() { + with_service(|service, runner| { + let mut runner = pin!(runner.run()); + latch_both(service, runner.as_mut()); + service + .set_timer_value(AcpiTimerId::AcPower, AlarmTimerSeconds(30)) + .unwrap(); + assert_eq!(wake_level(service), Poll::Ready(true)); + service + .set_timer_value(AcpiTimerId::DcPower, AlarmTimerSeconds::DISABLED) + .unwrap(); + assert_eq!(wake_level(service), Poll::Ready(false)); + }); +} + +#[test] +fn pending_publications_coalesce_to_the_latest_level() { + with_service(|service, runner| { + let source = AcpiTimerId::AcPower; + let mut runner = pin!(runner.run()); + service.set_timer_value(source, AlarmTimerSeconds(0)).unwrap(); + poll_runner(runner.as_mut()); + service.clear_wake_status(source); + assert_eq!(wake_level(service), Poll::Ready(false)); + assert_eq!(wake_level(service), Poll::Pending); + + service.set_timer_value(source, AlarmTimerSeconds(0)).unwrap(); + poll_runner(runner.as_mut()); + service.clear_wake_status(source); + service.set_timer_value(source, AlarmTimerSeconds(0)).unwrap(); + poll_runner(runner.as_mut()); + assert_eq!(wake_level(service), Poll::Ready(true)); + assert!(service.get_wake_status(source).timer_triggered_wake()); + }); +} + +#[test] +fn concurrent_clears_cannot_leave_a_stale_asserted_level() { + struct SharedService<'hw>(Service<'hw>); + + // SAFETY: This fixture's clock and NVRAM are Send, and all accesses use GlobalRawMutex. + unsafe impl Sync for SharedService<'_> {} + + impl SharedService<'_> { + fn clear(&self, source: AcpiTimerId) { + self.0.clear_wake_status(source); + } + } + + with_service(|service, runner| { + let shared = SharedService(service); + let barrier = Barrier::new(2); + let mut runner = pin!(runner.run()); + for _ in 0..128 { + latch_both(service, runner.as_mut()); + std::thread::scope(|scope| { + for source in SOURCES { + let shared = &shared; + let barrier = &barrier; + scope.spawn(move || { + barrier.wait(); + shared.clear(source); + }); + } + }); + for source in SOURCES { + assert_eq!(service.get_wake_status(source), Default::default()); + } + assert_eq!(wake_level(service), Poll::Ready(false)); + } + }); +}