Skip to content

Battery service: Split interface and relay logic into separate crates#780

Open
williampMSFT wants to merge 8 commits intoOpenDevicePartnership:feature/relay-splitfrom
williampMSFT:user/williamp/battery-relay-split
Open

Battery service: Split interface and relay logic into separate crates#780
williampMSFT wants to merge 8 commits intoOpenDevicePartnership:feature/relay-splitfrom
williampMSFT:user/williamp/battery-relay-split

Conversation

@williampMSFT
Copy link
Copy Markdown
Contributor

@williampMSFT williampMSFT commented Apr 8, 2026

This change splits the interface for the battery service into two crates - an 'interface' crate that contains traits and support types for the public interface for the battery service, and a 'relay' crate that's written against the 'interface' crate and handles relaying messages to and from a battery service implementation over MCTP.

This split should allow OEMs to substitute their own service implementations if they want to and still reuse our relay code (or write their own relay against the stock service implementation). Additionally, splitting the relay handling from the service implementation should also allow us to add support for different kinds of relay handlers (e.g. HID) without needing to pay the code size cost of having that code on platforms that aren't using it since it's no longer invasive on the service type.

This change only moves the pieces of the battery interface that are required for the relay service into a trait; a future change will migrate the pieces that are targeted at other code running on the EC into the trait as well.

Copilot AI review requested due to automatic review settings April 8, 2026 19:34
@williampMSFT williampMSFT requested review from a team as code owners April 8, 2026 19:34
Copy link
Copy Markdown
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 splits the battery service’s public interface and its MCTP relay/serialization logic into separate crates to allow alternate service implementations to reuse the relay layer.

Changes:

  • Introduces battery-service-interface with the battery service trait, error type, and shared ACPI-support types.
  • Adds battery-service-relay to handle ACPI request/response serialization and relay dispatch against the interface trait.
  • Updates battery-service to implement the new BatteryService trait and removes the in-crate MCTP relay handler and related dependencies.

Reviewed changes

Copilot reviewed 13 out of 17 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
examples/std/Cargo.lock Updates example lockfile to depend on battery-service-interface after the split.
examples/rt633/Cargo.lock Updates example lockfile dependency from messages crate to interface crate.
examples/pico-de-gallo/Cargo.lock Updates example lockfile dependency from messages crate to interface crate.
Cargo.toml Updates workspace members and workspace dependency alias from battery-service-messages to battery-service-interface, and adds battery-service-relay.
Cargo.lock Updates workspace lock to include battery-service-interface and new battery-service-relay crate.
battery-service/src/mock.rs Switches mock capability flag types to battery-service-interface.
battery-service/src/lib.rs Implements battery_service_interface::BatteryService and removes in-crate MCTP relay handler impls.
battery-service/src/device.rs Re-exports DeviceId from battery-service-interface.
battery-service/src/context.rs Removes ACPI request processing tied to the old messages/relay flow and updates imports.
battery-service/src/acpi.rs Refactors handlers to return interface types directly and use BatteryError.
battery-service/Cargo.toml Replaces dependency on old messages crate with battery-service-interface and drops relay-related deps.
battery-service-relay/src/serialization.rs New: serialization/deserialization for ACPI battery requests/responses and error mapping for relay transport.
battery-service-relay/src/lib.rs New: relay handler that translates ACPI requests into BatteryService calls.
battery-service-relay/Cargo.toml New: crate manifest for relay layer.
battery-service-messages/src/lib.rs Removed: old combined messages + serialization crate superseded by interface + relay crates.
battery-service-interface/src/lib.rs New: defines the battery service trait and shared support types/errors.
battery-service-interface/Cargo.toml Renames the old messages crate package to battery-service-interface.

kurtjd
kurtjd previously approved these changes Apr 8, 2026
Copy link
Copy Markdown
Contributor

@kurtjd kurtjd left a comment

Choose a reason for hiding this comment

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

LGTM. Though similar nit as last time, just some pub items missing doc strings in the interface and relay lib.rs files.

Copilot AI review requested due to automatic review settings April 8, 2026 21:05
kurtjd
kurtjd previously approved these changes Apr 8, 2026
Copy link
Copy Markdown
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

Copilot reviewed 13 out of 17 changed files in this pull request and generated 3 comments.

kurtjd
kurtjd previously approved these changes Apr 8, 2026
jerrysxie
jerrysxie previously approved these changes Apr 10, 2026
Copy link
Copy Markdown
Contributor

@jerrysxie jerrysxie left a comment

Choose a reason for hiding this comment

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

LGTM, but I only looked at the high-level structure. I will let @tullom focus on the individual battery API calls.

Copilot AI review requested due to automatic review settings April 10, 2026 18:43
@williampMSFT williampMSFT dismissed stale reviews from jerrysxie and kurtjd via dbb5670 April 10, 2026 18:43
Copy link
Copy Markdown
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

Copilot reviewed 13 out of 17 changed files in this pull request and generated 2 comments.

tullom
tullom previously approved these changes Apr 10, 2026
Copy link
Copy Markdown
Contributor

@tullom tullom left a comment

Choose a reason for hiding this comment

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

Just one nb comment, otherwise looks good! The API looks good

Copilot AI review requested due to automatic review settings April 10, 2026 20:54
Copy link
Copy Markdown
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

Copilot reviewed 13 out of 18 changed files in this pull request and generated 1 comment.

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.

5 participants