[RFC] Uniform task spawning (trait version)#726
[RFC] Uniform task spawning (trait version)#726williampMSFT wants to merge 4 commits intoOpenDevicePartnership:v0.2.0from
Conversation
There was a problem hiding this comment.
Pull request overview
Introduces a new embedded_services::service module that standardizes service initialization and “must-run” task driving via a RunnableService trait, a ServiceRunner handle, and an Embassy-focused spawn_service!() macro. This PR migrates time-alarm-service and espi-service away from per-service task modules to this uniform runner pattern, and updates tests/examples accordingly.
Changes:
- Add
RunnableService<'hw>,ServiceRunner,impl_runner_creation_token!(), andspawn_service!()toembedded-service. - Migrate
time-alarm-serviceto the new init/runner model (removetaskmodule; addServiceInitParams; implementRunnableService). - Migrate
espi-serviceto the new runner model (removetaskmodule; addServiceInitParams; implementRunnableService), and update an Embassy example + Tokio tests to drive viarunner.run().
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| time-alarm-service/tests/tad_test.rs | Updates tests to use RunnableService::init() returning (service, runner) and drive via runner.run() in tokio::select!. |
| time-alarm-service/src/task.rs | Removes the dedicated task wrapper in favor of the new ServiceRunner pattern. |
| time-alarm-service/src/lib.rs | Adds ServiceInitParams, implements RunnableService, and moves the run-loop into RunnableService::run. |
| examples/rt685s-evk/src/bin/time_alarm.rs | Switches to embedded_services::spawn_service!() to initialize+spawn the time-alarm service on Embassy. |
| espi-service/src/task.rs | Removes the dedicated task wrapper in favor of the new ServiceRunner pattern. |
| espi-service/src/lib.rs | Drops pub mod task export now that the task wrapper is removed. |
| espi-service/src/espi_service.rs | Adds ServiceInitParams and implements RunnableService (init returns (service, runner), run drives the loop). |
| embedded-service/src/service.rs | New module defining RunnableService, ServiceRunner, token macro, and spawn_service!() macro. |
| embedded-service/src/lib.rs | Exposes the new service module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
70e0f6f to
33b6b63
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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:
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_.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 adds a trait bound to RunnableService that requires an init() function with exactly two arguments. 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 instantiation of services requires the RunnableService trait to be in scope.