[RFC] Uniform task spawning (structural-typed version)

[williampMSFT]

This is an option for uniform spawning of tasks. It attempts to make it more difficult to incorrectly use tasks by doing the following three things:

1. Restructure tasks such that only one long-running task is required, and that task does a 'select!' over all the tasks that were previously required. This makes it impossible for users to only launch some of the tasks they need, or to launch them in an order that doesn't work
2. Make each service's init() function return a Result<&'hw Self, ServiceRunner> rather than just a &'hw Self. The user is required to call .run() on the ServiceRunner for the service to function, which they will typically do from a task that they've spawned on their executor. This makes it difficult (but not impossible) for a user to miss spawning the task, because to do that, they have to ignore the ServiceRunner that was returned at init() time, which requires them to explicitly ignore it by binding it to a name with a leading _.
3. Create a spawn_service!() macro that does all the boilerplate of declaring storage and spawning tasks on Embassy for a given service. This doesn't work on Tokio, but Tokio is mostly intended for test usage, where it's generally desirable to select!() over the spawn token and some 'test arm' because that lets the lifetime of the service not be 'static, which is probably what you want for tests.

This variant does not add a trait bound to RunnableService that requires an init() function with exactly two arguments - rather, it forwards all extraneous arguments to the init() function. This approach has the upside of requiring very little ceremony - you can just directly call spawn_service!() with the args you want to pass through to init(), but comes at the cost of not catching issues with the type signature of the init() function that might make it not work well

This approach has the upside of catching things that won't work with the spawn_service!() macro at service compile time rather than spawn_service!() instantiation time, but comes at the cost of a bit more ceremony - each service has to declare an 'init params' struct, and usages outside of spawn_service!() require the RunnableService trait to be in scope.

[Copilot]

Pull request overview

This PR introduces a new uniform task spawning pattern for services to prevent incorrect usage. It restructures service initialization to make it harder to forget spawning required tasks or spawning them incorrectly.

Changes:

• Adds new RunnableService trait and ServiceRunner infrastructure to embedded-service that enforces proper service lifecycle management through the type system
• Migrates time-alarm-service and espi-service to the new pattern, consolidating multiple tasks into a single select!-based event loop and removing standalone task modules
• Provides spawn_service! macro for Embassy executors to reduce boilerplate and ensure correct initialization

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
embedded-service/src/service.rs	New module defining RunnableService trait, ServiceRunner handle, impl_runner_creation_token! macro, and spawn_service! macro
embedded-service/src/lib.rs	Adds public service module export
time-alarm-service/src/lib.rs	Migrates init() to return (service, runner) tuple and implements RunnableService with consolidated select! loop
time-alarm-service/src/task.rs	Removes standalone task module (replaced by RunnableService impl)
time-alarm-service/tests/tad_test.rs	Updates tests to destructure init() result and call runner.run() directly
espi-service/src/espi_service.rs	Migrates init() to return (service, runner) tuple and implements RunnableService with consolidated event loop
espi-service/src/task.rs	Removes standalone task module (replaced by RunnableService impl)
espi-service/src/lib.rs	Removes task module export
examples/rt685s-evk/src/bin/time_alarm.rs	Demonstrates spawn_service! macro usage, replacing manual task spawning boilerplate

---

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

[Copilot]

The documentation example is inconsistent with the actual usage in the codebase. The example shows 4 parameters (including time_alarm_task as the first parameter), but the actual macro signature and usage (in examples/rt685s-evk/src/bin/time_alarm.rs) only require 3 parameters: spawner, service_ty, and init_args. The first parameter shown in the example (time_alarm_task) is not part of the macro's signature.

Remove time_alarm_task, from the example to match the actual macro usage.

[Copilot]

The macro documentation should explicitly state that spawn_service! must be called from an async context, since it internally awaits the service's init() function. This is not immediately obvious from the current documentation.

Add a note in the documentation such as: "This macro must be called from an async context (e.g., inside an async function or the main embassy executor task) because it internally awaits the service initialization."

[Copilot]

The init function now returns Result<..., Infallible> which can never fail. This is unnecessarily verbose for an infallible operation. Consider either:

1. Removing the Result wrapper entirely and returning just the tuple
2. Adding a clearer comment explaining why Result is used despite Infallible (e.g., for API consistency with other services that do have fallible init)

This affects API ergonomics as callers must unnecessarily unwrap or handle a Result that can never be Err.

[Copilot]

The ServiceRunner struct should implement Drop to warn if it's dropped without being run, as this would leave the service non-functional. Currently, silently dropping the runner means the service won't process any events, which could lead to hard-to-debug issues.

Consider adding a Drop implementation that logs a warning or panics in debug builds if the runner is dropped without having been consumed by the run() method. This could be tracked with a boolean flag that's set when run() consumes self.

[Copilot]

The macro uses must_spawn which will panic if spawning fails. This might not be appropriate for all use cases, especially during testing or in contexts where graceful error handling is preferred.

Consider either:

1. Documenting this behavior clearly in the macro documentation
2. Providing an alternative macro or parameter that uses spawn instead and propagates the error
3. Using unwrap() with a more descriptive panic message that helps debug which service failed to spawn

The current implementation is reasonable for embedded contexts where panicking on failed initialization is acceptable, but users should be aware of this behavior.