Skip to content

[RFC] Uniform service spawning with external 'resources'#740

Open
williampMSFT wants to merge 1 commit intoOpenDevicePartnership:v0.2.0from
williampMSFT:user/williamp/es-uniform-spawn-with-innersvc
Open

[RFC] Uniform service spawning with external 'resources'#740
williampMSFT wants to merge 1 commit intoOpenDevicePartnership:v0.2.0from
williampMSFT:user/williamp/es-uniform-spawn-with-innersvc

Conversation

@williampMSFT
Copy link
Contributor

This is a proof-of-concept attempt to create a common pattern for initializing and starting the services in embedded-services. It demonstrates this pattern on the time-alarm and espi service.

Currently, starting the various services in embedded-services is pretty complicated. Each service has a different set of 'worker tasks' that users have to declare and start, each of which has varying numbers of parameters of varying types. It's very easy to miss one, and whenever a change comes in that changes what state a given task needs to function, that's a breaking change that, in the best case, causes a build failure, and in the worst case causes the service to silently misbehave.

This attempts to solve this disparity with a few tricks:

  1. Rather than having N worker tasks, we can select!() over a list of worker tasks. This works because worker 'green threads' were already intended to never return, so the select never returns either. This lets us execute them all against a single embassy task.
  2. Rather than having each of those worker tasks require a different set of parameters, the onus for figuring out which parameters need to go to what is placed on the service author. At service construction time, the service author must return a (Service, ServiceRunner) tuple. The ServiceRunner implementation which has a single method that takes no arguments and never returns. This has a few benefits:
  • It makes changes to the number and shape of 'green thread' functions transparent to the end user.
  • It makes it impossible for users to only launch some of the tasks
  • It makes it impossible for users to launch tasks in an order that might cause problems
  • It makes it difficult (but not impossible) to forget to launch the task at all - they get a ServiceRunner object back when the instantiate the service, and if they don't use it the compiler will complain about unused variables. They can bypass the warning with a leading _, but that's an explicit action the programmer has to take. I haven't found a way to outright prevent this class of error, unfortunately.
  1. Require a &'hw mut Resources as an argument at service creation time. This Resources object is the 'real' service, and by taking it as a mutable borrow rather than a reference-to-oncelock or similar, we avoid prescribing any particular way of storing the memory required to run the service.
  2. Create a spawn_service!() macro that does all the boilerplate of declaring storage/tasks and spawning the tasks on Embassy for a given service. In product use cases, this makes it outright impossible to forget to start the runner, and reduces the ~10 lines of somewhat verbose boilerplate required for each service down to zero.

This approach aligns with a what a few other embassy services are doing - see embassy_net::new for a good example.

Some notes on testing -
The spawn_service!() macro isn't useful in test scenarios where you need to run on tokio, but in those situations you can manually call new() and run() with stack variables thanks to (3). In order to have the long-running task future terminate at the end of the test, you can simply select() over runner.run() and some test code; when the test code returns, the worker 'green thread' stops and the test exits. This technique is demonstrated in tad_test.rs.

Copilot AI review requested due to automatic review settings February 28, 2026 00:23
@williampMSFT williampMSFT requested review from a team as code owners February 28, 2026 00:23
@williampMSFT
Copy link
Contributor Author

williampMSFT commented Feb 28, 2026

A couple slightly different approaches to this problem are here: #727 #726

I think this is probably the best of them, though.

@williampMSFT williampMSFT requested a review from asasine February 28, 2026 00:28
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Introduces an RFC service initialization/spawning pattern in embedded-services based on explicit Resources storage and a Runner handle, and applies it to the time-alarm and eSPI services (plus updating an example and tests) to reduce per-service task boilerplate and make service startup uniform.

Changes:

  • Add embedded_services::service::{Service, ServiceResources, ServiceRunner} traits and a spawn_service!() macro to standardize service construction and task spawning.
  • Refactor time-alarm-service and espi-service to use (Service, Runner) returned from Service::new(...) with external Resources, removing the old task modules.
  • Update the RT685 example and time-alarm tokio tests to use the new APIs.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
embedded-service/src/service.rs Adds the new service traits and spawn_service!() macro for uniform initialization/spawning.
embedded-service/src/lib.rs Exposes the new service module publicly.
time-alarm-service/src/lib.rs Refactors time-alarm into Resources + Runner + control-handle Service implementing the new trait.
time-alarm-service/src/task.rs Removes the old dedicated task wrapper API.
time-alarm-service/tests/tad_test.rs Updates tokio tests to construct resources and drive the new Runner.
examples/rt685s-evk/src/bin/time_alarm.rs Switches the example to spawn_service!() and updates relay handler wiring accordingly.
espi-service/src/espi_service.rs Refactors eSPI service into Resources + Runner + control-handle Service implementing the new trait.
espi-service/src/task.rs Removes the old task wrapper API.
espi-service/src/lib.rs Stops exporting the removed task module.
Comments suppressed due to low confidence (4)

embedded-service/src/service.rs:27

  • Docs here still refer to an init() API, but the trait method is named new(...). Please update the wording so it matches the current public API and avoids sending users to a non-existent init() method.
/// 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<Output = Result<(Self, Self::Runner), Self::ErrorType>>;

embedded-service/src/service.rs:51

  • spawn_service! docs mention creating a OnceLock and calling init(), but the macro body uses StaticCell + Service::new(...). Please update this doc block to reflect the actual storage mechanism and call flow.
/// 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

time-alarm-service/src/lib.rs:300

  • Resources::new() is only provided via the ServiceResources trait impl, so callers must import that trait just to construct resources. Consider adding an inherent pub fn new() (or Default) and have the trait impl forward to it, to make manual instantiation ergonomic.

impl<'hw> embedded_services::service::ServiceResources for Resources<'hw> {
    /// Allocate storage for the service's resources.
    fn new() -> Self {
        Resources { inner: None }

espi-service/src/espi_service.rs:45

  • Resources::new() is only available via the ServiceResources trait method, so callers need that trait in scope (or UFCS) to construct resources. If you expect manual instantiation, consider adding an inherent pub fn new()/Default and forwarding the trait method to it.
{
    fn new() -> Self {
        Self { inner: None }
    }
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@williampMSFT
Copy link
Contributor Author

@asasine Robert mentioned you might have an interest in this - would love to get your feedback :)

@asasine
Copy link
Contributor

asasine commented Feb 28, 2026

Hey @williampMSFT, he's right, this is interesting. I think it has a lot of merit and we've been experimenting with a similar idea internally. We ultimately walked back on something as predefined like Service but did find some related benefits.

We found that the perceived rigidity of a Service-like trait wasn't appealing to devs without a strong background or inclination to Rust's more opinionated design guidelines as compared to C. One goal of the experiment was to simplify the process of breaking existing systems down into smaller components to improve extensibility and testability. However, the prototypes actually showed the opposite: the highly generic Service trait made boundaries between systems too fuzzy to identify clean cut points, and refactoring existing systems into the Service trait led to massive objects for the associated types (an accurate reflection of our existing monoliths but I digress) and that didn't garner much buy-in.

The main benefit we did discover was adding new traits for the behavior, which effectively makes the async fn run(self) -> ! generic over that trait. These can be spliced into existing systems with as small of a surface area as desired, making it low risk and immediately testable.

It's possible that a consolidated Service trait may become workable down the road, once we've broken down more of our monoliths. I'm hopeful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants