Make battery data caches OEM extensible#895
Open
tullom wants to merge 6 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the battery service to make cached battery data OEM-extensible by introducing StaticBatteryData / DynamicBatteryData traits and a generic State<S, D>, so OEM fuel-gauge drivers can embed the standard cache messages while adding custom fields, and the service can still answer ACPI queries from the standard subset.
Changes:
- Added extensible fuel-gauge state and cache model in
battery-service-interface(FuelGaugetrait + genericState<S, D>+ standard cache message structs). - Reworked
battery-serviceto own a fuel-gaugeRegistrationand answer ACPI queries by reading the fuel gauge’s cached state (removing the prior context/device/wrapper state-machine plumbing). - Updated examples and relay to the new registration/fuel-gauge ownership model and renamed the BatteryService PSR method to
is_psu_in_use.
Reviewed changes
Copilot reviewed 15 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/std/src/bin/battery.rs | Updates std example to OEM-owned FuelGauge behind a mutex; service answers ACPI by reading cached state. |
| examples/std/Cargo.lock | Lockfile updates reflecting removed deps from the new service model. |
| examples/pico-de-gallo/src/bin/battery.rs | Updates pico-de-gallo example to implement FuelGauge and drive caching directly. |
| examples/pico-de-gallo/Cargo.toml | Adjusts dependencies for new mutex/registration usage; updates cargo-machete ignores. |
| examples/pico-de-gallo/Cargo.lock | Lockfile updates reflecting dependency changes. |
| Cargo.lock | Workspace lockfile updates reflecting removed deps from battery-service. |
| battery-service/src/wrapper.rs | Removes legacy wrapper-based controller/device command loop. |
| battery-service/src/registration.rs | Adds Registration abstraction and array-backed registration implementation. |
| battery-service/src/mock.rs | Reworks mock fuel gauge to use cached State and provide coherent emulated pack constructors. |
| battery-service/src/lib.rs | Simplifies service to registration-based model; re-exports new fuel-gauge interface items. |
| battery-service/src/device.rs | Removes legacy device abstraction and per-device cache storage. |
| battery-service/src/controller.rs | Removes legacy controller trait abstraction. |
| battery-service/src/context.rs | Removes legacy context/state-machine/event plumbing. |
| battery-service/src/acpi.rs | Updates ACPI computations/handlers to read from extensible cached state; adds reference-based query APIs. |
| battery-service/Cargo.toml | Removes now-unneeded dependencies/features from the new service model. |
| battery-service-relay/src/lib.rs | Updates relay to call renamed is_psu_in_use method. |
| battery-service-interface/src/lib.rs | Exposes new fuel_gauge module; renames PSR method on BatteryService. |
| battery-service-interface/src/fuel_gauge.rs | Introduces FuelGauge trait, generic State<S, D>, and standard cache message types/traits for extensibility. |
Comments suppressed due to low confidence (1)
battery-service/src/mock.rs:495
set_at_rateis a no-op, so callers can't update the cachedat_rateused by the mock's getters (and by any logic that relies on the cached state). This breaks the expected Smart Battery behavior where AtRate is host-set and then reflected in subsequent reads.
async fn set_at_rate(&mut self, _rate: smart_battery::CapacityModeSignedValue) -> Result<(), Self::Error> {
Ok(())
}
4bed8fe to
72bcbdb
Compare
RobertZ2011
requested changes
Jun 16, 2026
…and move away from intrusive list and static lifetimes - Introduced a `Registration` trait and an `ArrayRegistration` implementation to manage fuel gauges. - OEMs handle their own state for each fuel gauge. - Replaced the `MockBatteryDriver` with `MockFuelGauge` to align with the new fuel gauge structure. - Removed the `wrapper.rs` and `controller.rs` files as its functionality is now integrated into the main battery service logic. - Modified the battery example applications to utilize the new registration system and updated the fuel gauge initialization and recovery logic.
- Fold the per-command DeviceId handlers into the BatteryService trait impl so each ACPI command has a single implementation: the trait methods look up and lock the fuel gauge, then delegate to the reference-based query methods that hold the actual logic - Make the Service registration field private and expose the two Registration trait methods through fuel_gauges() and get_fuel_gauge() accessors, so the registration implementation type is no longer part of the public API surface. - Replace the magic array lengths in StaticBatteryMsgs with named constants, note that the string fields are null-terminated, and size the mock scratch buffer from their compile-time maximum. Document why the mock's indexed slicing cannot panic.
- Introduce StaticBatteryData and DynamicBatteryData traits and make the fuel-gauge State generic over them (State<S, D>) so OEM drivers can cache a custom type that embeds the standard StaticBatteryMsgs / Dynamic BatteryMsgs and adds extra fields, while the battery service still reads the standard fields needed to answer ACPI queries. Rework the cache data model in the process: - Encode capacity and rate quantities with the Smart Battery CapacityModeValue / CapacityModeSignedValue enums so each value self-describes its unit (mA/mAh vs centiWatt) instead of assuming a fixed encoding. - Drop unit-suffixed field names in favor of the embedded-batteries typed aliases (MilliVolts, MilliAmps, Percent, Cycles, DeciKelvin, Minutes), keeping units in the types and docs. - Add the previously missing ACPI/SBS fields (manufacture_date, specification_info, remaining_capacity_alarm, at_rate and its predictions, run_time_to_empty, average_time_to_*), and document every field. Since the capacity enums have no Default, replace the derived Default on both cache structs with manual impls that default to the mA-zero variant. Update the mock fuel gauge to source every SmartBattery getter from the cached State and add coherent emulated-pack constructors (new, new_2s, new_4s) that scale pack voltage and power with the series cell count. Change State::on_static_data / on_dynamic_data to take an in-place updater closure (FnOnce(&mut S/D)) instead of consuming the data by value. The closure writes freshly read values directly into the cache storage, so a (potentially large or OEM-extended) S/D is never moved or copied through the call. Assisted-by: GitHub Copilot:claude-opus-4.8
9c3bb69 to
c998935
Compare
RobertZ2011
approved these changes
Jun 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fuel-gauge State generic over them (State<S, D>) so OEM drivers can cache
a custom type that embeds the standard StaticBatteryMsgs / Dynamic
BatteryMsgs and adds extra fields, while the battery service still reads
the standard fields needed to answer ACPI queries.
Rework the cache data model in the process:
CapacityModeValue / CapacityModeSignedValue enums so each value
self-describes its unit (mA/mAh vs centiWatt) instead of assuming a
fixed encoding.
typed aliases (MilliVolts, MilliAmps, Percent, Cycles, DeciKelvin,
Minutes), keeping units in the types and docs.
specification_info, remaining_capacity_alarm, at_rate and its
predictions, run_time_to_empty, average_time_to_*), and document
every field.
Since the capacity enums have no Default, replace the derived Default on
both cache structs with manual impls that default to the mA-zero variant.
Update the mock fuel gauge to source every SmartBattery getter from the
cached State and add coherent emulated-pack constructors (new, new_2s,
new_4s) that scale pack voltage and power with the series cell count.
Change State::on_static_data / on_dynamic_data to take an in-place
updater closure (FnOnce(&mut S/D)) instead of consuming the data by
value. The closure writes freshly read values directly into the cache
storage, so a (potentially large or OEM-extended) S/D is never moved or
copied through the call.
Assisted-by: GitHub Copilot:claude-opus-4.8
Depends on #893