Skip to content

thermal-service: Implement runnable service traits#758

Merged
kurtjd merged 1 commit intoOpenDevicePartnership:v0.2.0from
kurtjd:thermal-runnable-refactor
Mar 20, 2026
Merged

thermal-service: Implement runnable service traits#758
kurtjd merged 1 commit intoOpenDevicePartnership:v0.2.0from
kurtjd:thermal-runnable-refactor

Conversation

@kurtjd
Copy link
Contributor

@kurtjd kurtjd commented Mar 19, 2026

This PR implements the recently introduced odp_common::runnable_service traits for the thermal service.

However, this required a little bit of refactoring and rethinking. There was some initial difficulty in making this work for the thermal service as-is due to the fact that we don't know upfront how many sensors and fans will be instantiated by the user, making the ServiceRunner trait tricky.

Speaking with @williampMSFT, he pointed out that the thermal-service itself is not really a runnable service, but it's better to think of each sensor and fan as a standalone runnable service which is a good idea. Thus, I implement the traits for sensor and fans instead (with the thermal-service struct acting as a coordination and relay handler).

However, this still does point out some other little things I want to revisit and refactor. The plan is to move away from IPCs which I currently make use of with the Device struct as an intermediary between the thermal-service handler and individual sensor and fan service handlers. Also want to revisit how I can potentially remove the need for the fan service to need a thermal service handler to just talk to a sensor. This will require a bit more thought and I think I'll try to address some of this in the follow-up refactor where we start separating the interface from the implementation (see: #748).

This serves as a good start to unblock us and keep moving.

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

This PR migrates thermal-service to the new odp_service_common::runnable_service model by making each fan and sensor independently spawnable/runnable, while keeping the top-level thermal service as a coordinating handle for IPC/event routing.

Changes:

  • Remove the old task module entrypoints and replace them with runnable_service::Service + ServiceRunner implementations for fan and sensor.
  • Update the examples/std thermal example to spawn fan/sensor services via spawn_service!.
  • Add odp-service-common as a dependency where needed and update lockfiles.

Reviewed changes

Copilot reviewed 6 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
thermal-service/src/task.rs Deletes legacy fan/sensor task helpers now superseded by runnable service runners.
thermal-service/src/lib.rs Removes pub mod task; export consistent with task module deletion.
thermal-service/src/sensor.rs Adds runnable-service Resources, Service, Runner, and init params for sensors.
thermal-service/src/fan.rs Adds runnable-service Resources, Service, Runner, and init params for fans.
thermal-service/Cargo.toml Adds dependency on odp-service-common.
examples/std/src/bin/thermal.rs Updates example to spawn fan/sensor services via odp_service_common::spawn_service!.
examples/std/Cargo.toml Adds odp-service-common dependency for the std example.
examples/std/Cargo.lock Records the new odp-service-common dependency in the example lockfile.
Cargo.lock Records odp-service-common in the workspace lockfile due to the new thermal-service dependency.

Copy link
Contributor

@williampMSFT williampMSFT left a comment

Choose a reason for hiding this comment

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

:shipit:

@kurtjd kurtjd merged commit cabc36e into OpenDevicePartnership:v0.2.0 Mar 20, 2026
19 checks passed
@kurtjd kurtjd deleted the thermal-runnable-refactor branch March 20, 2026 20:30
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.

4 participants