[DRAFT] [Time/Alarm] Split interface and relay logic

Cargo.toml

@@ -17,7 +17,8 @@ members = [
     "power-button-service",
     "power-policy-service",
     "time-alarm-service",
-    "time-alarm-service-messages",
+    "time-alarm-service-interface",
+    "time-alarm-service-relay",
     "type-c-service",
     "debug-service",
     "debug-service-messages",
@@ -104,7 +105,8 @@ serde = { version = "1.0.*", default-features = false }
 static_cell = "2.1.0"
 toml = { version = "0.8", default-features = false }
 thermal-service-messages = { path = "./thermal-service-messages" }
-time-alarm-service-messages = { path = "./time-alarm-service-messages" }
+time-alarm-service-interface = { path = "./time-alarm-service-interface" }
+time-alarm-service-relay = { path = "./time-alarm-service-relay" }
 syn = "2.0"
 tps6699x = { git = "https://github.com/OpenDevicePartnership/tps6699x" }
 tokio = { version = "1.42.0" }

embedded-service/src/relay/mod.rs

@@ -125,7 +125,7 @@ pub mod mctp {
         fn process_request<'a>(
             &'a self,
             request: Self::RequestType,
-        ) -> impl core::future::Future<Output = Self::ResultType> + Send + 'a;
+        ) -> impl core::future::Future<Output = Self::ResultType> + 'a;
     }
 
     // Traits below this point are intended for consumption by relay services (e.g. the eSPI service), not individual services that want their messages relayed.
@@ -168,7 +168,7 @@ pub mod mctp {
         fn process_request<'a>(
             &'a self,
             message: Self::RequestEnumType,
-        ) -> impl core::future::Future<Output = Self::ResultEnumType> + Send + 'a;
+        ) -> impl core::future::Future<Output = Self::ResultEnumType> + 'a;
     }
 
     /// This macro generates a relay type over a collection of message types, which can be used by a relay service to
@@ -195,8 +195,8 @@ pub mod mctp {
     /// ```ignore
     ///
     ///     impl_odp_mctp_relay_handler!(
-    ///         MyRelayHanderType;
-    ///         Battery,   0x9, battery_service::Service<'static>;
+    ///         MyRelayHandlerType;
+    ///         Battery,   0x9, battery_service::Service<'static>; // TODO update example to reflect new expectation for service handler
     ///         TimeAlarm, 0xB, time_alarm_service::Service<'static>;
     ///     );
     ///
@@ -210,6 +210,7 @@ pub mod mctp {
     macro_rules! impl_odp_mctp_relay_handler {
         (
             $relay_type_name:ident;
+            // TODO I think we may need to take a lifetime as an argument here if we want to allow individual handler types to have their generic lifetime parameters be a non-'static value, which is probably valuable for testing
             $(
                 $service_name:ident,
                 $service_id:expr,
@@ -448,16 +449,16 @@ pub mod mctp {
                     }
 
 
-                    pub struct $relay_type_name<'hw> {
+                    pub struct $relay_type_name {
                         $(
-                            [<$service_name:snake _handler>]: &'hw $service_handler_type,
+                            [<$service_name:snake _handler>]: $service_handler_type,
                         )+
                     }
 
-                    impl<'hw> $relay_type_name<'hw> {
+                    impl $relay_type_name {
                         pub fn new(
                             $(
-                                [<$service_name:snake _handler>]: &'hw $service_handler_type,
+                                [<$service_name:snake _handler>]: $service_handler_type,
                             )+
                         ) -> Self {
                             Self {
@@ -468,7 +469,7 @@ pub mod mctp {
                         }
                     }
 
-                    impl<'hw> $crate::relay::mctp::RelayHandler for $relay_type_name<'hw> {
+                    impl $crate::relay::mctp::RelayHandler for $relay_type_name {
                         type ServiceIdType = OdpService;
                         type HeaderType = OdpHeader;
                         type RequestEnumType = HostRequest;
@@ -477,7 +478,7 @@ pub mod mctp {
                         fn process_request<'a>(
                             &'a self,
                             message: HostRequest,
-                        ) -> impl core::future::Future<Output = HostResult> + Send + 'a {
+                        ) -> impl core::future::Future<Output = HostResult> + 'a {
                             async move {
                                 match message {
                                     $(

examples/rt685s-evk/Cargo.toml

@@ -70,7 +70,10 @@ embedded-usb-pd = { git = "https://github.com/OpenDevicePartnership/embedded-usb
 ] }
 type-c-service = { path = "../../type-c-service", features = ["defmt"] }
 time-alarm-service = { path = "../../time-alarm-service", features = ["defmt"] }
-time-alarm-service-messages = { path = "../../time-alarm-service-messages", features = [
+time-alarm-service-interface = { path = "../../time-alarm-service-interface", features = [
+    "defmt",
+] }
+time-alarm-service-relay = { path = "../../time-alarm-service-relay", features = [
     "defmt",
 ] }
 

examples/rt685s-evk/src/bin/time_alarm.rs

@@ -7,9 +7,17 @@ use embedded_mcu_hal::{
 };
 use embedded_services::info;
 use static_cell::StaticCell;
-use time_alarm_service_messages::{AcpiDaylightSavingsTimeStatus, AcpiTimeZone, AcpiTimeZoneOffset, AcpiTimestamp};
+use time_alarm_service_interface::{
+    AcpiDaylightSavingsTimeStatus, AcpiTimeZone, AcpiTimeZoneOffset, AcpiTimestamp, TimeAlarmService,
+};
 use {defmt_rtt as _, panic_probe as _};
 
+// Type aliases to make it easier to use the service and relay handler types without needing to write out all the generic parameters every time.
+// This is especially helpful for the relay handler, which has a lot of generic parameters due to the traits it needs to implement.
+//
+type TimeAlarmServiceType = time_alarm_service::Service<'static>;
+type TimeAlarmServiceRelayHandlerType = time_alarm_service_relay::TimeAlarmServiceRelayHandler<TimeAlarmServiceType>;
+
 #[embassy_executor::main]
 async fn main(spawner: embassy_executor::Spawner) {
     let p = embassy_imxrt::init(Default::default());
@@ -25,7 +33,7 @@ async fn main(spawner: embassy_executor::Spawner) {
 
     let time_service = odp_service_common::spawn_service!(
         spawner,
-        time_alarm_service::Service<'static>,
+        TimeAlarmServiceType,
         time_alarm_service::InitParams {
             backing_clock: dt_clock,
             tz_storage: tz,
@@ -40,10 +48,10 @@ async fn main(spawner: embassy_executor::Spawner) {
     use embedded_services::relay::mctp::impl_odp_mctp_relay_handler;
     impl_odp_mctp_relay_handler!(
         EspiRelayHandler;
-        TimeAlarm, 0x0B, time_alarm_service::Service<'static>;
+        TimeAlarm, 0x0B, crate::TimeAlarmServiceRelayHandlerType;
     );
 
-    let _relay_handler = EspiRelayHandler::new(&time_service);
+    let _relay_handler = EspiRelayHandler::new(TimeAlarmServiceRelayHandlerType::new(time_service));
 
     // Here, you'd normally pass _relay_handler to your relay service (e.g. eSPI service).
     // In this example, we're not leveraging a relay service, so we'll just demonstrate some direct calls.

time-alarm-service-messages/Cargo.toml → time-alarm-service-interface/Cargo.toml

@@ -1,6 +1,5 @@
 [package]
-name = "time-alarm-service-messages"
-description = "Time and Alarm service message definitions"
+name = "time-alarm-service-interface"
 version.workspace = true
 edition.workspace = true
 license.workspace = true
@@ -11,13 +10,12 @@ bitfield.workspace = true
 defmt = { workspace = true, optional = true }
 log = { workspace = true, optional = true }
 embedded-mcu-hal.workspace = true
-embedded-services.workspace = true
 num_enum.workspace = true
 zerocopy = { workspace = true, features = ["derive"] }
 
 [features]
 defmt = ["dep:defmt", "embedded-mcu-hal/defmt"]
-log = ["dep:log", "embedded-services/log"]
+log = ["dep:log"]
 
 [lints]
 workspace = true

time-alarm-service-interface/src/lib.rs

@@ -0,0 +1,122 @@
+#![no_std]
+
+mod acpi_timestamp;
+pub use acpi_timestamp::{AcpiDaylightSavingsTimeStatus, AcpiTimeZone, AcpiTimeZoneOffset, AcpiTimestamp};
+
+use bitfield::bitfield;
+use embedded_mcu_hal::time::DatetimeClockError;
+
+#[derive(Clone, Copy, Debug, PartialEq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub struct AlarmTimerSeconds(pub u32);
+impl AlarmTimerSeconds {
+    pub const DISABLED: Self = Self(u32::MAX);
+}
+
+impl Default for AlarmTimerSeconds {
+    fn default() -> Self {
+        Self::DISABLED
+    }
+}
+
+// -------------------------------------------------
+
+#[derive(Clone, Copy, Debug, PartialEq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub struct AlarmExpiredWakePolicy(pub u32);
+impl AlarmExpiredWakePolicy {
+    pub const INSTANTLY: Self = Self(0);
+    pub const NEVER: Self = Self(u32::MAX);
+}
+
+impl Default for AlarmExpiredWakePolicy {
+    fn default() -> Self {
+        Self::NEVER
+    }
+}
+
+// -------------------------------------------------
+
+// Timer ID as defined in the ACPI spec.
+#[derive(Clone, Copy, Debug, PartialEq, num_enum::TryFromPrimitive, num_enum::IntoPrimitive)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+#[repr(u32)]
+pub enum AcpiTimerId {
+    AcPower = 0,
+    DcPower = 1,
+}
+
+impl AcpiTimerId {
+    pub fn get_other_timer_id(&self) -> Self {
+        match self {
+            AcpiTimerId::AcPower => AcpiTimerId::DcPower,
+            AcpiTimerId::DcPower => AcpiTimerId::AcPower,
+        }
+    }
+}
+
+bitfield!(
+    #[derive(Copy, Clone, Default, PartialEq, Eq)]
+    #[cfg_attr(feature = "defmt", derive(defmt::Format))]
+    pub struct TimerStatus(u32);
+    impl Debug;
+    bool;
+    pub timer_expired, set_timer_expired: 0;
+    pub timer_triggered_wake, set_timer_triggered_wake: 1;
+);
+
+// -------------------------------------------------
+
+bitfield!(
+    #[derive(Copy, Clone, Default, PartialEq, Eq)]
+    #[cfg_attr(feature = "defmt", derive(defmt::Format))]
+    pub struct TimeAlarmDeviceCapabilities(u32);
+    impl Debug;
+    bool;
+    pub ac_wake_implemented, set_ac_wake_implemented: 0;
+    pub dc_wake_implemented, set_dc_wake_implemented: 1;
+    pub realtime_implemented, set_realtime_implemented: 2;
+    pub realtime_accuracy_in_milliseconds, set_realtime_accuracy_in_milliseconds: 3;
+    pub get_wake_status_supported, set_get_wake_status_supported: 4;
+    pub ac_s4_wake_supported, set_ac_s4_wake_supported: 5;
+    pub ac_s5_wake_supported, set_ac_s5_wake_supported: 6;
+    pub dc_s4_wake_supported, set_dc_s4_wake_supported: 7;
+    pub dc_s5_wake_supported, set_dc_s5_wake_supported: 8;
+);
+
+// TODO figure out if we want to support a user-provided error type instead of forcing DatetimeClockError.
+// TODO figure out if we want to take &mut self for some of these methods - that does pretty much require that
+//      the caller will always use a mutex if they ever want to use it for anything other than exactly servicing
+//      ACPI method calls, though, and it seems like if 100% of real use cases require the mutex it might make sense
+//      to just do that for the user as part of the service implementation?
+pub trait TimeAlarmService {
+    fn get_capabilities(&self) -> TimeAlarmDeviceCapabilities;
+
+    /// Query the current time.  Analogous to ACPI TAD's _GRT method.
+    fn get_real_time(&self) -> Result<AcpiTimestamp, DatetimeClockError>;
+
+    /// Change the current time.  Analogous to ACPI TAD's _SRT method.
+    fn set_real_time(&self, timestamp: AcpiTimestamp) -> Result<(), DatetimeClockError>;
+
+    /// Query the current wake status.  Analogous to ACPI TAD's _GWS method.
+    fn get_wake_status(&self, timer_id: AcpiTimerId) -> TimerStatus;
+
+    /// Clear the current wake status.  Analogous to ACPI TAD's _CWS method.
+    fn clear_wake_status(&self, timer_id: AcpiTimerId);
+
+    /// Configures behavior when the timer expires while the system is on the other power source.  Analogous to ACPI TAD's _STP method.
+    fn set_expired_timer_policy(
+        &self,
+        timer_id: AcpiTimerId,
+        policy: AlarmExpiredWakePolicy,
+    ) -> Result<(), DatetimeClockError>;
+
+    /// Query current behavior when the timer expires while the system is on the other power source.  Analogous to ACPI TAD's _TIP method.
+    fn get_expired_timer_policy(&self, timer_id: AcpiTimerId) -> AlarmExpiredWakePolicy;
+
+    /// Change the expiry time for the given timer.  Analogous to ACPI TAD's _STV method.
+    fn set_timer_value(&self, timer_id: AcpiTimerId, timer_value: AlarmTimerSeconds) -> Result<(), DatetimeClockError>;
+
+    /// Query the expiry time for the given timer.  Analogous to ACPI TAD's _TIV method.
+    fn get_timer_value(&self, timer_id: AcpiTimerId) -> Result<AlarmTimerSeconds, DatetimeClockError>;
+}