Rework spawn_service macro to relax constraints on constructors

.github/copilot-instructions.md

@@ -65,9 +65,8 @@ cd examples/std && cargo clippy --locked
 Services implement the `odp_service_common::runnable_service::Service<'hw>` trait, which enforces a consistent structure:
 
 1. **`Resources`** — caller-allocated state (stored in a `StaticCell`), not an internal `OnceLock` singleton
-2. **`new(resources, params) -> (Self, Runner)`** — constructor returns a control handle and a `Runner`
-3. **`Runner`** — implements `ServiceRunner` with a single `run(self) -> !` method that drives the service's async event loop
-4. **`spawn_service!`** macro — handles boilerplate: allocates `Resources` in a `StaticCell`, calls `new()`, spawns the `Runner` on an Embassy executor
+2. **`Runner`** — implements `ServiceRunner` with a single `run(self) -> !` method that drives the service's async event loop
+3. **`spawn_service!`** macro — handles boilerplate: allocates `Resources` in a `StaticCell`, invokes a caller-provided initialization closure, spawns the `Runner` on an Embassy executor
 
 ```rust
 // Typical service using the Service trait
@@ -82,13 +81,13 @@ pub struct Runner<'hw> { /* holds refs into Resources */ }
 impl<'hw> Service<'hw> for MyService<'hw> {
     type Resources = Resources<'hw>;
     type Runner = Runner<'hw>;
-    type InitParams = MyInitParams<'hw>;
-    type ErrorType = MyError;
+}
 
-    async fn new(
-        resources: &'hw mut Self::Resources,
-        params: Self::InitParams,
-    ) -> Result<(Self, Self::Runner), Self::ErrorType> {
+impl<'hw> MyService<'hw> {
+    pub async fn new(
+        resources: &'hw mut Resources<'hw>,
+        params: MyInitParams<'hw>,
+    ) -> Result<(Self, Runner<'hw>), MyError> {
         // ...
     }
 }
@@ -115,7 +114,11 @@ Services use a variety of async IPC mechanisms from `embassy-sync` and `embedded
 At the top level, an EC is composed by spawning service tasks on an Embassy executor, using the `spawn_service!` macro:
 
 ```rust
-let my_service = spawn_service!(spawner, MyService, my_init_params)?;
+let my_service = spawn_service!(
+    spawner,
+    MyService,
+    |resources| MyService::new(resources, my_other_params),
+)?;
 ```
 
 ### Core Utilities (embedded-service crate)

battery-service/src/lib.rs

@@ -250,18 +250,18 @@ pub enum InitError {
     CommsRegistrationFailed,
 }
 
-impl<'hw, const N: usize> odp_service_common::runnable_service::Service<'hw> for Service<'hw, N>
-where
-    'hw: 'static, // TODO relax this 'static requirement when we drop usages of IntrusiveList (including comms)
-{
+impl<'hw, const N: usize> odp_service_common::runnable_service::Service<'hw> for Service<'hw, N> {
     type Runner = Runner<'hw, N>;
-    type ErrorType = InitError;
-    type InitParams = InitParams<'hw, N>;
     type Resources = Resources<N>;
+}
 
-    async fn new(
+impl<'hw, const N: usize> Service<'hw, N>
+where
+    'hw: 'static, // TODO relax this 'static requirement when we drop usages of IntrusiveList (including comms)
+{
+    pub async fn new(
         service_storage: &'hw mut Resources<N>,
-        init_params: Self::InitParams,
+        init_params: InitParams<'hw, N>,
     ) -> Result<(Self, Runner<'hw, N>), InitError> {
         let service = service_storage.inner.insert(ServiceInner::new(init_params.config));
 

espi-service/src/espi_service.rs

@@ -65,13 +65,13 @@ impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> odp_servic
 {
     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,
+impl<'hw, RelayHandler: embedded_services::relay::mctp::RelayHandler> Service<'hw, RelayHandler> {
+    pub async fn new(
+        resources: &'hw mut Resources<'hw, RelayHandler>,
         params: InitParams<'hw, RelayHandler>,
-    ) -> Result<(Self, Self::Runner), core::convert::Infallible> {
+    ) -> Result<(Self, Runner<'hw, RelayHandler>), core::convert::Infallible> {
         let inner = resources.inner.insert(ServiceInner::new(params).await);
         Ok((Self { _inner: inner }, Runner { inner }))
     }

examples/pico-de-gallo/src/bin/battery.rs

@@ -19,7 +19,7 @@
 use battery_service as bs;
 use bq40z50_rx::{BQ40Z50Error, Bq40z50R5};
 use embedded_batteries_async::smart_battery::{BatteryModeFields, SmartBattery};
-use odp_service_common::runnable_service::{Service, ServiceRunner};
+use odp_service_common::runnable_service::ServiceRunner;
 use static_cell::StaticCell;
 
 /// Platform specific battery errors.

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

@@ -31,18 +31,17 @@ async fn main(spawner: embassy_executor::Spawner) {
     embedded_services::init().await;
     info!("services initialized");
 
-    let time_service = odp_service_common::spawn_service!(
-        spawner,
-        TimeAlarmServiceType,
-        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
-        }
-    )
+    let time_service = odp_service_common::spawn_service!(spawner, TimeAlarmServiceType, |resources| {
+        time_alarm_service::Service::new(
+            resources,
+            dt_clock,
+            tz,
+            ac_expiration,
+            ac_policy,
+            dc_expiration,
+            dc_policy,
+        )
+    })
     .expect("Failed to spawn time alarm service");
 
     use embedded_services::relay::mctp::impl_odp_mctp_relay_handler;

examples/std/src/bin/battery.rs

@@ -17,14 +17,15 @@ async fn embassy_main(spawner: Spawner) {
     static BATTERY_DEVICE: StaticCell<bs::device::Device> = StaticCell::new();
     let device = BATTERY_DEVICE.init(bs::device::Device::new(Default::default()));
 
-    let battery_service = spawn_service!(
-        spawner,
-        battery_service::Service<'static, 1>,
-        battery_service::InitParams {
-            config: Default::default(),
-            devices: [device],
-        }
-    )
+    let battery_service = spawn_service!(spawner, battery_service::Service<'static, 1>, |resources| {
+        battery_service::Service::new(
+            resources,
+            battery_service::InitParams {
+                config: Default::default(),
+                devices: [device],
+            },
+        )
+    })
     .expect("Failed to initialize battery service");
 
     static BATTERY_WRAPPER: StaticCell<bs::mock::MockBattery> = StaticCell::new();

examples/std/src/bin/thermal.rs

@@ -36,29 +36,28 @@ async fn run(spawner: Spawner) {
     let event_senders = SENSOR_SENDERS.init([sensor_event_channel.sender()]);
 
     // Spawn the sensor service which will begin running and generating events
-    let sensor_service = odp_service_common::spawn_service!(
-        spawner,
-        MockSensorService,
-        ts::sensor::InitParams {
-            driver: ts::mock::sensor::MockSensor::new(),
-            config: ts::mock::sensor::MockSensor::config(),
-            event_senders,
-        }
-    )
-    .expect("Failed to spawn sensor service");
+    let sensor_service =
+        odp_service_common::spawn_service!(spawner, MockSensorService, |resources| ts::sensor::Service::new(
+            resources,
+            ts::sensor::InitParams {
+                driver: ts::mock::sensor::MockSensor::new(),
+                config: ts::mock::sensor::MockSensor::config(),
+                event_senders,
+            },
+        ))
+        .expect("Failed to spawn sensor service");
 
     // Spawn the fan service which uses the above sensor service for automatic speed control
     // In this example, we use an empty event sender list since the fan won't generate any events
-    let fan_service = odp_service_common::spawn_service!(
-        spawner,
-        MockFanService,
+    let fan_service = odp_service_common::spawn_service!(spawner, MockFanService, |resources| ts::fan::Service::new(
+        resources,
         ts::fan::InitParams {
             driver: ts::mock::fan::MockFan::new(),
             config: ts::mock::fan::MockFan::config(),
             sensor_service,
             event_senders: &mut [],
-        }
-    )
+        },
+    ))
     .expect("Failed to spawn fan service");
 
     // The thermal service accepts slices of associated sensors and fans,

odp-service-common/src/runnable_service.rs

@@ -2,76 +2,80 @@
 
 /// 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 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).
+    /// A type that can be used to run the service. This is returned by the service's constructor 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 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;
-
-    /// The error type that your `new` 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<Output = Result<(Self, Self::Runner), Self::ErrorType>>;
 }
 
-/// 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).
+/// A trait for a run handle used to execute a service's event loop.  This is returned by a service's
+/// constructor 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<Output = embedded_services::Never> + 'hw;
 }
 
+#[allow(clippy::doc_overindented_list_items)]
 /// Initializes a service, creates an embassy task to run it, and spawns that task.
 ///
 /// This macro handles the boilerplate of:
-/// 1. Creating a `static` [`StaticCell`](static_cell::StaticCell) to hold the service
-/// 2. Calling the service's `new()` method
+/// 1. Creating a `static` [`StaticCell`](static_cell::StaticCell) to hold the service's resources
+/// 2. Invoking the caller-provided initialization closure to construct the service
 /// 3. Defining an embassy_executor::task to run the service
 /// 4. Spawning the task on the provided executor
 ///
-/// Returns a Result<&Service, Error> where Error is the error type of $service_ty::new().
+/// Returns a `Result<Service, Error>` where `Error` is the error type produced by the initialization closure.
 ///
 /// 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::new()`
+/// - init_fn:    A function that takes a `&'static mut Resources` and returns an async future that
+///               returns a `Result<(Service, Runner), Error>`
+///               The function is typically a closure that's just a thin wrapper that calls the service's
+///               actual constructor with the provided resources.
 ///
 /// Example:
 ///
 /// ```ignore
 /// 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 }
+///     |resources| time_alarm_service::Service::new(
+///         resources,
+///         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) => {{
+    ($spawner:expr, $service_ty:ty, $init_fn:expr) => {{
         use $crate::runnable_service::{Service, ServiceRunner};
-        static SERVICE_RESOURCES: static_cell::StaticCell<(<$service_ty as Service>::Resources)> =
+        static SERVICE_RESOURCES: static_cell::StaticCell<<$service_ty as Service<'static>>::Resources> =
             static_cell::StaticCell::new();
-        let service_resources = SERVICE_RESOURCES.init(<<$service_ty as Service>::Resources as Default>::default());
+        let service_resources =
+            SERVICE_RESOURCES.init(<<$service_ty as Service<'static>>::Resources as Default>::default());
 
         #[embassy_executor::task]
         async fn service_task_fn(runner: <$service_ty as $crate::runnable_service::Service<'static>>::Runner) {
             runner.run().await;
         }
 
-        <$service_ty>::new(service_resources, $init_arg)
+        // Coerce init_fn to an `FnOnce` so it can capture values from the surrounding scope
+        fn call_once<F, Fut, T, E>(resources: &'static mut <$service_ty as Service<'static>>::Resources, f: F) -> Fut
+        where
+            F: FnOnce(&'static mut <$service_ty as Service<'static>>::Resources) -> Fut,
+            Fut: core::future::Future<Output = Result<T, E>>,
+        {
+            f(resources)
+        }
+
+        call_once(service_resources, $init_fn)
             .await
             .map(|(control_handle, runner)| {
                 $spawner.spawn(service_task_fn(runner).expect("Failed to spawn service task"));

thermal-service/src/fan.rs

@@ -404,13 +404,21 @@ impl<
 {
     type Runner = Runner<'hw, T, S, E, SAMPLE_BUF_LEN>;
     type Resources = Resources<T, SAMPLE_BUF_LEN>;
-    type ErrorType = fan::Error;
-    type InitParams = InitParams<'hw, T, S, E>;
+}
 
-    async fn new(
-        service_storage: &'hw mut Self::Resources,
-        init_params: Self::InitParams,
-    ) -> Result<(Self, Self::Runner), Self::ErrorType> {
+impl<
+    'hw,
+    T: fan::Driver,
+    S: sensor::SensorService + 'hw,
+    E: NonBlockingSender<fan::Event> + 'hw,
+    const SAMPLE_BUF_LEN: usize,
+> Service<'hw, T, S, E, SAMPLE_BUF_LEN>
+{
+    /// Initializes an instance of the fan service.
+    pub async fn new(
+        service_storage: &'hw mut Resources<T, SAMPLE_BUF_LEN>,
+        init_params: InitParams<'hw, T, S, E>,
+    ) -> Result<(Self, Runner<'hw, T, S, E, SAMPLE_BUF_LEN>), fan::Error> {
         let service = service_storage
             .inner
             .insert(ServiceInner::new(init_params.driver, init_params.config));

thermal-service/src/sensor.rs

@@ -310,13 +310,15 @@ impl<'hw, T: sensor::Driver, E: NonBlockingSender<sensor::Event> + 'hw, const SA
 {
     type Runner = Runner<'hw, T, E, SAMPLE_BUF_LEN>;
     type Resources = Resources<T, SAMPLE_BUF_LEN>;
-    type ErrorType = sensor::Error;
-    type InitParams = InitParams<'hw, T, E>;
+}
 
-    async fn new(
-        service_storage: &'hw mut Self::Resources,
-        init_params: Self::InitParams,
-    ) -> Result<(Self, Self::Runner), Self::ErrorType> {
+impl<'hw, T: sensor::Driver, E: NonBlockingSender<sensor::Event> + 'hw, const SAMPLE_BUF_LEN: usize>
+    Service<'hw, T, E, SAMPLE_BUF_LEN>
+{
+    pub async fn new(
+        service_storage: &'hw mut Resources<T, SAMPLE_BUF_LEN>,
+        init_params: InitParams<'hw, T, E>,
+    ) -> Result<(Self, Runner<'hw, T, E, SAMPLE_BUF_LEN>), sensor::Error> {
         let service = service_storage
             .inner
             .insert(ServiceInner::new(init_params.driver, init_params.config));