From 70ce52d9159578dfbfbfbf2b81a1738750a0a0e2 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Fri, 20 Feb 2026 09:53:44 -0800 Subject: [PATCH 01/17] Add support for uniform spawning of services using the resources + inner-service pattern --- embedded-service/src/lib.rs | 1 + embedded-service/src/service.rs | 92 ++++++++++ espi-service/src/espi_service.rs | 76 +++++++-- espi-service/src/lib.rs | 3 - espi-service/src/task.rs | 7 - examples/rt685s-evk/src/bin/time_alarm.rs | 34 ++-- time-alarm-service/src/lib.rs | 197 ++++++++++++++++------ time-alarm-service/src/task.rs | 9 - time-alarm-service/tests/tad_test.rs | 47 +++--- 9 files changed, 336 insertions(+), 130 deletions(-) create mode 100644 embedded-service/src/service.rs delete mode 100644 espi-service/src/task.rs delete mode 100644 time-alarm-service/src/task.rs diff --git a/embedded-service/src/lib.rs b/embedded-service/src/lib.rs index c53aba53..2db63f7c 100644 --- a/embedded-service/src/lib.rs +++ b/embedded-service/src/lib.rs @@ -22,6 +22,7 @@ pub mod init; pub mod ipc; pub mod keyboard; pub mod relay; +pub mod service; pub mod sync; /// Hidden re-exports used by macros defined in this crate. diff --git a/embedded-service/src/service.rs b/embedded-service/src/service.rs new file mode 100644 index 00000000..8b6c75fc --- /dev/null +++ b/embedded-service/src/service.rs @@ -0,0 +1,92 @@ +//! This module contains helper traits and functions for services that run on the EC. + +/// A trait for a service that can be run on the EC. +/// Implementations of Service should have an init() function to construct the service that +/// returns a Runner, which the user is expected to spawn a task for. +pub trait Service<'hw>: Sized { + /// A type that can be used to run the service. This is returned by the init() function and the user is + /// expected to call its run() method in an embassy task (or similar parallel execution context on other + /// async runtimes). + type Runner: ServiceRunner<'hw>; + + /// Any memory resources that your service needs. This is typically an opaque types that is only used by the service + /// and is not interacted with by users of the service. Must be default-constructible for spawn_service!() to work. + type Resources: ServiceResources; + + /// The error type that your `init` function can return on failure. + type ErrorType; + + /// Any initialization parameters that your service needs to run. + type InitParams; + + /// Initializes an instance of the service using the provided storage and returns a control handle for the service and + /// a runner that can be used to run the service. + fn new( + storage: &'hw mut Self::Resources, + params: Self::InitParams, + ) -> impl core::future::Future>; +} + +/// Represents the memory resources that a service needs to run. This is typically an opaque type that is only used by the service. +/// Must be default-constructible; the service is expected to populate its contents in its init() function. +pub trait ServiceResources { + /// Creates a new instance of the service's resources. + fn new() -> Self; +} + +/// A trait for a run handle used to execute a service's event loop. This is returned by Service::init() +/// and the user is expected to call its run() method in an embassy task (or similar parallel execution context +/// on other async runtimes). +pub trait ServiceRunner<'hw> { + /// Run the service event loop. This future never completes. + fn run(self) -> impl core::future::Future + 'hw; +} + +/// Initializes a service, creates an embassy task to run it, and spawns that task. +/// +/// This macro handles the boilerplate of: +/// 1. Creating a `static` [`OnceLock`](embassy_sync::once_lock::OnceLock) to hold the service +/// 2. Calling the service's `init()` method +/// 3. Defining an embassy_executor::task to run the service +/// 4. Spawning the task on the provided executor +/// +/// Returns a Result where Error is the error type of $service_ty::init(). +/// +/// Arguments +/// +/// - spawner: An embassy_executor::Spawner. +/// - service_ty: The service type that implements Service that you want to create and run. +/// - init_arg: The init argument type to pass to `Service::init()` +/// +/// Example: +/// +/// ```ignore +/// let time_service = embedded_services::spawn_service!( +/// spawner, +/// time_alarm_service::Service<'static>, +/// time_alarm_service::ServiceInitParams { dt_clock, tz, ac_expiration, ac_policy, dc_expiration, dc_policy } +/// ).expect("failed to initialize time_alarm service"); +/// ``` +#[macro_export] +macro_rules! spawn_service { + ($spawner:expr, $service_ty:ty, $init_arg:expr) => {{ + use $crate::service::{Service, ServiceResources, ServiceRunner}; + static SERVICE_RESOURCES: StaticCell<(<$service_ty as Service>::Resources)> = StaticCell::new(); + let service_resources = + SERVICE_RESOURCES.init(<<$service_ty as Service>::Resources as ServiceResources>::new()); + + #[embassy_executor::task] + async fn service_task_fn(runner: <$service_ty as $crate::service::Service<'static>>::Runner) { + runner.run().await; + } + + <$service_ty>::new(service_resources, $init_arg) + .await + .map(|(control_handle, runner)| { + $spawner.must_spawn(service_task_fn(runner)); + control_handle + }) + }}; +} + +pub use spawn_service; diff --git a/espi-service/src/espi_service.rs b/espi-service/src/espi_service.rs index 0a7c1d93..2dc4ba03 100644 --- a/espi-service/src/espi_service.rs +++ b/espi-service/src/espi_service.rs @@ -4,7 +4,6 @@ use embassy_futures::select::select; use embassy_imxrt::espi; use embassy_sync::channel::Channel; use embassy_sync::mutex::Mutex; -use embassy_sync::once_lock::OnceLock; use embedded_services::{GlobalRawMutex, error, info, trace}; use mctp_rs::smbus_espi::SmbusEspiMedium; use mctp_rs::smbus_espi::SmbusEspiReplyContext; @@ -32,28 +31,77 @@ pub enum Error { Buffer(embedded_services::buffer::Error), } +/// The memory required by the eSPI service to run +pub struct Resources<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> { + inner: Option>, +} + +impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> embedded_services::service::ServiceResources + for Resources<'hw, RelayHandler> +{ + fn new() -> Self { + Self { inner: None } + } +} + +/// Service runner for the eSPI service. Users must call the run() method on the service to start processing events. +pub struct Runner<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> { + inner: &'hw ServiceInner<'hw, RelayHandler>, +} + +impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> embedded_services::service::ServiceRunner<'hw> + for Runner<'hw, RelayHandler> +{ + /// Run the service event loop. + async fn run(self) -> embedded_services::Never { + self.inner.run().await + } +} + pub struct Service<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> { + _inner: &'hw ServiceInner<'hw, RelayHandler>, +} + +impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> embedded_services::service::Service<'hw> + for Service<'hw, RelayHandler> +{ + type Resources = Resources<'hw, RelayHandler>; + type Runner = Runner<'hw, RelayHandler>; + type ErrorType = core::convert::Infallible; + type InitParams = InitParams<'hw, RelayHandler>; + + async fn new( + resources: &'hw mut Self::Resources, + params: InitParams<'hw, RelayHandler>, + ) -> Result<(Self, Self::Runner), core::convert::Infallible> { + let inner = resources.inner.insert(ServiceInner::new(params).await); + Ok((Self { _inner: inner }, Runner { inner })) + } +} + +pub struct InitParams<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> { + pub espi: espi::Espi<'hw>, + pub relay_handler: RelayHandler, +} + +struct ServiceInner<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> { espi: Mutex>, host_tx_queue: Channel, HOST_TX_QUEUE_SIZE>, relay_handler: RelayHandler, } -impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> Service<'hw, RelayHandler> { - pub async fn init( - service_storage: &'hw OnceLock, - mut espi: espi::Espi<'hw>, - relay_handler: RelayHandler, - ) -> &'hw Self { - espi.wait_for_plat_reset().await; +impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> ServiceInner<'hw, RelayHandler> { + async fn new(mut init_params: InitParams<'hw, RelayHandler>) -> Self { + init_params.espi.wait_for_plat_reset().await; - service_storage.get_or_init(|| Service { - espi: Mutex::new(espi), + Self { + espi: Mutex::new(init_params.espi), host_tx_queue: Channel::new(), - relay_handler, - }) + relay_handler: init_params.relay_handler, + } } - pub(crate) async fn run_service(&self) -> ! { + async fn run(&self) -> embedded_services::Never { let mut espi = self.espi.lock().await; loop { let event = select(espi.wait_for_event(), self.host_tx_queue.receive()).await; @@ -180,9 +228,7 @@ impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> Service<'h } Ok(()) } -} -impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> Service<'hw, RelayHandler> { async fn process_request_to_ec( &self, (header, body): ( diff --git a/espi-service/src/lib.rs b/espi-service/src/lib.rs index c3f65e52..ce0c51fd 100644 --- a/espi-service/src/lib.rs +++ b/espi-service/src/lib.rs @@ -22,8 +22,5 @@ #[cfg(not(test))] mod espi_service; -#[cfg(not(test))] -pub mod task; - #[cfg(not(test))] pub use espi_service::*; diff --git a/espi-service/src/task.rs b/espi-service/src/task.rs deleted file mode 100644 index 578551c5..00000000 --- a/espi-service/src/task.rs +++ /dev/null @@ -1,7 +0,0 @@ -use crate::Service; - -pub async fn espi_service<'hw, R: embedded_services::relay::mctp::RelayHandler>( - espi_service: &'hw Service<'hw, R>, -) -> Result { - espi_service.run_service().await -} diff --git a/examples/rt685s-evk/src/bin/time_alarm.rs b/examples/rt685s-evk/src/bin/time_alarm.rs index 9b1a8182..be035eb5 100644 --- a/examples/rt685s-evk/src/bin/time_alarm.rs +++ b/examples/rt685s-evk/src/bin/time_alarm.rs @@ -1,7 +1,6 @@ #![no_std] #![no_main] -use embassy_sync::once_lock::OnceLock; use embedded_mcu_hal::{ Nvram, time::{Datetime, Month, UncheckedDatetime}, @@ -24,25 +23,19 @@ async fn main(spawner: embassy_executor::Spawner) { embedded_services::init().await; info!("services initialized"); - static TIME_SERVICE: OnceLock = OnceLock::new(); - let time_service = time_alarm_service::Service::init( - &TIME_SERVICE, - dt_clock, - tz, - ac_expiration, - ac_policy, - dc_expiration, - dc_policy, + let time_service = embedded_services::spawn_service!( + spawner, + time_alarm_service::Service<'static>, + time_alarm_service::InitParams { + backing_clock: dt_clock, + tz_storage: tz, + ac_expiration_storage: ac_expiration, + ac_policy_storage: ac_policy, + dc_expiration_storage: dc_expiration, + dc_policy_storage: dc_policy + } ) - .await - .expect("Failed to initialize time-alarm service"); - - #[embassy_executor::task] - async fn time_alarm_task(service: &'static time_alarm_service::Service<'static>) { - time_alarm_service::task::run_service(service).await - } - - spawner.must_spawn(time_alarm_task(time_service)); + .expect("Failed to spawn time alarm service"); use embedded_services::relay::mctp::impl_odp_mctp_relay_handler; impl_odp_mctp_relay_handler!( @@ -50,12 +43,11 @@ async fn main(spawner: embassy_executor::Spawner) { TimeAlarm, 0x0B, time_alarm_service::Service<'static>; ); - let _relay_handler = EspiRelayHandler::new(time_service); + let _relay_handler = EspiRelayHandler::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_service .set_real_time(AcpiTimestamp { datetime: Datetime::new(UncheckedDatetime { diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index 333420ae..f0125b3e 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -2,7 +2,6 @@ use core::cell::RefCell; use embassy_sync::blocking_mutex::Mutex; -use embassy_sync::once_lock::OnceLock; use embassy_sync::signal::Signal; use embedded_mcu_hal::NvramStorage; use embedded_mcu_hal::time::{Datetime, DatetimeClock, DatetimeClockError}; @@ -10,7 +9,6 @@ use embedded_services::GlobalRawMutex; use embedded_services::{info, warn}; use time_alarm_service_messages::*; -pub mod task; mod timer; use timer::Timer; @@ -104,7 +102,19 @@ impl<'hw> Timers<'hw> { // ------------------------------------------------- -pub struct Service<'hw> { +/// Parameters required to initialize the time/alarm service. +pub struct InitParams<'hw> { + pub backing_clock: &'hw mut dyn DatetimeClock, + pub tz_storage: &'hw mut dyn NvramStorage<'hw, u32>, + pub ac_expiration_storage: &'hw mut dyn NvramStorage<'hw, u32>, + pub ac_policy_storage: &'hw mut dyn NvramStorage<'hw, u32>, + pub dc_expiration_storage: &'hw mut dyn NvramStorage<'hw, u32>, + pub dc_policy_storage: &'hw mut dyn NvramStorage<'hw, u32>, +} + +/// The main service implementation. Users will interact with this via the Service struct, which is a thin wrapper around this that allows +/// the client to provide storage for the service. +struct ServiceInner<'hw> { clock_state: Mutex>>, // TODO [POWER_SOURCE] signal this whenever the power source changes @@ -115,29 +125,19 @@ pub struct Service<'hw> { capabilities: TimeAlarmDeviceCapabilities, } -impl<'hw> Service<'hw> { - pub async fn init( - service_storage: &'hw OnceLock>, - backing_clock: &'hw mut impl DatetimeClock, - tz_storage: &'hw mut dyn NvramStorage<'hw, u32>, - ac_expiration_storage: &'hw mut dyn NvramStorage<'hw, u32>, - ac_policy_storage: &'hw mut dyn NvramStorage<'hw, u32>, - dc_expiration_storage: &'hw mut dyn NvramStorage<'hw, u32>, - dc_policy_storage: &'hw mut dyn NvramStorage<'hw, u32>, - ) -> Result<&'hw Service<'hw>, DatetimeClockError> { - info!("Starting time-alarm service task"); - - let service = service_storage.get_or_init(|| Service { +impl<'hw> ServiceInner<'hw> { + fn new(init_params: InitParams<'hw>) -> Self { + Self { clock_state: Mutex::new(RefCell::new(ClockState { - datetime_clock: backing_clock, - tz_data: TimeZoneData::new(tz_storage), + datetime_clock: init_params.backing_clock, + tz_data: TimeZoneData::new(init_params.tz_storage), })), power_source_signal: Signal::new(), timers: Timers::new( - ac_expiration_storage, - ac_policy_storage, - dc_expiration_storage, - dc_policy_storage, + init_params.ac_expiration_storage, + init_params.ac_policy_storage, + init_params.dc_expiration_storage, + init_params.dc_policy_storage, ), 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 @@ -153,23 +153,16 @@ impl<'hw> Service<'hw> { caps.set_dc_s5_wake_supported(true); caps }, - }); - - // 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)?; - - Ok(service) + } } /// Query clock capabilities. Analogous to ACPI TAD's _GRT method. - pub fn get_capabilities(&self) -> TimeAlarmDeviceCapabilities { + fn get_capabilities(&self) -> TimeAlarmDeviceCapabilities { self.capabilities } /// Query the current time. Analogous to ACPI TAD's _GRT method. - pub fn get_real_time(&self) -> Result { + fn get_real_time(&self) -> Result { self.clock_state.lock(|clock_state| { let clock_state = clock_state.borrow(); let datetime = clock_state.datetime_clock.get_current_datetime()?; @@ -183,7 +176,7 @@ impl<'hw> Service<'hw> { } /// Change the current time. Analogous to ACPI TAD's _SRT method. - pub fn set_real_time(&self, timestamp: AcpiTimestamp) -> Result<(), DatetimeClockError> { + fn set_real_time(&self, timestamp: AcpiTimestamp) -> Result<(), DatetimeClockError> { self.clock_state.lock(|clock_state| { let mut clock_state = clock_state.borrow_mut(); clock_state.datetime_clock.set_current_datetime(×tamp.datetime)?; @@ -193,17 +186,17 @@ impl<'hw> Service<'hw> { } /// Query the current wake status. Analogous to ACPI TAD's _GWS method. - pub fn get_wake_status(&self, timer_id: AcpiTimerId) -> TimerStatus { + fn get_wake_status(&self, timer_id: AcpiTimerId) -> TimerStatus { self.timers.get_timer(timer_id).get_wake_status() } /// Clear the current wake status. Analogous to ACPI TAD's _CWS method. - pub fn clear_wake_status(&self, timer_id: AcpiTimerId) { + fn clear_wake_status(&self, timer_id: AcpiTimerId) { self.timers.get_timer(timer_id).clear_wake_status(); } /// Configures behavior when the timer expires while the system is on the other power source. Analogous to ACPI TAD's _STP method. - pub fn set_expired_timer_policy( + fn set_expired_timer_policy( &self, timer_id: AcpiTimerId, policy: AlarmExpiredWakePolicy, @@ -215,16 +208,12 @@ impl<'hw> Service<'hw> { } /// Query current behavior when the timer expires while the system is on the other power source. Analogous to ACPI TAD's _TIP method. - pub fn get_expired_timer_policy(&self, timer_id: AcpiTimerId) -> AlarmExpiredWakePolicy { + fn get_expired_timer_policy(&self, timer_id: AcpiTimerId) -> AlarmExpiredWakePolicy { self.timers.get_timer(timer_id).get_timer_wake_policy() } /// Change the expiry time for the given timer. Analogous to ACPI TAD's _STV method. - pub fn set_timer_value( - &self, - timer_id: AcpiTimerId, - timer_value: AlarmTimerSeconds, - ) -> Result<(), DatetimeClockError> { + fn set_timer_value(&self, timer_id: AcpiTimerId, timer_value: AlarmTimerSeconds) -> Result<(), DatetimeClockError> { let new_expiration_time = match timer_value { AlarmTimerSeconds::DISABLED => None, AlarmTimerSeconds(secs) => { @@ -245,7 +234,7 @@ impl<'hw> Service<'hw> { } /// Query the expiry time for the given timer. Analogous to ACPI TAD's _TIV method. - pub fn get_timer_value(&self, timer_id: AcpiTimerId) -> Result { + fn get_timer_value(&self, timer_id: AcpiTimerId) -> Result { let expiration_time = self.timers.get_timer(timer_id).get_expiration_time(); match expiration_time { Some(expiration_time) => { @@ -263,17 +252,6 @@ impl<'hw> Service<'hw> { } } - pub(crate) async fn run_service(&'hw self) -> ! { - loop { - embassy_futures::select::select3( - self.handle_power_source_updates(), - self.handle_timer(AcpiTimerId::AcPower), - self.handle_timer(AcpiTimerId::DcPower), - ) - .await; - } - } - async fn handle_power_source_updates(&'hw self) -> ! { loop { let new_power_source = self.power_source_signal.wait().await; @@ -311,6 +289,117 @@ impl<'hw> Service<'hw> { } } +/// The memory resources required by the time/alarm service. +pub struct Resources<'hw> { + inner: Option>, +} + +impl<'hw> embedded_services::service::ServiceResources for Resources<'hw> { + /// Allocate storage for the service's resources. + fn new() -> Self { + Resources { inner: None } + } +} + +/// A task runner for the time/alarm service. Users of the service must run this object in an embassy task or similar async execution context. +pub struct Runner<'hw> { + service: &'hw ServiceInner<'hw>, +} + +impl<'hw> embedded_services::service::ServiceRunner<'hw> for Runner<'hw> { + /// Run the service. + async fn run(self) -> embedded_services::Never { + loop { + embassy_futures::select::select3( + self.service.handle_power_source_updates(), + self.service.handle_timer(AcpiTimerId::AcPower), + self.service.handle_timer(AcpiTimerId::DcPower), + ) + .await; + } + } +} + +/// Control handle for the time-alarm service. Use this to manipulate the time on the service. +pub struct Service<'hw> { + inner: &'hw ServiceInner<'hw>, +} + +impl<'hw> Service<'hw> { + pub fn get_capabilities(&self) -> TimeAlarmDeviceCapabilities { + self.inner.get_capabilities() + } + + /// Query the current time. Analogous to ACPI TAD's _GRT method. + pub fn get_real_time(&self) -> Result { + self.inner.get_real_time() + } + + /// Change the current time. Analogous to ACPI TAD's _SRT method. + pub fn set_real_time(&self, timestamp: AcpiTimestamp) -> Result<(), DatetimeClockError> { + self.inner.set_real_time(timestamp) + } + + /// Query the current wake status. Analogous to ACPI TAD's _GWS method. + pub fn get_wake_status(&self, timer_id: AcpiTimerId) -> TimerStatus { + self.inner.get_wake_status(timer_id) + } + + /// Clear the current wake status. Analogous to ACPI TAD's _CWS method. + pub fn clear_wake_status(&self, timer_id: AcpiTimerId) { + self.inner.clear_wake_status(timer_id); + } + + /// Configures behavior when the timer expires while the system is on the other power source. Analogous to ACPI TAD's _STP method. + pub fn set_expired_timer_policy( + &self, + timer_id: AcpiTimerId, + policy: AlarmExpiredWakePolicy, + ) -> Result<(), DatetimeClockError> { + self.inner.set_expired_timer_policy(timer_id, policy) + } + + /// Query current behavior when the timer expires while the system is on the other power source. Analogous to ACPI TAD's _TIP method. + pub fn get_expired_timer_policy(&self, timer_id: AcpiTimerId) -> AlarmExpiredWakePolicy { + self.inner.get_expired_timer_policy(timer_id) + } + + /// Change the expiry time for the given timer. Analogous to ACPI TAD's _STV method. + pub fn set_timer_value( + &self, + timer_id: AcpiTimerId, + timer_value: AlarmTimerSeconds, + ) -> Result<(), DatetimeClockError> { + self.inner.set_timer_value(timer_id, timer_value) + } + + /// Query the expiry time for the given timer. Analogous to ACPI TAD's _TIV method. + pub fn get_timer_value(&self, timer_id: AcpiTimerId) -> Result { + self.inner.get_timer_value(timer_id) + } +} + +impl<'hw> embedded_services::service::Service<'hw> for Service<'hw> { + type Runner = Runner<'hw>; + type ErrorType = DatetimeClockError; + type InitParams = InitParams<'hw>; + type Resources = Resources<'hw>; + + async fn new( + service_storage: &'hw mut Resources<'hw>, + init_params: Self::InitParams, + ) -> Result<(Self, Runner<'hw>), DatetimeClockError> { + let service = service_storage.inner.insert(ServiceInner::new(init_params)); + + // 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)?; + + Ok((Self { inner: service }, Runner { service })) + } +} + impl<'hw> embedded_services::relay::mctp::RelayServiceHandlerTypes for Service<'hw> { type RequestType = AcpiTimeAlarmRequest; type ResultType = AcpiTimeAlarmResult; diff --git a/time-alarm-service/src/task.rs b/time-alarm-service/src/task.rs deleted file mode 100644 index a45fdf62..00000000 --- a/time-alarm-service/src/task.rs +++ /dev/null @@ -1,9 +0,0 @@ -use crate::Service; -use embedded_services::info; - -/// Call this from a dedicated async task. Must be called exactly once on each service instance. -/// Note that on-device, 'hw must be 'static. We're generic over 'hw to enable some test scenarios leveraging tokio and mocks. -pub async fn run_service<'hw>(service: &'hw Service<'hw>) -> ! { - info!("Starting time-alarm service task"); - service.run_service().await -} diff --git a/time-alarm-service/tests/tad_test.rs b/time-alarm-service/tests/tad_test.rs index dc3d43d8..8eabb764 100644 --- a/time-alarm-service/tests/tad_test.rs +++ b/time-alarm-service/tests/tad_test.rs @@ -6,9 +6,9 @@ mod common; #[cfg(test)] mod test { - use embassy_sync::once_lock::OnceLock; use embassy_time::Timer; use embedded_mcu_hal::time::{Datetime, DatetimeClock}; + use embedded_services::service::{Service, ServiceResources, ServiceRunner}; use time_alarm_service_messages as msg; @@ -23,16 +23,18 @@ mod test { let mut dc_pol_storage = MockNvramStorage::new(0); let mut clock = MockDatetimeClock::new_running(); + let mut storage = time_alarm_service::Resources::new(); - let storage = OnceLock::new(); - let service = time_alarm_service::Service::init( - &storage, - &mut clock, - &mut tz_storage, - &mut ac_exp_storage, - &mut ac_pol_storage, - &mut dc_exp_storage, - &mut dc_pol_storage, + let (service, runner) = time_alarm_service::Service::new( + &mut storage, + time_alarm_service::InitParams { + backing_clock: &mut clock, + tz_storage: &mut tz_storage, + ac_expiration_storage: &mut ac_exp_storage, + ac_policy_storage: &mut ac_pol_storage, + dc_expiration_storage: &mut dc_exp_storage, + dc_policy_storage: &mut dc_pol_storage, + }, ) .await .unwrap(); @@ -45,7 +47,7 @@ mod test { // return !, so we should go until the test arm completes and then shut down. // tokio::select! { - _ = time_alarm_service::task::run_service(service) => unreachable!("time alarm service task finished unexpectedly"), + _ = runner.run() => unreachable!("time alarm service task finished unexpectedly"), _ = async { let delay_secs = 2; let begin = service.get_real_time().unwrap(); @@ -73,21 +75,24 @@ mod test { .set_current_datetime(&Datetime::from_unix_time_seconds(TEST_UNIX_TIME)) .unwrap(); - let storage = OnceLock::new(); - let service = time_alarm_service::Service::init( - &storage, - &mut clock, - &mut tz_storage, - &mut ac_exp_storage, - &mut ac_pol_storage, - &mut dc_exp_storage, - &mut dc_pol_storage, + let mut storage = time_alarm_service::Resources::new(); + + let (service, runner) = time_alarm_service::Service::new( + &mut storage, + time_alarm_service::InitParams { + backing_clock: &mut clock, + tz_storage: &mut tz_storage, + ac_expiration_storage: &mut ac_exp_storage, + ac_policy_storage: &mut ac_pol_storage, + dc_expiration_storage: &mut dc_exp_storage, + dc_policy_storage: &mut dc_pol_storage, + }, ) .await .unwrap(); tokio::select! { - _ = time_alarm_service::task::run_service(service) => unreachable!("time alarm service task finished unexpectedly"), + _ = runner.run() => unreachable!("time alarm service task finished unexpectedly"), _ = async { // Clock is paused, so time shouldn't advance unless we set it. let begin = service.get_real_time().unwrap(); From 3392cb11bccdb60bd67d8256fcaee9ba8bfc17c9 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Thu, 5 Mar 2026 10:42:00 -0800 Subject: [PATCH 02/17] replace unnecessary trait with default --- embedded-service/src/service.rs | 14 +++----------- espi-service/src/espi_service.rs | 6 ++---- time-alarm-service/src/lib.rs | 4 ++-- time-alarm-service/tests/tad_test.rs | 6 +++--- 4 files changed, 10 insertions(+), 20 deletions(-) diff --git a/embedded-service/src/service.rs b/embedded-service/src/service.rs index 8b6c75fc..646aa55d 100644 --- a/embedded-service/src/service.rs +++ b/embedded-service/src/service.rs @@ -11,7 +11,7 @@ pub trait Service<'hw>: Sized { /// Any memory resources that your service needs. This is typically an opaque types that is only used by the service /// and is not interacted with by users of the service. Must be default-constructible for spawn_service!() to work. - type Resources: ServiceResources; + type Resources: Default; /// The error type that your `init` function can return on failure. type ErrorType; @@ -27,13 +27,6 @@ pub trait Service<'hw>: Sized { ) -> impl core::future::Future>; } -/// Represents the memory resources that a service needs to run. This is typically an opaque type that is only used by the service. -/// Must be default-constructible; the service is expected to populate its contents in its init() function. -pub trait ServiceResources { - /// Creates a new instance of the service's resources. - fn new() -> Self; -} - /// A trait for a run handle used to execute a service's event loop. This is returned by Service::init() /// and the user is expected to call its run() method in an embassy task (or similar parallel execution context /// on other async runtimes). @@ -70,10 +63,9 @@ pub trait ServiceRunner<'hw> { #[macro_export] macro_rules! spawn_service { ($spawner:expr, $service_ty:ty, $init_arg:expr) => {{ - use $crate::service::{Service, ServiceResources, ServiceRunner}; + use $crate::service::{Service, ServiceRunner}; static SERVICE_RESOURCES: StaticCell<(<$service_ty as Service>::Resources)> = StaticCell::new(); - let service_resources = - SERVICE_RESOURCES.init(<<$service_ty as Service>::Resources as ServiceResources>::new()); + let service_resources = SERVICE_RESOURCES.init(<<$service_ty as Service>::Resources as Default>::default()); #[embassy_executor::task] async fn service_task_fn(runner: <$service_ty as $crate::service::Service<'static>>::Runner) { diff --git a/espi-service/src/espi_service.rs b/espi-service/src/espi_service.rs index 2dc4ba03..5ee2235f 100644 --- a/espi-service/src/espi_service.rs +++ b/espi-service/src/espi_service.rs @@ -36,10 +36,8 @@ pub struct Resources<'hw, RelayHandler: embedded_services::relay::mctp::RelayHan inner: Option>, } -impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> embedded_services::service::ServiceResources - for Resources<'hw, RelayHandler> -{ - fn new() -> Self { +impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> Default for Resources<'hw, RelayHandler> { + fn default() -> Self { Self { inner: None } } } diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index f0125b3e..2c4ffd07 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -294,9 +294,9 @@ pub struct Resources<'hw> { inner: Option>, } -impl<'hw> embedded_services::service::ServiceResources for Resources<'hw> { +impl<'hw> Default for Resources<'hw> { /// Allocate storage for the service's resources. - fn new() -> Self { + fn default() -> Self { Resources { inner: None } } } diff --git a/time-alarm-service/tests/tad_test.rs b/time-alarm-service/tests/tad_test.rs index 8eabb764..f9fb41d8 100644 --- a/time-alarm-service/tests/tad_test.rs +++ b/time-alarm-service/tests/tad_test.rs @@ -8,7 +8,7 @@ mod common; mod test { use embassy_time::Timer; use embedded_mcu_hal::time::{Datetime, DatetimeClock}; - use embedded_services::service::{Service, ServiceResources, ServiceRunner}; + use embedded_services::service::{Service, ServiceRunner}; use time_alarm_service_messages as msg; @@ -23,7 +23,7 @@ mod test { let mut dc_pol_storage = MockNvramStorage::new(0); let mut clock = MockDatetimeClock::new_running(); - let mut storage = time_alarm_service::Resources::new(); + let mut storage = Default::default(); let (service, runner) = time_alarm_service::Service::new( &mut storage, @@ -75,7 +75,7 @@ mod test { .set_current_datetime(&Datetime::from_unix_time_seconds(TEST_UNIX_TIME)) .unwrap(); - let mut storage = time_alarm_service::Resources::new(); + let mut storage = Default::default(); let (service, runner) = time_alarm_service::Service::new( &mut storage, From c19c1c22663b8cfd7c9ee76882f50cd7b4525821 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 9 Mar 2026 10:30:05 -0700 Subject: [PATCH 03/17] derive default --- time-alarm-service/src/lib.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index 2c4ffd07..b5ee4769 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -290,17 +290,11 @@ impl<'hw> ServiceInner<'hw> { } /// The memory resources required by the time/alarm service. +#[derive(Default)] pub struct Resources<'hw> { inner: Option>, } -impl<'hw> Default for Resources<'hw> { - /// Allocate storage for the service's resources. - fn default() -> Self { - Resources { inner: None } - } -} - /// A task runner for the time/alarm service. Users of the service must run this object in an embassy task or similar async execution context. pub struct Runner<'hw> { service: &'hw ServiceInner<'hw>, From f39a32f20a8fe7ed0bfb5637bb6994076e994788 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 9 Mar 2026 14:30:16 -0700 Subject: [PATCH 04/17] Move common helper code to new crate --- Cargo.lock | 13 +- Cargo.toml | 2 + embedded-service/src/lib.rs | 1 - espi-service/Cargo.toml | 12 +- espi-service/src/espi_service.rs | 4 +- examples/rt633/Cargo.lock | 614 ++++-------------- examples/rt685s-evk/Cargo.lock | 9 + examples/rt685s-evk/Cargo.toml | 1 + examples/rt685s-evk/src/bin/time_alarm.rs | 2 +- odp-service-common/Cargo.toml | 12 + odp-service-common/src/lib.rs | 4 + .../src/runnable_service.rs | 6 +- time-alarm-service/Cargo.toml | 3 +- time-alarm-service/src/lib.rs | 4 +- time-alarm-service/tests/tad_test.rs | 2 +- 15 files changed, 172 insertions(+), 517 deletions(-) create mode 100644 odp-service-common/Cargo.toml create mode 100644 odp-service-common/src/lib.rs rename embedded-service/src/service.rs => odp-service-common/src/runnable_service.rs (92%) diff --git a/Cargo.lock b/Cargo.lock index 341488c5..cdca2cc1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1104,11 +1104,9 @@ dependencies = [ name = "espi-service" version = "0.1.0" dependencies = [ - "battery-service-messages", "bitfield 0.17.0", "cortex-m", "cortex-m-rt", - "debug-service-messages", "defmt 0.3.100", "embassy-futures", "embassy-imxrt", @@ -1118,8 +1116,7 @@ dependencies = [ "log", "mctp-rs", "num_enum", - "thermal-service-messages", - "time-alarm-service-messages", + "odp-service-common", ] [[package]] @@ -1714,6 +1711,13 @@ dependencies = [ "memchr", ] +[[package]] +name = "odp-service-common" +version = "0.1.0" +dependencies = [ + "embedded-services", +] + [[package]] name = "once_cell" version = "1.21.3" @@ -2346,6 +2350,7 @@ dependencies = [ "embedded-mcu-hal", "embedded-services", "log", + "odp-service-common", "time-alarm-service-messages", "tokio", "zerocopy", diff --git a/Cargo.toml b/Cargo.toml index 410ee4a6..ffe8c00b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ members = [ "debug-service-messages", "keyboard-service", "power-policy-interface", + "odp-service-common", ] exclude = ["examples/*"] @@ -86,6 +87,7 @@ embedded-storage-async = "0.4.1" embedded-usb-pd = { git = "https://github.com/OpenDevicePartnership/embedded-usb-pd", default-features = false } mctp-rs = { git = "https://github.com/dymk/mctp-rs" } num_enum = { version = "0.7.5", default-features = false } +odp-service-common = { path = "./odp-service-common" } portable-atomic = { version = "1.11", default-features = false } power-policy-interface = { path = "./power-policy-interface" } paste = "1.0.15" diff --git a/embedded-service/src/lib.rs b/embedded-service/src/lib.rs index 2db63f7c..c53aba53 100644 --- a/embedded-service/src/lib.rs +++ b/embedded-service/src/lib.rs @@ -22,7 +22,6 @@ pub mod init; pub mod ipc; pub mod keyboard; pub mod relay; -pub mod service; pub mod sync; /// Hidden re-exports used by macros defined in this crate. diff --git a/espi-service/Cargo.toml b/espi-service/Cargo.toml index 1c3214e7..91d91c5b 100644 --- a/espi-service/Cargo.toml +++ b/espi-service/Cargo.toml @@ -21,13 +21,7 @@ embassy-imxrt = { workspace = true, features = ["mimxrt633s"] } embassy-futures.workspace = true mctp-rs = { workspace = true, features = ["espi"] } num_enum.workspace = true - -# TODO Service message type crates are a temporary dependency until we can parameterize -# the supported messages types at eSPI service creation time. -battery-service-messages.workspace = true -debug-service-messages.workspace = true -thermal-service-messages.workspace = true -time-alarm-service-messages.workspace = true +odp-service-common.workspace = true [target.'cfg(target_os = "none")'.dependencies] cortex-m-rt.workspace = true @@ -45,16 +39,12 @@ embassy-imxrt = { workspace = true, features = [ default = [] defmt = [ "dep:defmt", - "battery-service-messages/defmt", - "debug-service-messages/defmt", "embedded-services/defmt", "embassy-time/defmt", "embassy-time/defmt-timestamp-uptime", "embassy-sync/defmt", "embassy-imxrt/defmt", "mctp-rs/defmt", - "thermal-service-messages/defmt", - "time-alarm-service-messages/defmt", ] log = ["dep:log", "embedded-services/log", "embassy-time/log"] diff --git a/espi-service/src/espi_service.rs b/espi-service/src/espi_service.rs index 5ee2235f..682b5826 100644 --- a/espi-service/src/espi_service.rs +++ b/espi-service/src/espi_service.rs @@ -47,7 +47,7 @@ pub struct Runner<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandle inner: &'hw ServiceInner<'hw, RelayHandler>, } -impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> embedded_services::service::ServiceRunner<'hw> +impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> odp_service_common::runnable_service::ServiceRunner<'hw> for Runner<'hw, RelayHandler> { /// Run the service event loop. @@ -60,7 +60,7 @@ pub struct Service<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandl _inner: &'hw ServiceInner<'hw, RelayHandler>, } -impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> embedded_services::service::Service<'hw> +impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> odp_service_common::runnable_service::Service<'hw> for Service<'hw, RelayHandler> { type Resources = Resources<'hw, RelayHandler>; diff --git a/examples/rt633/Cargo.lock b/examples/rt633/Cargo.lock index cfdc94d9..4b0ce9fb 100644 --- a/examples/rt633/Cargo.lock +++ b/examples/rt633/Cargo.lock @@ -2,24 +2,6 @@ # It is not intended for manual editing. version = 4 -[[package]] -name = "ahash" -version = "0.8.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a15f179cd60c4584b8a8c596927aadc462e27f2ca70c04e0071964a73ba7a75" -dependencies = [ - "cfg-if", - "once_cell", - "version_check", - "zerocopy", -] - -[[package]] -name = "anyhow" -version = "1.0.98" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e16d2d3311acee920a9eb8d33b8cbc1787ce4a264e85f964c2404b969bdcd487" - [[package]] name = "aquamarine" version = "0.6.0" @@ -31,50 +13,7 @@ dependencies = [ "proc-macro-error2", "proc-macro2", "quote", - "syn 2.0.104", -] - -[[package]] -name = "arraydeque" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d902e3d592a523def97af8f317b08ce16b7ab854c1985a0c671e6f15cebc236" - -[[package]] -name = "askama" -version = "0.14.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f75363874b771be265f4ffe307ca705ef6f3baa19011c149da8674a87f1b75c4" -dependencies = [ - "askama_derive", - "itoa", - "percent-encoding", - "serde", - "serde_json", -] - -[[package]] -name = "askama_derive" -version = "0.14.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "129397200fe83088e8a68407a8e2b1f826cf0086b21ccdb866a722c8bcd3a94f" -dependencies = [ - "askama_parser", - "memchr", - "proc-macro2", - "quote", - "rustc-hash", - "syn 2.0.104", -] - -[[package]] -name = "askama_parser" -version = "0.14.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6ab5630b3d5eaf232620167977f95eb51f3432fc76852328774afbd242d4358" -dependencies = [ - "memchr", - "winnow 0.7.12", + "syn", ] [[package]] @@ -85,9 +24,9 @@ checksum = "c08606f8c3cbf4ce6ec8e28fb0014a2c086708fe954eaa885384a6165172e7e8" [[package]] name = "az" -version = "1.2.1" +version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7b7e4c2464d97fe331d41de9d5db0def0a96f4d823b8b32a2efd503578988973" +checksum = "be5eb007b7cacc6c660343e96f650fedf4b5a77512399eb952ca6642cf8d13f7" [[package]] name = "bare-metal" @@ -107,7 +46,7 @@ dependencies = [ "embassy-futures", "embassy-sync", "embassy-time", - "embedded-batteries-async 0.3.4", + "embedded-batteries-async", "embedded-hal 1.0.0", "embedded-hal-async", "embedded-services", @@ -122,7 +61,7 @@ name = "battery-service-messages" version = "0.1.0" dependencies = [ "defmt 0.3.100", - "embedded-batteries-async 0.3.4", + "embedded-batteries-async", "embedded-services", "num_enum", ] @@ -164,22 +103,22 @@ checksum = "f798d2d157e547aa99aab0967df39edd0b70307312b6f8bd2848e6abe40896e0" [[package]] name = "bitfield" -version = "0.19.1" +version = "0.19.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "db1bcd90f88eabbf0cadbfb87a45bceeaebcd3b4bc9e43da379cd2ef0162590d" +checksum = "21ba6517c6b0f2bf08be60e187ab64b038438f22dd755614d8fe4d4098c46419" dependencies = [ "bitfield-macros", ] [[package]] name = "bitfield-macros" -version = "0.19.1" +version = "0.19.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3787a07661997bfc05dd3431e379c0188573f78857080cf682e1393ab8e4d64c" +checksum = "f48d6ace212fdf1b45fd6b566bb40808415344642b76c3224c07c8df9da81e97" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn", ] [[package]] @@ -190,7 +129,7 @@ checksum = "2be5a46ba01b60005ae2c51a36a29cfe134bcacae2dd5cedcd4615fbaad1494b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn", ] [[package]] @@ -201,7 +140,7 @@ checksum = "8769c4854c5ada2852ddf6fd09d15cf43d4c2aaeccb4de6432f5402f08a6003b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn", ] [[package]] @@ -212,9 +151,9 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "bitflags" -version = "2.9.1" +version = "2.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b8e56985ec62d17e9c1001dc89c88ecd7dc08e47eba5ec7c29c7b5eeecde967" +checksum = "843867be96c8daad0d758b57df9392b6d8d271134fce549de6ce169ff98a92af" [[package]] name = "bitvec" @@ -230,23 +169,23 @@ dependencies = [ [[package]] name = "bq25773" -version = "0.1.0" -source = "git+https://github.com/OpenDevicePartnership/bq25773#20bc26219b5372bc6146cdc509c21f6d43e257b3" +version = "0.1.1" +source = "git+https://github.com/OpenDevicePartnership/bq25773#0ca102dec24a617df5762cf0bf4217fd387d5c63" dependencies = [ "device-driver", - "embedded-batteries-async 0.2.1", + "embedded-batteries-async", "embedded-hal 1.0.0", "embedded-hal-async", ] [[package]] name = "bq40z50-rx" -version = "0.8.0" +version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "55262eaa8d376e3db634b2cf851d2f255fbe86076abc3d4f8088248340836bb6" +checksum = "09b6faf600295f12c3fb99b45266bc9140af5c344b08f2705bc06bfa0e8b549e" dependencies = [ "device-driver", - "embedded-batteries-async 0.3.4", + "embedded-batteries-async", "embedded-hal 1.0.0", "embedded-hal-async", "smbus-pec", @@ -254,9 +193,9 @@ dependencies = [ [[package]] name = "bytemuck" -version = "1.23.1" +version = "1.25.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c76a5792e44e4abe34d3abf15636779261d45a7450612059293d1d2cfc63422" +checksum = "c8efb64bd706a16a1bdde310ae86b351e4d21550d98d056f22f8a7f7a2183fec" [[package]] name = "byteorder" @@ -266,18 +205,9 @@ checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" [[package]] name = "cfg-if" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9555578bc9e57714c812a1f84e4fc5b4d21fcb063490c624de019f7464c91268" - -[[package]] -name = "convert_case" -version = "0.6.0" +version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec182b0ca2f35d8fc196cf3404988fd8b8c739a4d270ff118a398feb0cbec1ca" -dependencies = [ - "unicode-segmentation", -] +checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" [[package]] name = "cortex-m" @@ -309,7 +239,7 @@ checksum = "e37549a379a9e0e6e576fd208ee60394ccb8be963889eebba3ffe0980364f472" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn", ] [[package]] @@ -345,7 +275,7 @@ dependencies = [ "proc-macro2", "quote", "strsim", - "syn 2.0.104", + "syn", ] [[package]] @@ -356,16 +286,7 @@ checksum = "fc34b93ccb385b40dc71c6fceac4b2ad23662c7eeb248cf10d529b7e055b6ead" dependencies = [ "darling_core", "quote", - "syn 2.0.104", -] - -[[package]] -name = "dd-manifest-tree" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5793572036e0a6638977c7370c6afc423eac848ee8495f079b8fd3964de7b9f9" -dependencies = [ - "yaml-rust2", + "syn", ] [[package]] @@ -406,7 +327,7 @@ dependencies = [ "proc-macro-error2", "proc-macro2", "quote", - "syn 2.0.104", + "syn", ] [[package]] @@ -415,7 +336,7 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "10d60334b3b2e7c9d91ef8150abfb6fa4c1c39ebbcf4a81c2e346aad939fee3e" dependencies = [ - "thiserror 2.0.16", + "thiserror", ] [[package]] @@ -430,49 +351,19 @@ dependencies = [ [[package]] name = "device-driver" -version = "1.0.6" +version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1298272ea07037196af2fe8d1eb50792206f45476d79eefa435432b9323cf488" +checksum = "af0e43acfcbb0bb3b7435cc1b1dbb33596cacfec1eb243336b74a398e0bd6cbf" dependencies = [ - "device-driver-macros", "embedded-io", "embedded-io-async", ] -[[package]] -name = "device-driver-generation" -version = "1.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f86a17ed060a6119daeb05ad5596ac3bd945f7ab2213cc6260bf6a7623e73da1" -dependencies = [ - "anyhow", - "askama", - "bitvec", - "convert_case", - "dd-manifest-tree", - "itertools 0.14.0", - "kdl", - "proc-macro2", - "quote", - "syn 2.0.104", -] - -[[package]] -name = "device-driver-macros" -version = "1.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e1c29238099c66bf44098efaa772cae6f47d632aebb7ade8d3087bd565e8fae0" -dependencies = [ - "device-driver-generation", - "proc-macro2", - "syn 2.0.104", -] - [[package]] name = "document-features" -version = "0.2.11" +version = "0.2.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "95249b50c6c185bee49034bcb378a49dc2b5dff0be90ff6616d31d64febab05d" +checksum = "d4b8a88685455ed29a21542a33abd9cb6510b6b129abadabdcef0f4c55bc8f61" dependencies = [ "litrs", ] @@ -524,7 +415,7 @@ dependencies = [ "darling", "proc-macro2", "quote", - "syn 2.0.104", + "syn", ] [[package]] @@ -646,7 +537,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "31e14d288a59ef41f4e05468eae9b1c9fef6866977cea86d3f1a1ced295b6cab" dependencies = [ "bitfield-struct 0.10.1", - "bitflags 2.9.1", + "bitflags 2.11.0", "defmt 0.3.100", "embedded-hal 1.0.0", "zerocopy", @@ -654,28 +545,17 @@ dependencies = [ [[package]] name = "embedded-batteries" -version = "0.3.1" +version = "0.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b8996d7168535579180a0eead82efaba718ebd598782f986bfd635458259df2" +checksum = "40f975432b4e146342a1589c563cffab6b7a692024cb511bf87b6bfe78c84125" dependencies = [ - "bitfield-struct 0.10.1", - "bitflags 2.9.1", + "bitfield-struct 0.12.1", + "bitflags 2.11.0", "defmt 0.3.100", "embedded-hal 1.0.0", "zerocopy", ] -[[package]] -name = "embedded-batteries-async" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "98cb543f4eea7e2c57544f345a5cf40fd90e9d3593b96cb7515f6c1d62c7fc68" -dependencies = [ - "bitfield-struct 0.10.1", - "embedded-batteries 0.2.1", - "embedded-hal 1.0.0", -] - [[package]] name = "embedded-batteries-async" version = "0.3.4" @@ -684,14 +564,14 @@ checksum = "a3bf0e4be67770cfc31f1cea8b73baf98c0baf2c57d6bd8c3a4c315acb1d8bd4" dependencies = [ "bitfield-struct 0.12.1", "defmt 0.3.100", - "embedded-batteries 0.3.1", + "embedded-batteries 0.3.4", "embedded-hal 1.0.0", ] [[package]] name = "embedded-cfu-protocol" version = "0.2.0" -source = "git+https://github.com/OpenDevicePartnership/embedded-cfu#a4cc8707842b878048447abbf2af4efa79fed368" +source = "git+https://github.com/OpenDevicePartnership/embedded-cfu#e0d776017cf34c902c9f2a2be0c75fe73a3a4dda" dependencies = [ "defmt 0.3.100", "embedded-io-async", @@ -767,7 +647,7 @@ name = "embedded-services" version = "0.1.0" dependencies = [ "bitfield 0.17.0", - "bitflags 2.9.1", + "bitflags 2.11.0", "bitvec", "cfg-if", "cortex-m", @@ -814,27 +694,18 @@ source = "git+https://github.com/OpenDevicePartnership/embedded-usb-pd#9a42f07ce dependencies = [ "aquamarine", "bincode", - "bitfield 0.19.1", + "bitfield 0.19.4", "defmt 0.3.100", "embedded-hal-async", ] -[[package]] -name = "encoding_rs" -version = "0.8.35" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "75030f3c4f45dafd7586dd6780965a8c7e8e285a5ecb86713e63a79c5b2766f3" -dependencies = [ - "cfg-if", -] - [[package]] name = "espi-device" version = "0.1.0" -source = "git+https://github.com/OpenDevicePartnership/haf-ec-service#e9c43ec493ba9c4e3db84c73530f919448c07b6d" +source = "git+https://github.com/OpenDevicePartnership/haf-ec-service#9805f13c044b0e314d415410c57a8a59a40eabeb" dependencies = [ "bit-register", - "bitflags 2.9.1", + "bitflags 2.11.0", "num-traits", "num_enum", "static_assertions", @@ -858,15 +729,16 @@ dependencies = [ "embedded-services", "mctp-rs", "num_enum", + "odp-service-common", "thermal-service-messages", "time-alarm-service-messages", ] [[package]] name = "fixed" -version = "1.29.0" +version = "1.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "707070ccf8c4173548210893a0186e29c266901b71ed20cd9e2ca0193dfe95c3" +checksum = "c566da967934c6c7ee0458a9773de9b2a685bd2ce26a3b28ddfc740e640182f5" dependencies = [ "az", "bytemuck", @@ -888,9 +760,9 @@ checksum = "e6d5a32815ae3f33302d95fdcb2ce17862f8c65363dcfd29360480ba1001fc9c" [[package]] name = "futures" -version = "0.3.31" +version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "65bc07b1a8bc7c85c5f2e110c476c7389b4554ba72af57d8445ea63a576b0876" +checksum = "8b147ee9d1f6d097cef9ce628cd2ee62288d963e16fb287bd9286455b241382d" dependencies = [ "futures-channel", "futures-core", @@ -902,9 +774,9 @@ dependencies = [ [[package]] name = "futures-channel" -version = "0.3.31" +version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2dff15bf788c671c1934e366d07e30c1814a8ef514e1af724a602e8a2fbe1b10" +checksum = "07bbe89c50d7a535e539b8c17bc0b49bdb77747034daa8087407d655f3f7cc1d" dependencies = [ "futures-core", "futures-sink", @@ -912,61 +784,61 @@ dependencies = [ [[package]] name = "futures-core" -version = "0.3.31" +version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05f29059c0c2090612e8d742178b0580d2dc940c837851ad723096f87af6663e" +checksum = "7e3450815272ef58cec6d564423f6e755e25379b217b0bc688e295ba24df6b1d" [[package]] name = "futures-io" -version = "0.3.31" +version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e5c1b78ca4aae1ac06c48a526a655760685149f0d465d21f37abfe57ce075c6" +checksum = "cecba35d7ad927e23624b22ad55235f2239cfa44fd10428eecbeba6d6a717718" [[package]] name = "futures-macro" -version = "0.3.31" +version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "162ee34ebcb7c64a8abebc059ce0fee27c2262618d7b60ed8faf72fef13c3650" +checksum = "e835b70203e41293343137df5c0664546da5745f82ec9b84d40be8336958447b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn", ] [[package]] name = "futures-sink" -version = "0.3.31" +version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e575fab7d1e0dcb8d0c7bcf9a63ee213816ab51902e6d244a95819acacf1d4f7" +checksum = "c39754e157331b013978ec91992bde1ac089843443c49cbc7f46150b0fad0893" [[package]] name = "futures-task" -version = "0.3.31" +version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f90f7dce0722e95104fcb095585910c0977252f286e354b5e3bd38902cd99988" +checksum = "037711b3d59c33004d3856fbdc83b99d4ff37a24768fa1be9ce3538a1cde4393" [[package]] name = "futures-util" -version = "0.3.31" +version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9fa08315bb612088cc391249efdc3bc77536f16c91f6cf495e6fbe85b20a4a81" +checksum = "389ca41296e6190b48053de0321d02a77f32f8a5d2461dd38762c0593805c6d6" dependencies = [ "futures-core", "futures-macro", "futures-sink", "futures-task", "pin-project-lite", - "pin-utils", ] [[package]] name = "half" -version = "2.6.0" +version = "2.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "459196ed295495a68f7d7fe1d84f6c4b7ff0e21fe3017b2f283c6fac3ad803c9" +checksum = "6ea2d84b969582b4b1864a92dc5d27cd2b77b622a8d79306834f1be5ba20d84b" dependencies = [ "cfg-if", "crunchy", + "zerocopy", ] [[package]] @@ -978,24 +850,6 @@ dependencies = [ "byteorder", ] -[[package]] -name = "hashbrown" -version = "0.14.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" -dependencies = [ - "ahash", -] - -[[package]] -name = "hashlink" -version = "0.9.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ba4ff7128dee98c7dc9794b6a411377e1404dba1c97deb8d1a55297bd25d8af" -dependencies = [ - "hashbrown", -] - [[package]] name = "heapless" version = "0.8.0" @@ -1008,9 +862,9 @@ dependencies = [ [[package]] name = "heck" -version = "0.4.1" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" +checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" [[package]] name = "ident_case" @@ -1055,38 +909,11 @@ dependencies = [ "either", ] -[[package]] -name = "itertools" -version = "0.14.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2b192c782037fadd9cfa75548310488aabdbf3d2da73885b31bd0abd03351285" -dependencies = [ - "either", -] - -[[package]] -name = "itoa" -version = "1.0.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4a5f13b858c8d314ee3e8f639011f7ccefe71f97f96e50151fb991f267928e2c" - -[[package]] -name = "kdl" -version = "6.3.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "12661358400b02cbbf1fbd05f0a483335490e8a6bd1867620f2eeb78f304a22f" -dependencies = [ - "miette", - "num", - "thiserror 1.0.69", - "winnow 0.6.24", -] - [[package]] name = "litrs" -version = "0.4.1" +version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b4ce301924b7887e9d637144fdade93f9dfff9b60981d4ac161db09720d39aa5" +checksum = "11d3d7f243d5c5a8b9bb5d6dd2b1602c0cb0b9db1621bafc7ed66e35ff9fe092" [[package]] name = "mctp-rs" @@ -1099,35 +926,7 @@ dependencies = [ "espi-device", "num_enum", "smbus-pec", - "thiserror 2.0.16", -] - -[[package]] -name = "memchr" -version = "2.7.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32a282da65faaf38286cf3be983213fcf1d2e2a58700e808f83f4ea9a4804bc0" - -[[package]] -name = "miette" -version = "7.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f98efec8807c63c752b5bd61f862c165c115b0a35685bdcfd9238c7aeb592b7" -dependencies = [ - "cfg-if", - "miette-derive", - "unicode-width", -] - -[[package]] -name = "miette-derive" -version = "7.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "db5b29714e950dbb20d5e6f74f9dcec4edbcc1067bb7f8ed198c097b8c1a818b" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.104", + "thiserror", ] [[package]] @@ -1180,70 +979,6 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8d5439c4ad607c3c23abf66de8c8bf57ba8adcd1f129e699851a6e43935d339d" -[[package]] -name = "num" -version = "0.4.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "35bd024e8b2ff75562e5f34e7f4905839deb4b22955ef5e73d2fea1b9813cb23" -dependencies = [ - "num-bigint", - "num-complex", - "num-integer", - "num-iter", - "num-rational", - "num-traits", -] - -[[package]] -name = "num-bigint" -version = "0.4.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a5e44f723f1133c9deac646763579fdb3ac745e418f2a7af9cd0c431da1f20b9" -dependencies = [ - "num-integer", - "num-traits", -] - -[[package]] -name = "num-complex" -version = "0.4.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "73f88a1307638156682bada9d7604135552957b7818057dcef22705b4d509495" -dependencies = [ - "num-traits", -] - -[[package]] -name = "num-integer" -version = "0.1.46" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7969661fd2958a5cb096e56c8e1ad0444ac2bbcd0061bd28660485a44879858f" -dependencies = [ - "num-traits", -] - -[[package]] -name = "num-iter" -version = "0.1.45" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1429034a0490724d0075ebb2bc9e875d6503c3cf69e235a8941aa757d83ef5bf" -dependencies = [ - "autocfg", - "num-integer", - "num-traits", -] - -[[package]] -name = "num-rational" -version = "0.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" -dependencies = [ - "num-bigint", - "num-integer", - "num-traits", -] - [[package]] name = "num-traits" version = "0.2.19" @@ -1271,14 +1006,15 @@ checksum = "ff32365de1b6743cb203b710788263c44a03de03802daf96092f2da4fe6ba4d7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn", ] [[package]] -name = "once_cell" -version = "1.21.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "42f5e15c9953c5e4ccceeb2e7382a716482c34515315f7b03532b8b4e8393d2d" +name = "odp-service-common" +version = "0.1.0" +dependencies = [ + "embedded-services", +] [[package]] name = "panic-probe" @@ -1296,29 +1032,17 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" -[[package]] -name = "percent-encoding" -version = "2.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e3148f5046208a5d56bcfc03053e3ca6334e51da8dfb19b6cdc8b306fae3283e" - [[package]] name = "pin-project-lite" version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3b3cff922bd51709b605d9ead9aa71031d81447142d828eb4a6eba76fe619f9b" -[[package]] -name = "pin-utils" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" - [[package]] name = "portable-atomic" -version = "1.11.1" +version = "1.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f84267b20a16ea918e43c6a88433c2d54fa145c92a811b5b047ccbe153674483" +checksum = "c33a9471896f1c69cecef8d20cbe2f7accd12527ce60845ff44c153bb2a21b49" [[package]] name = "power-policy-interface" @@ -1328,7 +1052,7 @@ dependencies = [ "defmt 0.3.100", "embassy-futures", "embassy-sync", - "embedded-batteries-async 0.3.4", + "embedded-batteries-async", "embedded-services", "heapless", "num_enum", @@ -1353,23 +1077,23 @@ dependencies = [ "proc-macro-error-attr2", "proc-macro2", "quote", - "syn 2.0.104", + "syn", ] [[package]] name = "proc-macro2" -version = "1.0.95" +version = "1.0.106" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "02b3e5e68a3a1a02aad3ec490a98007cbc13c37cbe84a3cd7b8e406d76e7f778" +checksum = "8fd00f0bb2e90d81d1044c2b32617f68fcb9fa3bb7640c23e9c748e53fb30934" dependencies = [ "unicode-ident", ] [[package]] name = "quote" -version = "1.0.40" +version = "1.0.44" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1885c039570dc00dcb4ff087a89e185fd56bae234ddc7f056a945bf36467248d" +checksum = "21b2ebcf727b7760c461f091f9f0f539b77b8e87f2fd88131e7f1b433b3cece4" dependencies = [ "proc-macro2", ] @@ -1418,7 +1142,7 @@ dependencies = [ "embassy-imxrt", "embassy-sync", "embassy-time", - "embedded-batteries-async 0.3.4", + "embedded-batteries-async", "embedded-hal-async", "embedded-services", "espi-service", @@ -1430,12 +1154,6 @@ dependencies = [ "static_cell", ] -[[package]] -name = "rustc-hash" -version = "2.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "357703d41365b4b27c590e3ed91eabb1b663f07c4c084095e60cbed4362dff0d" - [[package]] name = "rustc_version" version = "0.2.3" @@ -1451,12 +1169,6 @@ version = "1.0.22" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d" -[[package]] -name = "ryu" -version = "1.0.20" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "28d3b2b1366ec20994f1fd18c3c594f05c5dd4bc44d8bb0c1c632c8d6829481f" - [[package]] name = "semver" version = "0.9.0" @@ -1474,34 +1186,32 @@ checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" [[package]] name = "serde" -version = "1.0.219" +version = "1.0.228" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f0e2c6ed6606019b4e29e69dbaba95b11854410e5347d525002456dbbb786b6" +checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e" dependencies = [ + "serde_core", "serde_derive", ] [[package]] -name = "serde_derive" -version = "1.0.219" +name = "serde_core" +version = "1.0.228" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b0276cf7f2c73365f7157c8123c21cd9a50fbbd844757af28ca1f5925fc2a00" +checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad" dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.104", + "serde_derive", ] [[package]] -name = "serde_json" -version = "1.0.141" +name = "serde_derive" +version = "1.0.228" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30b9eff21ebe718216c6ec64e1d9ac57087aad11efc64e32002bce4a0d4c03d3" +checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ - "itoa", - "memchr", - "ryu", - "serde", + "proc-macro2", + "quote", + "syn", ] [[package]] @@ -1515,9 +1225,9 @@ dependencies = [ [[package]] name = "stable_deref_trait" -version = "1.2.0" +version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" +checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "static_assertions" @@ -1547,32 +1257,21 @@ checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" [[package]] name = "subenum" -version = "1.1.2" +version = "1.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4f5d5dfb8556dd04017db5e318bbeac8ab2b0c67b76bf197bfb79e9b29f18ecf" +checksum = "ec3d08fe7078c57309d5c3d938e50eba95ba1d33b9c3a101a8465fc6861a5416" dependencies = [ "heck", "proc-macro2", "quote", - "syn 1.0.109", + "syn", ] [[package]] name = "syn" -version = "1.0.109" +version = "2.0.117" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72b64191b275b66ffe2469e8af2c1cfe3bafa67b529ead792a6d0160888b4237" -dependencies = [ - "proc-macro2", - "quote", - "unicode-ident", -] - -[[package]] -name = "syn" -version = "2.0.104" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "17b6f705963418cdb9927482fa304bc562ece2fdd4f616084c50b7023b435a40" +checksum = "e665b8803e7b1d2a727f4023456bbbbe74da67099c585258af0ad9c5013b9b99" dependencies = [ "proc-macro2", "quote", @@ -1597,42 +1296,22 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.69" +version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" +checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" dependencies = [ - "thiserror-impl 1.0.69", -] - -[[package]] -name = "thiserror" -version = "2.0.16" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3467d614147380f2e4e374161426ff399c91084acd2363eaf549172b3d5e60c0" -dependencies = [ - "thiserror-impl 2.0.16", + "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.69" +version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" +checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", -] - -[[package]] -name = "thiserror-impl" -version = "2.0.16" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c5e1be1c48b9172ee610da68fd9cd2770e7a4056cb3fc98710ee6906f0c7960" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.104", + "syn", ] [[package]] @@ -1649,27 +1328,15 @@ dependencies = [ [[package]] name = "typenum" -version = "1.18.0" +version = "1.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1dccffe3ce07af9386bfd29e80c0ab1a8205a2fc34e4bcd40364df902cfa8f3f" +checksum = "562d481066bde0658276a35467c4af00bdc6ee726305698a55b86e61d7ad82bb" [[package]] name = "unicode-ident" -version = "1.0.18" +version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a5f39404a5da50712a4c1eecf25e90dd62b613502b7e925fd4e4d19b5c96512" - -[[package]] -name = "unicode-segmentation" -version = "1.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6ccf251212114b54433ec949fd6a7841275f9ada20dddd2f29e9ceea4501493" - -[[package]] -name = "unicode-width" -version = "0.1.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" +checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" [[package]] name = "unty" @@ -1689,12 +1356,6 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "77439c1b53d2303b20d9459b1ade71a83c716e3f9c34f3228c00e6f185d6c002" -[[package]] -name = "version_check" -version = "0.9.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" - [[package]] name = "void" version = "1.0.2" @@ -1710,24 +1371,6 @@ dependencies = [ "vcell", ] -[[package]] -name = "winnow" -version = "0.6.24" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c8d71a593cc5c42ad7876e2c1fda56f314f3754c084128833e64f1345ff8a03a" -dependencies = [ - "memchr", -] - -[[package]] -name = "winnow" -version = "0.7.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f3edebf492c8125044983378ecb5766203ad3b4c2f7a922bd7dd207f6d443e95" -dependencies = [ - "memchr", -] - [[package]] name = "wyz" version = "0.5.1" @@ -1737,33 +1380,22 @@ dependencies = [ "tap", ] -[[package]] -name = "yaml-rust2" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a1a1c0bc9823338a3bdf8c61f994f23ac004c6fa32c08cd152984499b445e8d" -dependencies = [ - "arraydeque", - "encoding_rs", - "hashlink", -] - [[package]] name = "zerocopy" -version = "0.8.26" +version = "0.8.39" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1039dd0d3c310cf05de012d8a39ff557cb0d23087fd44cad61df08fc31907a2f" +checksum = "db6d35d663eadb6c932438e763b262fe1a70987f9ae936e60158176d710cae4a" dependencies = [ "zerocopy-derive", ] [[package]] name = "zerocopy-derive" -version = "0.8.26" +version = "0.8.39" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9ecf5b4cc5364572d7f4c329661bcc82724222973f2cab6f050a4e5c22f75181" +checksum = "4122cd3169e94605190e77839c9a40d40ed048d305bfdc146e7df40ab0f3e517" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn", ] diff --git a/examples/rt685s-evk/Cargo.lock b/examples/rt685s-evk/Cargo.lock index b01a71d8..b69ff511 100644 --- a/examples/rt685s-evk/Cargo.lock +++ b/examples/rt685s-evk/Cargo.lock @@ -1231,6 +1231,13 @@ dependencies = [ "syn 2.0.106", ] +[[package]] +name = "odp-service-common" +version = "0.1.0" +dependencies = [ + "embedded-services", +] + [[package]] name = "once_cell" version = "1.21.3" @@ -1409,6 +1416,7 @@ dependencies = [ "futures", "mimxrt600-fcb 0.1.0", "mimxrt685s-pac 0.4.0", + "odp-service-common", "panic-probe", "platform-service", "power-button-service", @@ -1628,6 +1636,7 @@ dependencies = [ "embassy-time", "embedded-mcu-hal", "embedded-services", + "odp-service-common", "time-alarm-service-messages", "zerocopy", ] diff --git a/examples/rt685s-evk/Cargo.toml b/examples/rt685s-evk/Cargo.toml index 4c5310c5..18c0f795 100644 --- a/examples/rt685s-evk/Cargo.toml +++ b/examples/rt685s-evk/Cargo.toml @@ -51,6 +51,7 @@ mimxrt685s-pac = { version = "*", features = ["rt", "critical-section"] } embedded-cfu-protocol = { git = "https://github.com/OpenDevicePartnership/embedded-cfu" } embedded-services = { path = "../../embedded-service", features = ["defmt"] } +odp-service-common = { path = "../../odp-service-common" } power-button-service = { path = "../../power-button-service", features = [ "defmt", ] } diff --git a/examples/rt685s-evk/src/bin/time_alarm.rs b/examples/rt685s-evk/src/bin/time_alarm.rs index be035eb5..b3ea0808 100644 --- a/examples/rt685s-evk/src/bin/time_alarm.rs +++ b/examples/rt685s-evk/src/bin/time_alarm.rs @@ -23,7 +23,7 @@ async fn main(spawner: embassy_executor::Spawner) { embedded_services::init().await; info!("services initialized"); - let time_service = embedded_services::spawn_service!( + let time_service = odp_service_common::spawn_service!( spawner, time_alarm_service::Service<'static>, time_alarm_service::InitParams { diff --git a/odp-service-common/Cargo.toml b/odp-service-common/Cargo.toml new file mode 100644 index 00000000..5eadba5a --- /dev/null +++ b/odp-service-common/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "odp-service-common" +version.workspace = true +edition.workspace = true +license.workspace = true +repository.workspace = true + +[dependencies] +embedded-services.workspace = true + +[lints] +workspace = true diff --git a/odp-service-common/src/lib.rs b/odp-service-common/src/lib.rs new file mode 100644 index 00000000..6ae7498c --- /dev/null +++ b/odp-service-common/src/lib.rs @@ -0,0 +1,4 @@ +//! This crate contains code that is common to multiple ODP service implementations. +#![no_std] + +pub mod runnable_service; diff --git a/embedded-service/src/service.rs b/odp-service-common/src/runnable_service.rs similarity index 92% rename from embedded-service/src/service.rs rename to odp-service-common/src/runnable_service.rs index 646aa55d..bfeeef87 100644 --- a/embedded-service/src/service.rs +++ b/odp-service-common/src/runnable_service.rs @@ -32,7 +32,7 @@ pub trait Service<'hw>: Sized { /// on other async runtimes). pub trait ServiceRunner<'hw> { /// Run the service event loop. This future never completes. - fn run(self) -> impl core::future::Future + 'hw; + fn run(self) -> impl core::future::Future + 'hw; } /// Initializes a service, creates an embassy task to run it, and spawns that task. @@ -63,12 +63,12 @@ pub trait ServiceRunner<'hw> { #[macro_export] macro_rules! spawn_service { ($spawner:expr, $service_ty:ty, $init_arg:expr) => {{ - use $crate::service::{Service, ServiceRunner}; + use $crate::runnable_service::{Service, ServiceRunner}; static SERVICE_RESOURCES: StaticCell<(<$service_ty as Service>::Resources)> = StaticCell::new(); let service_resources = SERVICE_RESOURCES.init(<<$service_ty as Service>::Resources as Default>::default()); #[embassy_executor::task] - async fn service_task_fn(runner: <$service_ty as $crate::service::Service<'static>>::Runner) { + async fn service_task_fn(runner: <$service_ty as $crate::runnable_service::Service<'static>>::Runner) { runner.run().await; } diff --git a/time-alarm-service/Cargo.toml b/time-alarm-service/Cargo.toml index a1e1d12b..2b2c4af4 100644 --- a/time-alarm-service/Cargo.toml +++ b/time-alarm-service/Cargo.toml @@ -16,6 +16,7 @@ embassy-sync.workspace = true embassy-time.workspace = true embedded-mcu-hal.workspace = true embedded-services.workspace = true +odp-service-common.workspace = true time-alarm-service-messages.workspace = true zerocopy.workspace = true @@ -36,6 +37,6 @@ workspace = true [dev-dependencies] tokio = { workspace = true, features = ["rt", "macros", "time"] } -critical-section = { version = "1.1", features = ["std"]} +critical-section = { version = "1.1", features = ["std"] } embassy-time = { workspace = true, features = ["std", "generic-queue-8"] } embassy-time-driver = { workspace = true } diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index b5ee4769..84613cef 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -300,7 +300,7 @@ pub struct Runner<'hw> { service: &'hw ServiceInner<'hw>, } -impl<'hw> embedded_services::service::ServiceRunner<'hw> for Runner<'hw> { +impl<'hw> odp_service_common::runnable_service::ServiceRunner<'hw> for Runner<'hw> { /// Run the service. async fn run(self) -> embedded_services::Never { loop { @@ -373,7 +373,7 @@ impl<'hw> Service<'hw> { } } -impl<'hw> embedded_services::service::Service<'hw> for Service<'hw> { +impl<'hw> odp_service_common::runnable_service::Service<'hw> for Service<'hw> { type Runner = Runner<'hw>; type ErrorType = DatetimeClockError; type InitParams = InitParams<'hw>; diff --git a/time-alarm-service/tests/tad_test.rs b/time-alarm-service/tests/tad_test.rs index f9fb41d8..25deaa48 100644 --- a/time-alarm-service/tests/tad_test.rs +++ b/time-alarm-service/tests/tad_test.rs @@ -8,7 +8,7 @@ mod common; mod test { use embassy_time::Timer; use embedded_mcu_hal::time::{Datetime, DatetimeClock}; - use embedded_services::service::{Service, ServiceRunner}; + use odp_service_common::runnable_service::{Service, ServiceRunner}; use time_alarm_service_messages as msg; From afe558ecf310f0c6630dadbc20eb87adb060bacf Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 9 Mar 2026 14:35:50 -0700 Subject: [PATCH 05/17] readme --- docs/api-guidelines.md | 254 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 254 insertions(+) create mode 100644 docs/api-guidelines.md diff --git a/docs/api-guidelines.md b/docs/api-guidelines.md new file mode 100644 index 00000000..5f04112c --- /dev/null +++ b/docs/api-guidelines.md @@ -0,0 +1,254 @@ +# Service API Guidelines + +This document establishes some guidelines that APIs in this repo should try to conform to, and explains the rationale behind those guidelines to help guide decisionmaking when tradeoffs need to be made. + +These guidelines attempt to make our APIs easier to compose, test, and use. + +## Guidelines + +### No 'static references + +References with lifetime `'static` in API functions should be avoided, even if the lifetime will always be `'static` in production use cases. Instead, make your type generic over a lifetime with the expectation that that lifetime will be `'static` in production use cases. + +__Reason__: Testability. If something needs to take a reference to an object 'O' with lifetime `'static`, that means that O can never be destroyed. This can make it pretty difficult to test things that use that API. + +__Example__: +Instead of this: +```rust +trait Subscriber {} +struct Notifier { subscriber: &'static Subscriber } +// ^^^^^^^^ + +impl Notifier { + fn new(subscriber: &'static Subscriber) -> Self { + // ^^^^^^^^ + Self { subscriber } + } +} +``` + +Consider something like this: +```rust +trait Subscriber {} +struct Notifier<'sub> { subscriber: &'sub Subscriber } +// ^^^^^^ ^^^^^ + +impl Notifier { + fn new(subscriber: &'sub Subscriber) -> Self { + // ^^^^^ + Self { subscriber } + } +} +``` + +### External memory allocation / no static memory allocation + +Memory allocation should always be the role of the caller of the API. If you need memory, have your caller pass it into your constructor. Do not have things like `static INSTANCE: OnceLock` in your service module. + +Note that while this applies to code in this repo, it does not necessarily apply to other ODP repos (e.g. HAL crates that know exactly how many instances of peripheral X are available on the platform). + +__Reason__: Most code in this repos is expected to run primarily in environments that don't have a heap. In heapless environments, your options are either to have your caller provide you memory or to allocate it as a static variable in your module. Allocating it as a static variable in your module has negative impacts on flexibility, testability, performance, and code size. +Flexibility - Memory allocation in your module rather than by your caller means that the size of your object must be known when the module is compiled rather than when you're instantiated. This prevents you from storing any owned caller-provided types in your object (since you can't know those types when your module is compiled). +Testability - if you have a private singleton instance, tests can't arbitrary destroy and recreate that state. This makes it difficult to test multiple startup paths. +Performance - if you can't be generic over a type, the only way you can interact with user-provided types is by dyn references to trait impls. This means you have to pay for dynamic dispatch and the compiler can't optimize or inline across that dyn boundary. +Code size - The compiler has to generate a bunch of code to handle dynamic dispatch, even if there's only ever a single concrete type that implements the trait you want to be generic over. + +__Example__: +Note that in the below example, the `OnceLock` / external `Resources` is only necessary if you need to hand out references to the contents of the `OnceLock` / `FooInner`. That's elided in the example and assumed to be implemented in the `/* ... */` blocks for simplicity. +```rust +pub struct Foo { /* ... */ } + +static INSTANCE: OnceLock = OnceLock::new(); + +impl Foo { + async fn init(/* ... */) -> &'static Foo { + INSTANCE.get_or_init(|| Foo{ /* ... */ }).await + } +} +``` + +Consider something like this: +```rust +struct FooInner<'hw> { /* ... */ } + +#[derive(Default)] +pub struct Resources<'hw> { + inner: Option +} + +pub struct Foo<'hw> { + inner: &'hw FooInner +} + +impl<'hw> Foo<'hw> { + fn new(resources: &'hw mut Resources, /* ... */) -> Self { + let inner = resources.insert(FooInner::new(/* ... */)); + Self{ inner } + } +} +``` + + +### Use runner objects for concurrency + +Don't declare embassy tasks in your module - instead, have the constructor for your type return a `(Self, Runner)` tuple. The `Runner` object should have a single method `run(self) -> !` that the entity that instantiated your object must execute. You should have only one `Runner` object returned. Use the `odp-service-common::runnable_service::Service` trait to enforce this patttern. + +__Reason__: Declarations of embassy tasks are functionally static memory allocations. They can't be generic and you have to declare at declaration time a maximum number of instances that can be run concurrently. They also commit you to running on embassy, which is not necessarily desirable in test contexts. Pushing responsibility for the allocation out of your module allows your types to be generic. +However, it also means that your caller needs to be able to declare a task that can run your runner, and if you have multiple things that each need different pieces of state and need to run concurrently, setting up those tasks can make your API unwieldly and brittle. +Returning a simple `Runner` object at the same time as your object makes it difficult to forget to execute the runner. +Allowing only a single `Runner` with only one method that takes no external arguments makes it difficult to misuse the runner. + +__Example__: +Instead of this: +```rust +///// Your type's definition ///// +struct MyRunnableTypeInner { /* ... */ } + +impl<'hw> MyRunnableTypeInner<'hw> { + /* ... */ +} + +#[derive(Default)] +pub struct Resources<'hw> { + inner: Option +} + +pub struct MyRunnableType<'hw> { + inner: &'hw MyRunnableTypeInner +} + +impl<'hw> MyRunnableType<'hw> { + fn new(resources: &mut Resources, /* ... */ ) -> Self { + let inner = resources.insert(RunnableTypeInner::new(/* ... */)) + /* ... */ + Self { inner } + } +} + +mod tasks { + pub async fn run_task_1<'hw>(runnable: &'hw MyRunnableType, foo: Foo) -> ! { /* ... */ } + pub async fn run_task_2<'hw>(runnable: &'hw MyRunnableType, bar: Bar) -> ! { /* ... */ } + pub async fn run_task_3<'hw>(runnable: &'hw MyRunnableType, baz: Baz) -> ! { /* ... */ } +} + +///// End-user code ///// + +fn main() { + let instance = MyRunnableType::new(/* ... */); + #[embassy_task] + fn runner_1(runnable: &'static MyRunnableType, foo: Foo) -> ! { + my_runnable_type::tasks::run_task_1(runnable, foo).await + } + #[embassy_task] + fn runner_2(runnable: &'static MyRunnableType, bar: Bar) -> ! { + my_runnable_type::tasks::run_task_2(runnable, bar).await + } + #[embassy_task] + fn runner_3(runnable: &'static MyRunnableType, baz: Baz) -> ! { + my_runnable_type::tasks::run_task_3(runnable, baz).await + } + + spawner.must_spawn(runner_1(&instance, Foo::new( /* ... */ ))); + spawner.must_spawn(runner_1(&instance, Bar::new( /* ... */ ))); + spawner.must_spawn(runner_1(&instance, Baz::new( /* ... */ ))); +} + +``` + +Consider something like this: +```rust +///// Your type's definition ///// +struct MyRunnableTypeInner { /* ... */ } + +impl<'hw> MyRunnableTypeInner<'hw> { + async fn task_1(&self, foo: Foo) -> ! { /* ... */ } + async fn task_2(&self, bar: Bar) -> ! { /* ... */ } + async fn task_3(&self, baz: Baz) -> ! { /* ... */ } +} + +#[derive(Default)] +pub struct Resources<'hw> { + inner: Option +} + +pub struct MyRunnableType<'hw> { + inner: &'hw MyRunnableTypeInner +} + +pub struct Runner<'hw> { + inner: &'hw MyRunnableTypeInner, + foo: Foo, + bar: Bar, + baz: Baz +} + +impl<'hw> Runner<'hw> { + fn run(self) -> ! { + loop { + embassy_sync::join::join3( + self.inner.task_1(self.foo), + self.inner.task_2(self.bar), + self.inner.task_3(self.baz) + ).await; + } + } +} + +impl<'hw> MyRunnableType<'hw> { + fn new(resources: &mut Resources, foo: Foo, bar: Bar, baz: Baz /* ... */ ) -> (Self, Runner) { + let inner = resources.insert(RunnableTypeInner::new( /* ... */ )); + (Self { inner }, Runner { inner, foo, bar, baz }) + } +} + +///// End-user code ///// + +fn main() { + let (instance, runner) = MyRunnableType::new(/* ... */); + #[embassy_task] + fn runner_fn(runner: Runner) { + runner.run().await + } + + spawner.must_spawn(runner_fn(runner)); +} +``` +Notice that most of the complexity has been moved into internal implementation details and the client doesn't have to think about it. Also notice that if you want to add a new 'green thread', or change what state is available to which 'green threads' you can do that entirely in private code in the `run()` method, without requiring changes to your client. + +### Use traits for public methods expected to be used at run time whenever possible + +In most cases, public APIs in this repo should be exposed in terms of traits rather than methods directly on the object, and objects that need references to other embedded-services objects should reference them by trait rather than by name. This does not apply to public methods used to construct or initialize a service, because those generally need to know something about the concrete implementation type to properly initialize it. + +__Reason__: Improved testability and customizability. +Testability - if all our types interact with each other via traits rather than direct dependencies on the type, it makes it much easier to mock out individual components. +Customizability - if an OEM needs to insert a special behavior, they can substitute in a different implementation of that trait and continue using the rest of the embedded-services code without modification. + +__Example__: +Instead of +```rust +pub struct MyService { /* */ } +impl MyService { + fn foo(&mut self) -> Result<()> { /* ... */ } + fn bar(&mut self) -> Result<()> { /* ... */ } + fn baz(&mut self) -> Result<()> { /* ... */ } +} +``` + +Consider: +```rust +// In the embedded-services crate +pub trait MyServiceInterface { + fn foo(&mut self) -> Result<()>; + fn bar(&mut self) -> Result<()>; + fn baz(&mut self) -> Result<()>; +} + +// In the reference implementation crate +pub struct MyService { /* ... */ } + +impl embedded_services::MyServiceInterface for MyService { + fn foo(&mut self) -> Result<()> { /* ... */ } + fn bar(&mut self) -> Result<()> { /* ... */ } + fn baz(&mut self) -> Result<()> { /* ... */ } +} +``` \ No newline at end of file From fe8e4086975b32f49ce0aeca2133c39dbc07d93a Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 9 Mar 2026 14:46:05 -0700 Subject: [PATCH 06/17] api guidelines --- docs/api-guidelines.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/api-guidelines.md b/docs/api-guidelines.md index 5f04112c..dec41e5d 100644 --- a/docs/api-guidelines.md +++ b/docs/api-guidelines.md @@ -219,6 +219,8 @@ Notice that most of the complexity has been moved into internal implementation d In most cases, public APIs in this repo should be exposed in terms of traits rather than methods directly on the object, and objects that need references to other embedded-services objects should reference them by trait rather than by name. This does not apply to public methods used to construct or initialize a service, because those generally need to know something about the concrete implementation type to properly initialize it. +These traits should be defined in the `embedded-services` crate. + __Reason__: Improved testability and customizability. Testability - if all our types interact with each other via traits rather than direct dependencies on the type, it makes it much easier to mock out individual components. Customizability - if an OEM needs to insert a special behavior, they can substitute in a different implementation of that trait and continue using the rest of the embedded-services code without modification. From 9bd110d025bec6e7ab5029dd1a88afa5dad1fe74 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 9 Mar 2026 14:49:15 -0700 Subject: [PATCH 07/17] ci fixes --- espi-service/src/espi_service.rs | 4 +-- examples/rt633/Cargo.lock | 35 ---------------------- odp-service-common/src/runnable_service.rs | 21 +++++++------ 3 files changed, 12 insertions(+), 48 deletions(-) diff --git a/espi-service/src/espi_service.rs b/espi-service/src/espi_service.rs index 682b5826..f654b011 100644 --- a/espi-service/src/espi_service.rs +++ b/espi-service/src/espi_service.rs @@ -47,8 +47,8 @@ pub struct Runner<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandle inner: &'hw ServiceInner<'hw, RelayHandler>, } -impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> odp_service_common::runnable_service::ServiceRunner<'hw> - for Runner<'hw, RelayHandler> +impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> + odp_service_common::runnable_service::ServiceRunner<'hw> for Runner<'hw, RelayHandler> { /// Run the service event loop. async fn run(self) -> embedded_services::Never { diff --git a/examples/rt633/Cargo.lock b/examples/rt633/Cargo.lock index 4b0ce9fb..d68b91c8 100644 --- a/examples/rt633/Cargo.lock +++ b/examples/rt633/Cargo.lock @@ -289,15 +289,6 @@ dependencies = [ "syn", ] -[[package]] -name = "debug-service-messages" -version = "0.1.0" -dependencies = [ - "defmt 0.3.100", - "embedded-services", - "num_enum", -] - [[package]] name = "defmt" version = "0.3.100" @@ -716,11 +707,9 @@ dependencies = [ name = "espi-service" version = "0.1.0" dependencies = [ - "battery-service-messages", "bitfield 0.17.0", "cortex-m", "cortex-m-rt", - "debug-service-messages", "defmt 0.3.100", "embassy-futures", "embassy-imxrt", @@ -730,8 +719,6 @@ dependencies = [ "mctp-rs", "num_enum", "odp-service-common", - "thermal-service-messages", - "time-alarm-service-messages", ] [[package]] @@ -1284,16 +1271,6 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" -[[package]] -name = "thermal-service-messages" -version = "0.1.0" -dependencies = [ - "defmt 0.3.100", - "embedded-services", - "num_enum", - "uuid", -] - [[package]] name = "thiserror" version = "2.0.18" @@ -1314,18 +1291,6 @@ dependencies = [ "syn", ] -[[package]] -name = "time-alarm-service-messages" -version = "0.1.0" -dependencies = [ - "bitfield 0.17.0", - "defmt 0.3.100", - "embedded-mcu-hal", - "embedded-services", - "num_enum", - "zerocopy", -] - [[package]] name = "typenum" version = "1.19.0" diff --git a/odp-service-common/src/runnable_service.rs b/odp-service-common/src/runnable_service.rs index bfeeef87..cd99466d 100644 --- a/odp-service-common/src/runnable_service.rs +++ b/odp-service-common/src/runnable_service.rs @@ -1,15 +1,13 @@ //! This module contains helper traits and functions for services that run on the EC. -/// A trait for a service that can be run on the EC. -/// Implementations of Service should have an init() function to construct the service that -/// returns a Runner, which the user is expected to spawn a task for. +/// A trait for a service that requires the caller to launch a long-running task on its behalf to operate. pub trait Service<'hw>: Sized { - /// A type that can be used to run the service. This is returned by the init() function and the user is + /// A type that can be used to run the service. This is returned by the new() function and the user is /// expected to call its run() method in an embassy task (or similar parallel execution context on other /// async runtimes). type Runner: ServiceRunner<'hw>; - /// Any memory resources that your service needs. This is typically an opaque types that is only used by the service + /// Any memory resources that your service needs. This is typically an opaque type that is only used by the service /// and is not interacted with by users of the service. Must be default-constructible for spawn_service!() to work. type Resources: Default; @@ -38,23 +36,23 @@ pub trait ServiceRunner<'hw> { /// Initializes a service, creates an embassy task to run it, and spawns that task. /// /// This macro handles the boilerplate of: -/// 1. Creating a `static` [`OnceLock`](embassy_sync::once_lock::OnceLock) to hold the service -/// 2. Calling the service's `init()` method +/// 1. Creating a `static` [`StaticCell`](static_cell::StaticCell) to hold the service +/// 2. Calling the service's `new()` method /// 3. Defining an embassy_executor::task to run the service /// 4. Spawning the task on the provided executor /// -/// Returns a Result where Error is the error type of $service_ty::init(). +/// Returns a Result<&Service, Error> where Error is the error type of $service_ty::new(). /// /// Arguments /// /// - spawner: An embassy_executor::Spawner. /// - service_ty: The service type that implements Service that you want to create and run. -/// - init_arg: The init argument type to pass to `Service::init()` +/// - init_arg: The init argument type to pass to `Service::new()` /// /// Example: /// /// ```ignore -/// let time_service = embedded_services::spawn_service!( +/// let time_service = odp_service_common::runnable_service::spawn_service!( /// spawner, /// time_alarm_service::Service<'static>, /// time_alarm_service::ServiceInitParams { dt_clock, tz, ac_expiration, ac_policy, dc_expiration, dc_policy } @@ -64,7 +62,8 @@ pub trait ServiceRunner<'hw> { macro_rules! spawn_service { ($spawner:expr, $service_ty:ty, $init_arg:expr) => {{ use $crate::runnable_service::{Service, ServiceRunner}; - static SERVICE_RESOURCES: StaticCell<(<$service_ty as Service>::Resources)> = StaticCell::new(); + static SERVICE_RESOURCES: static_cell::StaticCell<(<$service_ty as Service>::Resources)> = + static_cell::StaticCell::new(); let service_resources = SERVICE_RESOURCES.init(<<$service_ty as Service>::Resources as Default>::default()); #[embassy_executor::task] From eb2587f4606b4d7d165fafc07f8288830751e72c Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 9 Mar 2026 14:57:43 -0700 Subject: [PATCH 08/17] doc --- Cargo.lock | 1 + odp-service-common/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index cdca2cc1..c875565c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1716,6 +1716,7 @@ name = "odp-service-common" version = "0.1.0" dependencies = [ "embedded-services", + "static_cell", ] [[package]] diff --git a/odp-service-common/Cargo.toml b/odp-service-common/Cargo.toml index 5eadba5a..ef6b5a46 100644 --- a/odp-service-common/Cargo.toml +++ b/odp-service-common/Cargo.toml @@ -7,6 +7,7 @@ repository.workspace = true [dependencies] embedded-services.workspace = true +static_cell.workspace = true [lints] workspace = true From a1ecc5d35b0072292e663198c796c01c27b01625 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 9 Mar 2026 15:07:27 -0700 Subject: [PATCH 09/17] example cargo.lock --- examples/rt633/Cargo.lock | 1 + examples/rt685s-evk/Cargo.lock | 1 + 2 files changed, 2 insertions(+) diff --git a/examples/rt633/Cargo.lock b/examples/rt633/Cargo.lock index d68b91c8..10c51c85 100644 --- a/examples/rt633/Cargo.lock +++ b/examples/rt633/Cargo.lock @@ -1001,6 +1001,7 @@ name = "odp-service-common" version = "0.1.0" dependencies = [ "embedded-services", + "static_cell", ] [[package]] diff --git a/examples/rt685s-evk/Cargo.lock b/examples/rt685s-evk/Cargo.lock index b69ff511..f227b319 100644 --- a/examples/rt685s-evk/Cargo.lock +++ b/examples/rt685s-evk/Cargo.lock @@ -1236,6 +1236,7 @@ name = "odp-service-common" version = "0.1.0" dependencies = [ "embedded-services", + "static_cell", ] [[package]] From 23cc09875fd7963080ec9101d17b0c983d1eeaa5 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 9 Mar 2026 15:11:19 -0700 Subject: [PATCH 10/17] sp --- docs/api-guidelines.md | 4 ++-- odp-service-common/src/runnable_service.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/api-guidelines.md b/docs/api-guidelines.md index dec41e5d..42f90852 100644 --- a/docs/api-guidelines.md +++ b/docs/api-guidelines.md @@ -91,10 +91,10 @@ impl<'hw> Foo<'hw> { ### Use runner objects for concurrency -Don't declare embassy tasks in your module - instead, have the constructor for your type return a `(Self, Runner)` tuple. The `Runner` object should have a single method `run(self) -> !` that the entity that instantiated your object must execute. You should have only one `Runner` object returned. Use the `odp-service-common::runnable_service::Service` trait to enforce this patttern. +Don't declare embassy tasks in your module - instead, have the constructor for your type return a `(Self, Runner)` tuple. The `Runner` object should have a single method `run(self) -> !` that the entity that instantiated your object must execute. You should have only one `Runner` object returned. Use the `odp-service-common::runnable_service::Service` trait to enforce this pattern. __Reason__: Declarations of embassy tasks are functionally static memory allocations. They can't be generic and you have to declare at declaration time a maximum number of instances that can be run concurrently. They also commit you to running on embassy, which is not necessarily desirable in test contexts. Pushing responsibility for the allocation out of your module allows your types to be generic. -However, it also means that your caller needs to be able to declare a task that can run your runner, and if you have multiple things that each need different pieces of state and need to run concurrently, setting up those tasks can make your API unwieldly and brittle. +However, it also means that your caller needs to be able to declare a task that can run your runner, and if you have multiple things that each need different pieces of state and need to run concurrently, setting up those tasks can make your API unwieldy and brittle. Returning a simple `Runner` object at the same time as your object makes it difficult to forget to execute the runner. Allowing only a single `Runner` with only one method that takes no external arguments makes it difficult to misuse the runner. diff --git a/odp-service-common/src/runnable_service.rs b/odp-service-common/src/runnable_service.rs index cd99466d..7f32e3cf 100644 --- a/odp-service-common/src/runnable_service.rs +++ b/odp-service-common/src/runnable_service.rs @@ -11,7 +11,7 @@ pub trait Service<'hw>: Sized { /// and is not interacted with by users of the service. Must be default-constructible for spawn_service!() to work. type Resources: Default; - /// The error type that your `init` function can return on failure. + /// The error type that your `new` function can return on failure. type ErrorType; /// Any initialization parameters that your service needs to run. From af8d6e43fcd50743ca273627f79c44462decca19 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 9 Mar 2026 15:19:56 -0700 Subject: [PATCH 11/17] doc --- odp-service-common/src/runnable_service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/odp-service-common/src/runnable_service.rs b/odp-service-common/src/runnable_service.rs index 7f32e3cf..d6510024 100644 --- a/odp-service-common/src/runnable_service.rs +++ b/odp-service-common/src/runnable_service.rs @@ -25,7 +25,7 @@ pub trait Service<'hw>: Sized { ) -> impl core::future::Future>; } -/// A trait for a run handle used to execute a service's event loop. This is returned by Service::init() +/// A trait for a run handle used to execute a service's event loop. This is returned by Service::new() /// and the user is expected to call its run() method in an embassy task (or similar parallel execution context /// on other async runtimes). pub trait ServiceRunner<'hw> { From 5fa35bec96c157bc7b6b1b8ddde244dcefc37956 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 9 Mar 2026 15:20:58 -0700 Subject: [PATCH 12/17] doc --- docs/api-guidelines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api-guidelines.md b/docs/api-guidelines.md index 42f90852..2ffe453a 100644 --- a/docs/api-guidelines.md +++ b/docs/api-guidelines.md @@ -33,7 +33,7 @@ trait Subscriber {} struct Notifier<'sub> { subscriber: &'sub Subscriber } // ^^^^^^ ^^^^^ -impl Notifier { +impl<'sub> Notifier<'sub> { fn new(subscriber: &'sub Subscriber) -> Self { // ^^^^^ Self { subscriber } From 642c9fff144a8d5e250e1f223462e70b596f71c0 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Mon, 9 Mar 2026 15:45:21 -0700 Subject: [PATCH 13/17] update example code to disambiguate "my" --- docs/api-guidelines.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/api-guidelines.md b/docs/api-guidelines.md index 2ffe453a..a9859466 100644 --- a/docs/api-guidelines.md +++ b/docs/api-guidelines.md @@ -228,8 +228,8 @@ Customizability - if an OEM needs to insert a special behavior, they can substit __Example__: Instead of ```rust -pub struct MyService { /* */ } -impl MyService { +pub struct ExampleService { /* */ } +impl ExampleService { fn foo(&mut self) -> Result<()> { /* ... */ } fn bar(&mut self) -> Result<()> { /* ... */ } fn baz(&mut self) -> Result<()> { /* ... */ } @@ -239,16 +239,16 @@ impl MyService { Consider: ```rust // In the embedded-services crate -pub trait MyServiceInterface { +pub trait ExampleService { fn foo(&mut self) -> Result<()>; fn bar(&mut self) -> Result<()>; fn baz(&mut self) -> Result<()>; } // In the reference implementation crate -pub struct MyService { /* ... */ } +pub struct OdpExampleService { /* ... */ } -impl embedded_services::MyServiceInterface for MyService { +impl embedded_services::ExampleService for OdpExampleService { fn foo(&mut self) -> Result<()> { /* ... */ } fn bar(&mut self) -> Result<()> { /* ... */ } fn baz(&mut self) -> Result<()> { /* ... */ } From eabbbe84bb470faf01a6d24b795a923a3751be8b Mon Sep 17 00:00:00 2001 From: Billy Price Date: Tue, 10 Mar 2026 10:19:31 -0700 Subject: [PATCH 14/17] update docs --- docs/api-guidelines.md | 107 ++++++++++++++++++++++++++--------------- 1 file changed, 67 insertions(+), 40 deletions(-) diff --git a/docs/api-guidelines.md b/docs/api-guidelines.md index a9859466..ef466d96 100644 --- a/docs/api-guidelines.md +++ b/docs/api-guidelines.md @@ -16,11 +16,11 @@ __Example__: Instead of this: ```rust trait Subscriber {} -struct Notifier { subscriber: &'static Subscriber } +struct Notifier { subscriber: &'static dyn Subscriber } // ^^^^^^^^ impl Notifier { - fn new(subscriber: &'static Subscriber) -> Self { + fn new(subscriber: &'static dyn Subscriber) -> Self { // ^^^^^^^^ Self { subscriber } } @@ -30,46 +30,67 @@ impl Notifier { Consider something like this: ```rust trait Subscriber {} -struct Notifier<'sub> { subscriber: &'sub Subscriber } +struct Notifier<'sub> { subscriber: &'sub dyn Subscriber } // ^^^^^^ ^^^^^ impl<'sub> Notifier<'sub> { - fn new(subscriber: &'sub Subscriber) -> Self { + fn new(subscriber: &'sub dyn Subscriber) -> Self { // ^^^^^ Self { subscriber } } } ``` +In cases like this, if you know that there will only be one concrete type for your reference, consider being generic over the type rather than taking it as `dyn`. This is particularly common for HAL trait implementations. This allows the compiler to inline and simplify code, which can result in performance and code size improvements in some circumstances. + +Alternatively, if you can take an owned `Subscriber` rather than a reference, something like this is probably better: +```rust +trait Subscriber {} +struct Notifier { + sub: S, +} + +impl Notifier { + const fn new(sub: S) -> Self { + Self { sub } + } +} +``` ### External memory allocation / no static memory allocation Memory allocation should always be the role of the caller of the API. If you need memory, have your caller pass it into your constructor. Do not have things like `static INSTANCE: OnceLock` in your service module. +If you don't need dynamic dispatch over user-provided types, additionally consider being generic over those user-provided types rather than taking `dyn` arguments - this is only possible if you have external memory allocation. + Note that while this applies to code in this repo, it does not necessarily apply to other ODP repos (e.g. HAL crates that know exactly how many instances of peripheral X are available on the platform). __Reason__: Most code in this repos is expected to run primarily in environments that don't have a heap. In heapless environments, your options are either to have your caller provide you memory or to allocate it as a static variable in your module. Allocating it as a static variable in your module has negative impacts on flexibility, testability, performance, and code size. Flexibility - Memory allocation in your module rather than by your caller means that the size of your object must be known when the module is compiled rather than when you're instantiated. This prevents you from storing any owned caller-provided types in your object (since you can't know those types when your module is compiled). -Testability - if you have a private singleton instance, tests can't arbitrary destroy and recreate that state. This makes it difficult to test multiple startup paths. -Performance - if you can't be generic over a type, the only way you can interact with user-provided types is by dyn references to trait impls. This means you have to pay for dynamic dispatch and the compiler can't optimize or inline across that dyn boundary. -Code size - The compiler has to generate a bunch of code to handle dynamic dispatch, even if there's only ever a single concrete type that implements the trait you want to be generic over. +Testability - if you have a private singleton instance, tests can't arbitrarily destroy and recreate that state. This makes it difficult to test multiple startup paths. +Performance - if you can't be generic over a type, the only way you can interact with user-provided types is by dyn references to trait impls. External memory allocation allows you to be generic over a type, which means you don't have to pay for dynamic dispatch and the compiler can potentially inline code / optimize the interaction between your code and the user-provided type's code. +Code size - The compiler has to generate a bunch of code to handle dynamic dispatch, even if there's only ever a single concrete type that implements the trait you want to be generic over, which is common with HAL traits. __Example__: -Note that in the below example, the `OnceLock` / external `Resources` is only necessary if you need to hand out references to the contents of the `OnceLock` / `FooInner`. That's elided in the example and assumed to be implemented in the `/* ... */` blocks for simplicity. +Note that in the below example, the `OnceLock` / external `Resources` is only necessary if you need to hand out references to the contents of the `OnceLock` / `FooInner`. That's elided in the example and assumed to be implemented in the `/* .. */` blocks for simplicity. ```rust -pub struct Foo { /* ... */ } +pub struct Foo { /* .. */ } static INSTANCE: OnceLock = OnceLock::new(); impl Foo { - async fn init(/* ... */) -> &'static Foo { - INSTANCE.get_or_init(|| Foo{ /* ... */ }).await + async fn init(/* .. */) -> &'static Foo { + let instance = INSTANCE.get_or_init(|| Foo{ /* .. */ }).await; + + // Create another reference to some state in 'inner' - perhaps by passing it to something in /* .. */ + + instance } } ``` Consider something like this: ```rust -struct FooInner<'hw> { /* ... */ } +struct FooInner<'hw> { /* .. */ } #[derive(Default)] pub struct Resources<'hw> { @@ -81,8 +102,14 @@ pub struct Foo<'hw> { } impl<'hw> Foo<'hw> { - fn new(resources: &'hw mut Resources, /* ... */) -> Self { - let inner = resources.insert(FooInner::new(/* ... */)); + fn new(resources: &'hw mut Resources, /* .. */) -> Self { + let inner = resources.insert(FooInner::new(/* .. */)); + + // Create another reference to some state in `inner` here that outlasts this function - perhaps by returning + // a `Runner` that contains a reference to `inner` or passing a reference to `inner` to one of the elided + // arguments in /* .. */. See the 'Use runner objects for concurrency' section for a concrete example of this. + // If you don't have a requirement to do this, you don't need the indirection / external `Resources` object at all. + Self{ inner } } } @@ -102,10 +129,10 @@ __Example__: Instead of this: ```rust ///// Your type's definition ///// -struct MyRunnableTypeInner { /* ... */ } +struct MyRunnableTypeInner { /* .. */ } impl<'hw> MyRunnableTypeInner<'hw> { - /* ... */ + /* .. */ } #[derive(Default)] @@ -118,23 +145,23 @@ pub struct MyRunnableType<'hw> { } impl<'hw> MyRunnableType<'hw> { - fn new(resources: &mut Resources, /* ... */ ) -> Self { - let inner = resources.insert(RunnableTypeInner::new(/* ... */)) - /* ... */ + fn new(resources: &mut Resources, /* .. */ ) -> Self { + let inner = resources.insert(RunnableTypeInner::new(/* .. */)) + /* .. */ Self { inner } } } mod tasks { - pub async fn run_task_1<'hw>(runnable: &'hw MyRunnableType, foo: Foo) -> ! { /* ... */ } - pub async fn run_task_2<'hw>(runnable: &'hw MyRunnableType, bar: Bar) -> ! { /* ... */ } - pub async fn run_task_3<'hw>(runnable: &'hw MyRunnableType, baz: Baz) -> ! { /* ... */ } + pub async fn run_task_1<'hw>(runnable: &'hw MyRunnableType, foo: Foo) -> ! { /* .. */ } + pub async fn run_task_2<'hw>(runnable: &'hw MyRunnableType, bar: Bar) -> ! { /* .. */ } + pub async fn run_task_3<'hw>(runnable: &'hw MyRunnableType, baz: Baz) -> ! { /* .. */ } } ///// End-user code ///// fn main() { - let instance = MyRunnableType::new(/* ... */); + let instance = MyRunnableType::new(/* .. */); #[embassy_task] fn runner_1(runnable: &'static MyRunnableType, foo: Foo) -> ! { my_runnable_type::tasks::run_task_1(runnable, foo).await @@ -148,9 +175,9 @@ fn main() { my_runnable_type::tasks::run_task_3(runnable, baz).await } - spawner.must_spawn(runner_1(&instance, Foo::new( /* ... */ ))); - spawner.must_spawn(runner_1(&instance, Bar::new( /* ... */ ))); - spawner.must_spawn(runner_1(&instance, Baz::new( /* ... */ ))); + spawner.must_spawn(runner_1(&instance, Foo::new( /* .. */ ))); + spawner.must_spawn(runner_1(&instance, Bar::new( /* .. */ ))); + spawner.must_spawn(runner_1(&instance, Baz::new( /* .. */ ))); } ``` @@ -158,12 +185,12 @@ fn main() { Consider something like this: ```rust ///// Your type's definition ///// -struct MyRunnableTypeInner { /* ... */ } +struct MyRunnableTypeInner { /* .. */ } impl<'hw> MyRunnableTypeInner<'hw> { - async fn task_1(&self, foo: Foo) -> ! { /* ... */ } - async fn task_2(&self, bar: Bar) -> ! { /* ... */ } - async fn task_3(&self, baz: Baz) -> ! { /* ... */ } + async fn task_1(&self, foo: Foo) -> ! { /* .. */ } + async fn task_2(&self, bar: Bar) -> ! { /* .. */ } + async fn task_3(&self, baz: Baz) -> ! { /* .. */ } } #[derive(Default)] @@ -195,8 +222,8 @@ impl<'hw> Runner<'hw> { } impl<'hw> MyRunnableType<'hw> { - fn new(resources: &mut Resources, foo: Foo, bar: Bar, baz: Baz /* ... */ ) -> (Self, Runner) { - let inner = resources.insert(RunnableTypeInner::new( /* ... */ )); + fn new(resources: &mut Resources, foo: Foo, bar: Bar, baz: Baz /* .. */ ) -> (Self, Runner) { + let inner = resources.insert(RunnableTypeInner::new( /* .. */ )); (Self { inner }, Runner { inner, foo, bar, baz }) } } @@ -204,7 +231,7 @@ impl<'hw> MyRunnableType<'hw> { ///// End-user code ///// fn main() { - let (instance, runner) = MyRunnableType::new(/* ... */); + let (instance, runner) = MyRunnableType::new(/* .. */); #[embassy_task] fn runner_fn(runner: Runner) { runner.run().await @@ -230,9 +257,9 @@ Instead of ```rust pub struct ExampleService { /* */ } impl ExampleService { - fn foo(&mut self) -> Result<()> { /* ... */ } - fn bar(&mut self) -> Result<()> { /* ... */ } - fn baz(&mut self) -> Result<()> { /* ... */ } + fn foo(&mut self) -> Result<()> { /* .. */ } + fn bar(&mut self) -> Result<()> { /* .. */ } + fn baz(&mut self) -> Result<()> { /* .. */ } } ``` @@ -246,11 +273,11 @@ pub trait ExampleService { } // In the reference implementation crate -pub struct OdpExampleService { /* ... */ } +pub struct OdpExampleService { /* .. */ } impl embedded_services::ExampleService for OdpExampleService { - fn foo(&mut self) -> Result<()> { /* ... */ } - fn bar(&mut self) -> Result<()> { /* ... */ } - fn baz(&mut self) -> Result<()> { /* ... */ } + fn foo(&mut self) -> Result<()> { /* .. */ } + fn bar(&mut self) -> Result<()> { /* .. */ } + fn baz(&mut self) -> Result<()> { /* .. */ } } ``` \ No newline at end of file From b16157cf586f33055d83103df36bb85e9d8d5c34 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Tue, 10 Mar 2026 11:02:44 -0700 Subject: [PATCH 15/17] comment --- espi-service/src/espi_service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/espi-service/src/espi_service.rs b/espi-service/src/espi_service.rs index f654b011..fdbdc1de 100644 --- a/espi-service/src/espi_service.rs +++ b/espi-service/src/espi_service.rs @@ -42,7 +42,7 @@ impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> Default fo } } -/// Service runner for the eSPI service. Users must call the run() method on the service to start processing events. +/// Service runner for the eSPI service. Users must call the run() method on the runner for the service to start processing events. pub struct Runner<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> { inner: &'hw ServiceInner<'hw, RelayHandler>, } From 7657af80112c9bb84b9cdbe1ad5c117ebad793bb Mon Sep 17 00:00:00 2001 From: Billy Price Date: Tue, 10 Mar 2026 12:48:53 -0700 Subject: [PATCH 16/17] update docs on interface patterns --- docs/api-guidelines.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/api-guidelines.md b/docs/api-guidelines.md index ef466d96..90ef2eb3 100644 --- a/docs/api-guidelines.md +++ b/docs/api-guidelines.md @@ -153,9 +153,9 @@ impl<'hw> MyRunnableType<'hw> { } mod tasks { - pub async fn run_task_1<'hw>(runnable: &'hw MyRunnableType, foo: Foo) -> ! { /* .. */ } - pub async fn run_task_2<'hw>(runnable: &'hw MyRunnableType, bar: Bar) -> ! { /* .. */ } - pub async fn run_task_3<'hw>(runnable: &'hw MyRunnableType, baz: Baz) -> ! { /* .. */ } + pub async fn run_task_1(runnable: &MyRunnableType, foo: Foo) -> ! { /* .. */ } + pub async fn run_task_2(runnable: &MyRunnableType, bar: Bar) -> ! { /* .. */ } + pub async fn run_task_3(runnable: &MyRunnableType, baz: Baz) -> ! { /* .. */ } } ///// End-user code ///// @@ -244,9 +244,9 @@ Notice that most of the complexity has been moved into internal implementation d ### Use traits for public methods expected to be used at run time whenever possible -In most cases, public APIs in this repo should be exposed in terms of traits rather than methods directly on the object, and objects that need references to other embedded-services objects should reference them by trait rather than by name. This does not apply to public methods used to construct or initialize a service, because those generally need to know something about the concrete implementation type to properly initialize it. +In most cases, public APIs in this repo should be exposed in terms of traits rather than methods directly on the object, and objects that need to interact with other embedded-services objects should refer to them by trait rather than by name. This does not apply to public methods used to construct or initialize a service, because those generally need to know something about the concrete implementation type to properly initialize it. -These traits should be defined in the `embedded-services` crate. +These traits should be defined in standalone 'interface' crates (i.e. `battery-service-interface`) alongside any support types needed for the interface (e.g. an Error enum) __Reason__: Improved testability and customizability. Testability - if all our types interact with each other via traits rather than direct dependencies on the type, it makes it much easier to mock out individual components. @@ -265,7 +265,7 @@ impl ExampleService { Consider: ```rust -// In the embedded-services crate +// In a standalone interface crate pub trait ExampleService { fn foo(&mut self) -> Result<()>; fn bar(&mut self) -> Result<()>; From 5d3d0c9965739e2eaa0823dd7e1c44b783633cae Mon Sep 17 00:00:00 2001 From: Billy Price Date: Tue, 10 Mar 2026 13:09:57 -0700 Subject: [PATCH 17/17] docs --- docs/api-guidelines.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/api-guidelines.md b/docs/api-guidelines.md index 90ef2eb3..b6e7e75a 100644 --- a/docs/api-guidelines.md +++ b/docs/api-guidelines.md @@ -64,7 +64,7 @@ If you don't need dynamic dispatch over user-provided types, additionally consid Note that while this applies to code in this repo, it does not necessarily apply to other ODP repos (e.g. HAL crates that know exactly how many instances of peripheral X are available on the platform). -__Reason__: Most code in this repos is expected to run primarily in environments that don't have a heap. In heapless environments, your options are either to have your caller provide you memory or to allocate it as a static variable in your module. Allocating it as a static variable in your module has negative impacts on flexibility, testability, performance, and code size. +__Reason__: Most code in this repo is expected to run primarily in environments that don't have a heap. In heapless environments, your options are either to have your caller provide you memory or to allocate it as a static variable in your module. Allocating it as a static variable in your module has negative impacts on flexibility, testability, performance, and code size. Flexibility - Memory allocation in your module rather than by your caller means that the size of your object must be known when the module is compiled rather than when you're instantiated. This prevents you from storing any owned caller-provided types in your object (since you can't know those types when your module is compiled). Testability - if you have a private singleton instance, tests can't arbitrarily destroy and recreate that state. This makes it difficult to test multiple startup paths. Performance - if you can't be generic over a type, the only way you can interact with user-provided types is by dyn references to trait impls. External memory allocation allows you to be generic over a type, which means you don't have to pay for dynamic dispatch and the compiler can potentially inline code / optimize the interaction between your code and the user-provided type's code. @@ -145,7 +145,7 @@ pub struct MyRunnableType<'hw> { } impl<'hw> MyRunnableType<'hw> { - fn new(resources: &mut Resources, /* .. */ ) -> Self { + pub fn new(resources: &mut Resources, /* .. */ ) -> Self { let inner = resources.insert(RunnableTypeInner::new(/* .. */)) /* .. */ Self { inner } @@ -176,8 +176,8 @@ fn main() { } spawner.must_spawn(runner_1(&instance, Foo::new( /* .. */ ))); - spawner.must_spawn(runner_1(&instance, Bar::new( /* .. */ ))); - spawner.must_spawn(runner_1(&instance, Baz::new( /* .. */ ))); + spawner.must_spawn(runner_2(&instance, Bar::new( /* .. */ ))); + spawner.must_spawn(runner_3(&instance, Baz::new( /* .. */ ))); } ``` @@ -210,7 +210,7 @@ pub struct Runner<'hw> { } impl<'hw> Runner<'hw> { - fn run(self) -> ! { + pub async fn run(self) -> ! { loop { embassy_sync::join::join3( self.inner.task_1(self.foo), @@ -222,7 +222,7 @@ impl<'hw> Runner<'hw> { } impl<'hw> MyRunnableType<'hw> { - fn new(resources: &mut Resources, foo: Foo, bar: Bar, baz: Baz /* .. */ ) -> (Self, Runner) { + pub fn new(resources: &mut Resources, foo: Foo, bar: Bar, baz: Baz /* .. */ ) -> (Self, Runner) { let inner = resources.insert(RunnableTypeInner::new( /* .. */ )); (Self { inner }, Runner { inner, foo, bar, baz }) }