From ab0f79d235d08844e59b13f92dc0250c7f50670e Mon Sep 17 00:00:00 2001 From: Matteo Tullo Date: Thu, 22 Oct 2026 13:08:56 -0700 Subject: [PATCH 1/5] feat(battery)!: route power-policy events Cache routed power-policy updates in each fuel gauge and use them for ACPI power-source queries. Share a power-policy subscriber with TCPM and integrate event handling into the battery examples. BREAKING CHANGE: Registrations must provide a power-policy router; ArrayRegistration now requires its routing type and value. Assisted-by: GitHub Copilot:gpt-6-astra --- Cargo.lock | 2 + battery-service-interface/Cargo.toml | 9 +- battery-service-interface/src/fuel_gauge.rs | 43 +++++- battery-service-interface/src/lib.rs | 130 +++++++++++++++++- battery-service/Cargo.toml | 1 + battery-service/src/acpi.rs | 97 +++++++------- battery-service/src/event_receiver.rs | 79 +++++++++++ battery-service/src/lib.rs | 50 ++++++- battery-service/src/registration.rs | 43 ++++-- battery-service/src/routing.rs | 64 +++++++++ examples/pico-de-gallo/Cargo.lock | 2 + examples/pico-de-gallo/src/bin/battery.rs | 6 +- examples/std/Cargo.lock | 2 + examples/std/src/bin/battery.rs | 40 +++++- examples/std/src/bin/power_policy.rs | 141 ++++++++++++++++++-- power-policy-interface/src/psu/mod.rs | 3 +- power-policy-interface/src/service/event.rs | 31 ++++- tcpm-service/src/service/event_receiver.rs | 19 +-- 18 files changed, 653 insertions(+), 109 deletions(-) create mode 100644 battery-service/src/event_receiver.rs create mode 100644 battery-service/src/routing.rs diff --git a/Cargo.lock b/Cargo.lock index a234b82f5..2a0783cac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -118,6 +118,7 @@ version = "0.1.0" dependencies = [ "battery-service-interface", "defmt 0.3.100", + "embassy-futures", "embassy-time", "embedded-batteries-async", "embedded-services", @@ -132,6 +133,7 @@ dependencies = [ "defmt 0.3.100", "embedded-batteries-async", "log", + "power-policy-interface", ] [[package]] diff --git a/battery-service-interface/Cargo.toml b/battery-service-interface/Cargo.toml index e9c9538f5..d4a6b3864 100644 --- a/battery-service-interface/Cargo.toml +++ b/battery-service-interface/Cargo.toml @@ -12,10 +12,15 @@ ignored = ["log"] defmt = { workspace = true, optional = true } log = { workspace = true, optional = true } embedded-batteries-async.workspace = true +power-policy-interface.workspace = true [lints] workspace = true [features] -defmt = ["dep:defmt", "embedded-batteries-async/defmt"] -log = ["dep:log"] +defmt = [ + "dep:defmt", + "embedded-batteries-async/defmt", + "power-policy-interface/defmt", +] +log = ["dep:log", "power-policy-interface/log"] diff --git a/battery-service-interface/src/fuel_gauge.rs b/battery-service-interface/src/fuel_gauge.rs index a80f47c69..3b3b842ba 100644 --- a/battery-service-interface/src/fuel_gauge.rs +++ b/battery-service-interface/src/fuel_gauge.rs @@ -17,6 +17,7 @@ use embedded_batteries_async::{ MilliAmpsSigned, Minutes, Percent, }, }; +pub use power_policy_interface::psu::PsuState; /// Fuel gauge errors. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -381,9 +382,10 @@ pub enum InternalState { /// Fuel gauge state, owned by the driver (OEM) and managed via the `on_*` transition methods. /// -/// This holds both the fuel gauge state machine state and the cached static and -/// dynamic battery data. The battery service reads this state (via -/// [`FuelGauge::state`]) when answering ACPI queries. +/// This holds the fuel gauge's internal state, cached static and dynamic battery +/// data, and PSU state from routed power-policy notifications. It keeps no PSU +/// reference. The battery service reads this state through +/// [`FuelGauge::state`] to answer ACPI queries. /// /// The cached data types are generic and default to the standard /// [`StaticBatteryMsgs`] / [`DynamicBatteryMsgs`]. OEMs may substitute custom @@ -394,6 +396,8 @@ pub struct State State { @@ -422,6 +426,39 @@ impl State { &mut self.dynamic_cache } + /// The cached state of this gauge's external power source. + /// + /// Defaults to [`PsuState::Detached`] until a state is reported. + /// [`PsuState::Idle`] means the source is attached but not supplying power. + /// Unconstrained power is tracked separately by [`Self::is_psu_unconstrained`]. + pub fn psu_state(&self) -> PsuState { + self.psu_state + } + + /// Update external power state from a routed notification. + /// + /// Refreshes the unconstrained flag from the consumer capability, or clears it + /// for other states. + /// Leaves the fuel gauge's presence and operational state unchanged. + pub fn set_psu_state(&mut self, state: PsuState) { + self.psu_state = state; + self.psu_unconstrained = + matches!(state, PsuState::ConnectedConsumer(capability) if capability.flags.unconstrained_power); + } + + /// Whether cached external power is online with unlimited output. + pub fn is_psu_unconstrained(&self) -> bool { + self.psu_unconstrained + } + + /// Set the unconstrained override without changing the cached PSU state. + /// + /// Clearing it restores the cached consumer's negotiated wattage, or offline + /// status if no consumer is connected. + pub fn set_psu_unconstrained(&mut self, unconstrained: bool) { + self.psu_unconstrained = unconstrained; + } + /// Returns `true` if the fuel gauge is present. pub fn is_present(&self) -> bool { matches!(self.state, InternalState::Present(_)) diff --git a/battery-service-interface/src/lib.rs b/battery-service-interface/src/lib.rs index a46ecb73e..4a72a9404 100644 --- a/battery-service-interface/src/lib.rs +++ b/battery-service-interface/src/lib.rs @@ -23,6 +23,11 @@ pub const STD_PIF_SERIAL_SIZE: usize = 8; /// Standard Power Policy Service OEM Info String Size pub const STD_PIF_OEM_SIZE: usize = 8; +/// Service-specific sentinel for unlimited output power (mW). +/// +/// `0xFFFFFFFF` remains reserved for unavailable power. +pub const UNLIMITED_POWER_MW: u32 = u32::MAX - 1; + #[derive(PartialEq, Clone, Copy, Default)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub struct BixFixedStrings { @@ -77,7 +82,7 @@ pub struct PifFixedStrings { pub power_source_state: PowerSourceState, /// Maximum rated output power in milliwatts (mW). /// - /// 0xFFFFFFFF indicates the value is unavailable. + /// [`UNLIMITED_POWER_MW`] indicates unconstrained output; `0xFFFFFFFF` means unavailable. pub max_output_power: u32, /// Maximum rated input power in milliwatts (mW). /// @@ -96,6 +101,7 @@ pub struct PifFixedStrings { #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub struct DeviceId(pub u8); +/// Host-facing battery operations. pub trait BatteryService { /// Queries the estimated time remaining until the battery reaches the specified charge level. Corresponds to ACPI's _BCT method fn battery_charge_time( @@ -177,12 +183,20 @@ pub trait BatteryService { ) -> impl core::future::Future>; /// Queries whether the power supply unit is currently in use (i.e., providing power to the system). Corresponds to ACPI's _PSR method. + /// + /// The query reads the cached external-power state of the fuel gauge selected by `power_source_id`. + /// A cached `ConnectedConsumer` state or unconstrained-power override reports online. + /// Other sources report offline. An unregistered ID returns an error. fn is_psu_in_use( &self, power_source_id: DeviceId, ) -> impl core::future::Future>; /// Queries information about the battery's power source. Corresponds to ACPI's _PIF method. + /// + /// The query reads the cached external-power state of the fuel gauge selected by `power_source_id`. + /// Unconstrained output is [`UNLIMITED_POWER_MW`]; otherwise it is the cached consumer's + /// negotiated wattage, or unknown without a consumer. An unregistered ID returns an error. fn power_source_information( &self, power_source_id: DeviceId, @@ -195,6 +209,120 @@ pub trait BatteryService { ) -> impl core::future::Future>; } +impl BatteryService for &T { + fn battery_charge_time( + &self, + battery_id: DeviceId, + charge_level: Bct, + ) -> impl core::future::Future> { + T::battery_charge_time(*self, battery_id, charge_level) + } + + fn battery_info( + &self, + battery_id: DeviceId, + ) -> impl core::future::Future> { + T::battery_info(*self, battery_id) + } + + fn set_battery_measurement_averaging_interval( + &self, + battery_id: DeviceId, + bma: Bma, + ) -> impl core::future::Future> { + T::set_battery_measurement_averaging_interval(*self, battery_id, bma) + } + + fn battery_maintenance_control( + &self, + battery_id: DeviceId, + bmc: Bmc, + ) -> impl core::future::Future> { + T::battery_maintenance_control(*self, battery_id, bmc) + } + + fn battery_maintenance_data( + &self, + battery_id: DeviceId, + ) -> impl core::future::Future> { + T::battery_maintenance_data(*self, battery_id) + } + + fn set_battery_measurement_sampling_time( + &self, + battery_id: DeviceId, + battery_measurement_sampling: Bms, + ) -> impl core::future::Future> { + T::set_battery_measurement_sampling_time(*self, battery_id, battery_measurement_sampling) + } + + fn battery_power_characteristics( + &self, + battery_id: DeviceId, + ) -> impl core::future::Future> { + T::battery_power_characteristics(*self, battery_id) + } + + fn battery_power_state( + &self, + battery_id: DeviceId, + ) -> impl core::future::Future> { + T::battery_power_state(*self, battery_id) + } + + fn set_battery_power_threshold( + &self, + battery_id: DeviceId, + power_threshold: Bpt, + ) -> impl core::future::Future> { + T::set_battery_power_threshold(*self, battery_id, power_threshold) + } + + fn battery_status( + &self, + battery_id: DeviceId, + ) -> impl core::future::Future> { + T::battery_status(*self, battery_id) + } + + fn battery_time_to_empty( + &self, + battery_id: DeviceId, + battery_discharge_rate: Btm, + ) -> impl core::future::Future> { + T::battery_time_to_empty(*self, battery_id, battery_discharge_rate) + } + + fn set_battery_trip_point( + &self, + battery_id: DeviceId, + btp: Btp, + ) -> impl core::future::Future> { + T::set_battery_trip_point(*self, battery_id, btp) + } + + fn is_psu_in_use( + &self, + power_source_id: DeviceId, + ) -> impl core::future::Future> { + T::is_psu_in_use(*self, power_source_id) + } + + fn power_source_information( + &self, + power_source_id: DeviceId, + ) -> impl core::future::Future> { + T::power_source_information(*self, power_source_id) + } + + fn device_status( + &self, + battery_id: DeviceId, + ) -> impl core::future::Future> { + T::device_status(*self, battery_id) + } +} + #[derive(Copy, Clone, Debug, PartialEq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] /// Errors that can occur when interacting with the battery service. diff --git a/battery-service/Cargo.toml b/battery-service/Cargo.toml index cea3dfede..4e34b8b00 100644 --- a/battery-service/Cargo.toml +++ b/battery-service/Cargo.toml @@ -16,6 +16,7 @@ workspace = true [dependencies] defmt = { workspace = true, optional = true } battery-service-interface.workspace = true +embassy-futures.workspace = true embassy-time.workspace = true embedded-batteries-async.workspace = true embedded-services.workspace = true diff --git a/battery-service/src/acpi.rs b/battery-service/src/acpi.rs index 06b89cfd0..374fb9a8f 100644 --- a/battery-service/src/acpi.rs +++ b/battery-service/src/acpi.rs @@ -2,29 +2,18 @@ use battery_service_interface::BatteryError; use battery_service_interface::fuel_gauge::{DynamicBatteryData, FuelGauge, StaticBatteryData}; -use embedded_batteries_async::acpi::{PowerSourceState, PowerUnit}; +use embedded_batteries_async::acpi::{PowerSource, PowerSourceState, PowerUnit}; use embedded_batteries_async::smart_battery::CapacityModeValue; use embedded_services::sync::Lockable; -use embedded_services::{info, trace}; +use embedded_services::{error, info, trace}; use battery_service_interface::{ BctReturnResult, BixFixedStrings, Bmd, Bpc, Bps, BstReturn, BtmReturnResult, DeviceId, PifFixedStrings, PsrReturn, STD_BIX_BATTERY_SIZE, STD_BIX_MODEL_SIZE, STD_BIX_OEM_SIZE, STD_BIX_SERIAL_SIZE, STD_PIF_MODEL_SIZE, - STD_PIF_OEM_SIZE, STD_PIF_SERIAL_SIZE, StaReturn, + STD_PIF_OEM_SIZE, STD_PIF_SERIAL_SIZE, StaReturn, UNLIMITED_POWER_MW, }; -use power_policy_interface::capability::PowerCapability; - -/// Cached power-supply state used when answering ACPI power-source queries. -/// -/// Currently always the default; this is a placeholder for future power policy -/// integration. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub(crate) struct PsuState { - pub psu_connected: bool, - pub power_capability: Option, -} +use power_policy_interface::psu::PsuState; /// Extract the raw numeric value from a [`CapacityModeValue`], discarding the unit /// tag. The unit (mA/mAh vs centiWatt) is conveyed to ACPI separately via the BIX @@ -176,32 +165,32 @@ pub(crate) fn compute_sta() -> embedded_batteries_async::acpi::StaReturn { embedded_batteries_async::acpi::StaReturn::all() } -pub(crate) fn compute_psr(psu_state: &PsuState) -> embedded_batteries_async::acpi::PsrReturn { - // TODO: Refactor to check if battery if force discharged, - // which should give an offline result even when the PSU is attached. - embedded_batteries_async::acpi::PsrReturn { - power_source: if psu_state.psu_connected { - embedded_batteries_async::acpi::PowerSource::Online +pub(crate) fn compute_psr(psu_state: PsuState, unconstrained: bool) -> PsrReturn { + PsrReturn { + power_source: if unconstrained || matches!(psu_state, PsuState::ConnectedConsumer(_)) { + PowerSource::Online } else { - embedded_batteries_async::acpi::PowerSource::Offline + PowerSource::Offline }, } } -pub(crate) fn compute_pif(psu_state: &PsuState) -> PifFixedStrings { - // TODO: Grab real values from power policy - let capability = psu_state.power_capability.unwrap_or(PowerCapability { - voltage_mv: 0, - current_ma: 0, - }); - +pub(crate) fn compute_pif(psu_state: PsuState, unconstrained: bool) -> PifFixedStrings { PifFixedStrings { power_source_state: PowerSourceState::empty(), - max_output_power: capability.max_power_mw(), - max_input_power: capability.max_power_mw(), - model_number: [0u8; STD_PIF_MODEL_SIZE], - serial_number: [0u8; STD_PIF_SERIAL_SIZE], - oem_info: [0u8; STD_PIF_OEM_SIZE], + // Policy reports the available or negotiated output, which may differ from the adapter's nameplate rating. + max_output_power: if unconstrained { + UNLIMITED_POWER_MW + } else { + match psu_state { + PsuState::ConnectedConsumer(capability) => capability.capability.max_power_mw(), + _ => u32::MAX, + } + }, + max_input_power: u32::MAX, + model_number: [0; STD_PIF_MODEL_SIZE], + serial_number: [0; STD_PIF_SERIAL_SIZE], + oem_info: [0; STD_PIF_OEM_SIZE], } } @@ -212,21 +201,25 @@ impl<'hw, Reg: crate::registration::Registration<'hw>> crate::Service<'hw, Reg> /// [`DeviceId`] into its registered fuel gauge with this helper, locks it, and delegates to the /// corresponding reference-based query method below, which holds the actual logic. pub(super) fn fuel_gauge(&self, device_id: DeviceId) -> Result<&'hw Reg::FuelGauge, BatteryError> { - self.registration - .get_fuel_gauge(device_id) - .ok_or(BatteryError::UnknownDeviceId) + self.registration.get_fuel_gauge(device_id).ok_or_else(|| { + error!("Unknown fuel gauge ID {:?}", device_id); + BatteryError::UnknownDeviceId + }) } } /// Reference-based ACPI query API. /// -/// Unlike the [`BatteryService`](battery_service_interface::BatteryService) trait -/// methods (which identify a battery by [`DeviceId`] and look the fuel gauge up -/// through the service's [`Registration`](crate::registration::Registration)), -/// these methods take an exclusive reference to the fuel gauge directly. The -/// exclusive borrow proves the caller has sole access to the fuel gauge's cached -/// state for the duration of the query, replacing the registration lookup and -/// runtime lock with a compile-time guarantee. +/// Battery queries take an exclusive reference to the fuel gauge. The borrow +/// guarantees at compile time that the caller has sole access to its cached +/// state during the query. No registration lookup or runtime lock is needed. +/// +/// The [`BatteryService`](battery_service_interface::BatteryService) trait +/// methods use a [`DeviceId`] to look up the fuel gauge through the service's +/// [`Registration`](crate::registration::Registration). +/// +/// Power-source queries use the same reference-based API and read only the +/// external PSU state cached in the fuel gauge. /// /// TODO: Use this over DeviceId based approach? impl<'hw, Reg: crate::registration::Registration<'hw>> crate::Service<'hw, Reg> { @@ -363,19 +356,27 @@ impl<'hw, Reg: crate::registration::Registration<'hw>> crate::Service<'hw, Reg> /// Queries whether the power supply unit is currently in use (i.e., providing power to the system). Corresponds to ACPI's _PSR method. pub fn is_psu_in_use( &self, - _fuel_gauge: &mut ::Inner, + fuel_gauge: &mut ::Inner, ) -> Result { trace!("Battery service: got PSR command!"); - Ok(compute_psr(&PsuState::default())) + let state = fuel_gauge.state(); + Ok(compute_psr(state.psu_state(), state.is_psu_unconstrained())) } /// Queries information about the battery's power source. Corresponds to ACPI's _PIF method. + /// + /// Reports [`UNLIMITED_POWER_MW`] for unconstrained power, otherwise the cached + /// consumer's negotiated wattage or `u32::MAX` without a consumer. + /// The negotiated value may be below the rated maximum. Input rating is always unknown. + /// Notifications do not include adapter identity or redundancy/shared-source + /// flags, so the strings are empty and those flags are clear. pub fn power_source_information( &self, - _fuel_gauge: &mut ::Inner, + fuel_gauge: &mut ::Inner, ) -> Result { trace!("Battery service: got PIF command!"); - Ok(compute_pif(&PsuState::default())) + let state = fuel_gauge.state(); + Ok(compute_pif(state.psu_state(), state.is_psu_unconstrained())) } /// Queries the battery's status. Corresponds to ACPI's _STA method. diff --git a/battery-service/src/event_receiver.rs b/battery-service/src/event_receiver.rs new file mode 100644 index 000000000..1e438797a --- /dev/null +++ b/battery-service/src/event_receiver.rs @@ -0,0 +1,79 @@ +use embassy_futures::select::{Either, select}; +use embedded_services::event::Receiver; +use power_policy_interface::service::event::{EventData as PowerPolicyEventData, PowerPolicySubscriber}; + +/// Events handled by a battery loop that polls fuel gauges and receives power-policy notifications. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum Event { + /// The caller's polling deadline is ready. + Poll, + /// A power-policy notification to route through the service. + PowerPolicy(E), +} + +/// Receives power-policy notifications for the battery service. +/// +/// The subscriber wraps a caller-provided receiver and is independent of +/// [`Service`](crate::Service). Use full events to route by source identity, +/// or payload events for a dedicated source. The receiver's event type +/// must match [`PowerPolicyRouting::Event`](crate::PowerPolicyRouting::Event) +/// for the registration's router. +/// +/// Deliver every event in order to [`Service::process_power_policy_event`](crate::Service::process_power_policy_event). +/// The handler updates gauge caches without locking a PSU, so bounded channels +/// can apply backpressure independently of the PSU lock used by live queries. +/// +/// Use [`Self::wait_next_with_poll`] to handle notifications and periodic +/// fuel-gauge polling in an existing task. +pub struct EventReceiver, E = PowerPolicyEventData> { + power_policy_event_subscriber: PowerPolicySubscriber, + poll_first: bool, +} + +impl, E> EventReceiver { + /// Create a battery event receiver from power-policy notifications. + pub fn new(power_policy_event_receiver: PowerReceiver) -> Self { + Self { + power_policy_event_subscriber: PowerPolicySubscriber::new(power_policy_event_receiver), + poll_first: true, + } + } + + /// Wait for a polling deadline or a power-policy notification. + /// + /// Pass a cancellation-safe wait such as [`embassy_time::Ticker::next`]. + /// Keep the ticker outside the event loop so notifications do not reset its + /// deadline. When both sources are ready, priority alternates between polling + /// and notifications, starting with polling, so neither a busy notification + /// stream nor overdue sampling can starve the other source. + /// + /// The losing wait is dropped. Do not pass hardware updates or event + /// processing as `poll`; finish those operations after this method returns. + /// The underlying notification receiver must also support cancelled waits. + pub async fn wait_next_with_poll(&mut self, poll: impl Future) -> Event { + let event = if self.poll_first { + match select(poll, self.power_policy_event_subscriber.wait_next()).await { + Either::First(()) => Event::Poll, + Either::Second(event) => Event::PowerPolicy(event), + } + } else { + match select(self.power_policy_event_subscriber.wait_next(), poll).await { + Either::First(event) => Event::PowerPolicy(event), + Either::Second(()) => Event::Poll, + } + }; + self.poll_first = matches!(&event, Event::PowerPolicy(_)); + event + } +} + +impl, E> Receiver for EventReceiver { + fn try_next(&mut self) -> Option { + self.power_policy_event_subscriber.try_next() + } + + fn wait_next(&mut self) -> impl Future { + self.power_policy_event_subscriber.wait_next() + } +} diff --git a/battery-service/src/lib.rs b/battery-service/src/lib.rs index e373468a2..68751a080 100644 --- a/battery-service/src/lib.rs +++ b/battery-service/src/lib.rs @@ -5,21 +5,26 @@ use battery_service_interface::{ BtmReturnResult, Btp, PifFixedStrings, PsrReturn, StaReturn, }; use core::marker::PhantomData; -use embedded_services::info; use embedded_services::sync::Lockable; +use embedded_services::{info, trace}; mod acpi; +pub mod event_receiver; #[cfg(feature = "mock")] pub mod mock; pub mod registration; +pub mod routing; +pub use event_receiver::{Event, EventReceiver}; +pub use power_policy_interface::service::event::EventData as PowerPolicyEventData; pub use registration::{ArrayRegistration, Registration}; +pub use routing::{DefaultPowerPolicyRouting, PowerPolicyRouting, PowerPolicyUpdate}; // Re-export the fuel gauge interface so that OEM drivers and integrators can // implement and use the battery service without depending on the interface crate directly. pub use battery_service_interface::fuel_gauge::{ DynamicBatteryData, DynamicBatteryMsgs, FuelGauge, FuelGaugeError, InternalState, OperationalSubstate, - PresentSubstate, State, StaticBatteryData, StaticBatteryMsgs, + PresentSubstate, PsuState, State, StaticBatteryData, StaticBatteryMsgs, }; pub use battery_service_interface::{BatteryService, DeviceId}; @@ -29,6 +34,8 @@ pub use battery_service_interface::{BatteryService, DeviceId}; /// ACPI battery queries (via the [`BatteryService`] trait) by reading each /// registered fuel gauge's cached state. The OEM drives each registered fuel /// gauge directly through the [`FuelGauge`] trait methods. +/// +/// Each fuel gauge caches its external [`PsuState`] and unconstrained override in [`State`]. pub struct Service<'hw, Reg: Registration<'hw>> { registration: Reg, _phantom: PhantomData<&'hw ()>, @@ -44,6 +51,45 @@ impl<'hw, Reg: Registration<'hw>> Service<'hw, Reg> { } } + /// Apply a notification through the registration's router. + /// + /// [`PowerPolicyRouting::route`] selects each fuel gauge's cache updates. + /// The service ignores empty routes. For other + /// routes, it validates all target IDs before changing any cache, then + /// applies the updates without locking or retaining a PSU. + /// + /// Set up reliable, ordered event delivery before power policy starts. Await + /// this handler outside any selection that could cancel it. Updates to + /// multiple gauges take one lock at a time and are not atomic. Keep the + /// event until the handler finishes. If processing is cancelled, retry the + /// event idempotently before processing newer events. + pub async fn process_power_policy_event( + &self, + event: &::Event, + ) -> Result<(), BatteryError> { + let updates = self.registration.routing().route(event); + if updates.clone().next().is_none() { + trace!("Battery service: no fuel gauge updates for power-policy event"); + return Ok(()); + } + for (id, _) in updates.clone() { + self.fuel_gauge(id)?; + } + for (id, update) in updates { + { + let mut gauge = self.fuel_gauge(id)?.lock().await; + match update { + PowerPolicyUpdate::PsuState(state) => gauge.state_mut().set_psu_state(state), + PowerPolicyUpdate::Unconstrained(unconstrained) => { + gauge.state_mut().set_psu_unconstrained(unconstrained); + } + } + } + info!("Battery service: fuel gauge {:?} external power: {:?}", id, update); + } + Ok(()) + } + /// Returns the registered fuel gauges. pub fn fuel_gauges(&self) -> &[&'hw Reg::FuelGauge] { self.registration.fuel_gauges() diff --git a/battery-service/src/registration.rs b/battery-service/src/registration.rs index 1a83136a5..6b9c6773a 100644 --- a/battery-service/src/registration.rs +++ b/battery-service/src/registration.rs @@ -1,27 +1,34 @@ //! Fuel gauge registration with the battery service. //! -//! The [`Registration`] trait abstracts over how a set of fuel gauges is provided to -//! the service, and [`ArrayRegistration`] is a simple array-backed -//! implementation. The OEM owns each fuel gauge (typically behind an -//! `embassy_sync` `Mutex`) and drives it directly via the -//! [`FuelGauge`](battery_service_interface::fuel_gauge::FuelGauge) trait methods; the -//! battery service only reads each fuel gauge's cached state to answer ACPI -//! queries. +//! [`Registration`] supplies fuel gauges to the service; [`ArrayRegistration`] +//! stores them in an array. The OEM owns each fuel gauge (typically behind an +//! `embassy_sync` `Mutex`) and drives it directly through the +//! [`FuelGauge`](battery_service_interface::fuel_gauge::FuelGauge) trait methods. +//! The battery service reads each fuel gauge's cached battery and external-power +//! state to answer ACPI queries. use battery_service_interface::DeviceId; use embedded_services::sync::Lockable; +use crate::PowerPolicyRouting; + /// Registration trait that abstracts over how fuel gauges are provided to the service. pub trait Registration<'hw> { /// The lockable fuel gauge type. Its inner type implements /// [`FuelGauge`](battery_service_interface::fuel_gauge::FuelGauge). type FuelGauge: Lockable + 'hw; + /// The service's required power-policy router. + type Routing: PowerPolicyRouting; + + /// The router used for every power-policy notification handled by the service. + fn routing(&self) -> &Self::Routing; + /// Returns a slice of the registered fuel gauges. /// - /// The position of a fuel gauge in this slice is its - /// [`DeviceId`] for ACPI queries (the - /// first registered fuel gauge is battery `0`, and so on). + /// A fuel gauge's position in this slice is its [`DeviceId`] for ACPI queries: + /// the first registered gauge is battery `0`, and so on. Power-source queries + /// use the same ID to read the gauge's cached external-power state. fn fuel_gauges(&self) -> &[&'hw Self::FuelGauge]; /// Look up a registered fuel gauge by its device ID. @@ -38,15 +45,27 @@ pub struct ArrayRegistration< 'hw, FG: Lockable + 'hw, const N: usize, + R: PowerPolicyRouting, > { /// The registered fuel gauges. pub fuel_gauges: [&'hw FG; N], + /// The required router, whose type determines which notifications the service accepts. + pub routing: R, } -impl<'hw, FG: Lockable + 'hw, const N: usize> Registration<'hw> - for ArrayRegistration<'hw, FG, N> +impl< + 'hw, + FG: Lockable + 'hw, + const N: usize, + R: PowerPolicyRouting, +> Registration<'hw> for ArrayRegistration<'hw, FG, N, R> { type FuelGauge = FG; + type Routing = R; + + fn routing(&self) -> &Self::Routing { + &self.routing + } fn fuel_gauges(&self) -> &[&'hw Self::FuelGauge] { &self.fuel_gauges diff --git a/battery-service/src/routing.rs b/battery-service/src/routing.rs new file mode 100644 index 000000000..7459b326b --- /dev/null +++ b/battery-service/src/routing.rs @@ -0,0 +1,64 @@ +//! Routing of power-policy notifications to fuel gauges. + +use battery_service_interface::{DeviceId, fuel_gauge::PsuState}; +use power_policy_interface::{capability::DisconnectReason, service::event::EventData as PowerPolicyEventData}; + +/// A routed update to a fuel gauge's external-power cache. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum PowerPolicyUpdate { + /// Replace the PSU state and refresh its unconstrained flag. + PsuState(PsuState), + /// Set or clear the unconstrained override, preserving the cached PSU state. + Unconstrained(bool), +} + +/// Maps power-policy events to fuel-gauge cache updates. +/// +/// Use full or application-tagged [`Self::Event`] values to route by source +/// identity without PSU locks. The service validates all IDs before updating +/// caches under gauge locks. +pub trait PowerPolicyRouting { + /// Notification type; [`Self::route`] uses its payload by default. + type Event: Clone + Into; + + /// Returns per-gauge cache updates. An empty iterator ignores the event. + /// + /// The default updates registered gauge `0`: consumer connections set + /// [`PsuState::ConnectedConsumer`]; consumer disconnects set [`PsuState::Detached`] + /// for [`DisconnectReason::Detached`], otherwise [`PsuState::Idle`]. + /// Unconstrained events set or clear the power override. Other events are ignored. + /// Override to choose gauges and their updates. + /// + /// Iteration must have no side effects; clones must yield identical updates in order. + /// Retries must reproduce all updates, even if cancellation left some applied. + /// Retry the same event before processing newer events. Stateful routers must not consume + /// pending updates or apply an event twice to their policy state on retry. + fn route(&self, event: &Self::Event) -> impl Iterator + Clone { + let update = match event.clone().into() { + PowerPolicyEventData::ConsumerConnected(capability) => { + Some(PowerPolicyUpdate::PsuState(PsuState::ConnectedConsumer(capability))) + } + PowerPolicyEventData::ConsumerDisconnected(disconnect) => { + Some(PowerPolicyUpdate::PsuState(match disconnect.reason { + Some(DisconnectReason::Detached) => PsuState::Detached, + _ => PsuState::Idle, + })) + } + PowerPolicyEventData::Unconstrained(state) => Some(PowerPolicyUpdate::Unconstrained(state.unconstrained)), + _ => None, + }; + update.map(|update| (DeviceId(0), update)).into_iter() + } +} + +/// Payload-only routing for single-battery controllers. +/// +/// Uses [`PowerPolicyRouting::route`] to update gauge `0` in delivery order from +/// every PSU. Assumes only one PSU is attached; simultaneous connections are not aggregated. +#[derive(Debug, Default, Clone, Copy)] +pub struct DefaultPowerPolicyRouting; + +impl PowerPolicyRouting for DefaultPowerPolicyRouting { + type Event = PowerPolicyEventData; +} diff --git a/examples/pico-de-gallo/Cargo.lock b/examples/pico-de-gallo/Cargo.lock index 3e3de61bf..86b4ce0ec 100644 --- a/examples/pico-de-gallo/Cargo.lock +++ b/examples/pico-de-gallo/Cargo.lock @@ -96,6 +96,7 @@ name = "battery-service" version = "0.1.0" dependencies = [ "battery-service-interface", + "embassy-futures", "embassy-time", "embedded-batteries-async", "embedded-services", @@ -109,6 +110,7 @@ version = "0.1.0" dependencies = [ "embedded-batteries-async", "log", + "power-policy-interface", ] [[package]] diff --git a/examples/pico-de-gallo/src/bin/battery.rs b/examples/pico-de-gallo/src/bin/battery.rs index 4f5763147..70f4a5a21 100644 --- a/examples/pico-de-gallo/src/bin/battery.rs +++ b/examples/pico-de-gallo/src/bin/battery.rs @@ -18,7 +18,7 @@ use battery_service as bs; use bq40z50_rx::{BQ40Z50Error, Bq40z50R5}; -use bs::FuelGauge as _; +use bs::{DefaultPowerPolicyRouting, FuelGauge as _}; use embassy_sync::mutex::Mutex; use embedded_batteries_async::smart_battery::{BatteryModeFields, SmartBattery}; use embedded_services::GlobalRawMutex; @@ -164,8 +164,9 @@ impl bs::FuelGauge for Battery { /// The fuel gauge, wrapped in a mutex so it can be shared between the OEM driving /// code and the battery service. type FuelGauge = Mutex; + /// The registration: a single fuel gauge, which becomes battery `0`. -type Reg<'hw> = bs::ArrayRegistration<'hw, FuelGauge, 1>; +type Reg<'hw> = bs::ArrayRegistration<'hw, FuelGauge, 1, DefaultPowerPolicyRouting>; async fn init_state_machine(fuel_gauge: &FuelGauge) -> Result<(), BatteryError> { let mut fg = fuel_gauge.lock().await; @@ -270,6 +271,7 @@ async fn main() { let battery_service = bs::Service::new(bs::ArrayRegistration { fuel_gauges: [&fuel_gauge], + routing: DefaultPowerPolicyRouting, }); run_app(battery_service).await; diff --git a/examples/std/Cargo.lock b/examples/std/Cargo.lock index 8c7a311ca..e243b3b2c 100644 --- a/examples/std/Cargo.lock +++ b/examples/std/Cargo.lock @@ -81,6 +81,7 @@ name = "battery-service" version = "0.1.0" dependencies = [ "battery-service-interface", + "embassy-futures", "embassy-time", "embedded-batteries-async", "embedded-services", @@ -94,6 +95,7 @@ version = "0.1.0" dependencies = [ "embedded-batteries-async", "log", + "power-policy-interface", ] [[package]] diff --git a/examples/std/src/bin/battery.rs b/examples/std/src/bin/battery.rs index dbdc6a9d1..e55a8996b 100644 --- a/examples/std/src/bin/battery.rs +++ b/examples/std/src/bin/battery.rs @@ -4,23 +4,29 @@ //! gauge (behind a `Mutex`) and drives it directly through the [`bs::FuelGauge`] //! trait methods, while the battery service holds the registration and answers //! ACPI queries by reading the fuel gauge's cached state. +//! The battery task handles power-policy notifications and periodic polling. //! //! The example can be run simply by typing `cargo run --bin battery` use battery_service as bs; -use bs::FuelGauge as _; use bs::mock::MockFuelGauge; +use bs::{DefaultPowerPolicyRouting, FuelGauge as _}; use embassy_executor::{Executor, Spawner}; -use embassy_sync::mutex::Mutex; -use embassy_time::{Duration, Timer}; +use embassy_sync::{ + channel::{Channel, DynamicReceiver}, + mutex::Mutex, +}; +use embassy_time::{Duration, Ticker, Timer}; use embedded_services::GlobalRawMutex; use static_cell::StaticCell; /// The fuel gauge, wrapped in a mutex so it can be shared between the OEM driving /// code and the battery service. type FuelGauge = Mutex; + /// The registration: a single fuel gauge, which becomes battery `0`. -type Reg = bs::ArrayRegistration<'static, FuelGauge, 1>; +type Reg = bs::ArrayRegistration<'static, FuelGauge, 1, DefaultPowerPolicyRouting>; +type BatteryEvents = bs::EventReceiver>; #[embassy_executor::task] async fn embassy_main(spawner: Spawner) { @@ -34,13 +40,23 @@ async fn embassy_main(spawner: Spawner) { let battery_service = bs::Service::new(bs::ArrayRegistration { fuel_gauges: [fuel_gauge], + routing: DefaultPowerPolicyRouting, }); - spawner.spawn(run_app(fuel_gauge, battery_service).expect("Failed to create run_app task")); + // A platform's power-policy notifier sends into this channel. + static POWER_POLICY_EVENTS: StaticCell> = StaticCell::new(); + let power_policy_events = POWER_POLICY_EVENTS.init(Channel::new()); + let events = bs::EventReceiver::new(power_policy_events.dyn_receiver()); + + spawner.spawn(run_app(fuel_gauge, battery_service, events).expect("Failed to create run_app task")); } #[embassy_executor::task] -pub async fn run_app(fuel_gauge: &'static FuelGauge, battery_service: bs::Service<'static, Reg>) { +async fn run_app( + fuel_gauge: &'static FuelGauge, + battery_service: bs::Service<'static, Reg>, + mut events: BatteryEvents, +) { // Initialize the fuel gauge by driving it directly. let mut retries = 5; while let Err(e) = bs::mock::init_state_machine(fuel_gauge).await { @@ -54,8 +70,18 @@ pub async fn run_app(fuel_gauge: &'static FuelGauge, battery_service: bs::Servic let mut failures: u32 = 0; let mut count: usize = 1; + let mut ticker = Ticker::every(Duration::from_secs(1)); loop { - Timer::after(Duration::from_secs(1)).await; + match events.wait_next_with_poll(ticker.next()).await { + bs::Event::Poll => {} + bs::Event::PowerPolicy(event) => { + if let Err(e) = battery_service.process_power_policy_event(&event).await { + embedded_services::error!("Battery power-policy routing error: {:?}", e); + } + continue; + } + } + if count.is_multiple_of(const { 60 * 60 * 60 }) && let Err(e) = fuel_gauge.lock().await.update_static_data().await { diff --git a/examples/std/src/bin/power_policy.rs b/examples/std/src/bin/power_policy.rs index 92146c036..36d1b8004 100644 --- a/examples/std/src/bin/power_policy.rs +++ b/examples/std/src/bin/power_policy.rs @@ -1,13 +1,18 @@ +use battery_service::{ + BatteryService, DefaultPowerPolicyRouting, DeviceId, Event as BatteryEvent, FuelGauge as _, PowerPolicyRouting, + PowerPolicyUpdate, +}; use embassy_executor::{Executor, Spawner}; use embassy_sync::{ blocking_mutex::raw::NoopRawMutex, channel::{self, Channel}, mutex::Mutex, }; -use embassy_time::{self as _, Timer}; +use embassy_time::{Duration, Ticker, Timer}; use embedded_batteries_async::charger::{MilliAmps, MilliVolts}; -use embedded_services::{GlobalRawMutex, event::NoopSender, named::Named}; +use embedded_services::{GlobalRawMutex, named::Named, sync::Lockable}; use log::*; +use power_policy_interface::service::event::EventData as PowerPolicyEventData; use power_policy_interface::{ capability::{ConsumerFlags, ConsumerPowerCapability, PowerCapability, ProviderPowerCapability}, charger, psu, @@ -21,8 +26,12 @@ use power_policy_service::{ }; use static_cell::StaticCell; -type ServiceNotifier = - power_policy_interface::service::event::NonBlockingSenderNotifier<'static, DeviceType, NoopSender>; +type ServiceEvent = power_policy_interface::service::event::Event<'static, DeviceType>; +type ServiceNotifier = power_policy_interface::service::event::SenderNotifier< + 'static, + DeviceType, + channel::DynamicSender<'static, ServiceEvent>, +>; type ServiceType = Mutex< GlobalRawMutex, power_policy_service::service::Service< @@ -126,7 +135,56 @@ impl Named for ExampleDevice<'_> { } } -type DeviceType = Mutex>; +struct DeviceType { + source_id: u8, + inner: Mutex>, +} + +impl DeviceType { + fn new(source_id: u8, device: ExampleDevice<'static>) -> Self { + Self { + source_id, + inner: Mutex::new(device), + } + } +} + +impl Lockable for DeviceType { + type Inner = ExampleDevice<'static>; + + fn try_lock(&self) -> Option> { + Lockable::try_lock(&self.inner) + } + + async fn lock(&self) -> impl core::ops::DerefMut { + self.inner.lock().await + } +} + +/// Routes consumer notifications one-to-one by PSU source ID. +/// +/// Uses the default consumer-state interpretation and immutable source IDs to +/// map PSU `0` to fuel gauge `0` and PSU `1` to fuel gauge `1`. +/// Consumer events include unconstrained flags; aggregate events have no source ID. +struct BatteryPowerRouting; + +impl PowerPolicyRouting for BatteryPowerRouting { + type Event = ServiceEvent; + + fn route(&self, event: &Self::Event) -> impl Iterator + Clone { + use power_policy_interface::service::event::Event; + + let data = PowerPolicyEventData::from(*event); + let update = DefaultPowerPolicyRouting.route(&data).next().map(|(_, update)| update); + match event { + Event::ConsumerConnected(psu, _) | Event::ConsumerDisconnected(psu, _) => { + update.map(|update| (DeviceId(psu.source_id), update)) + } + _ => None, + } + .into_iter() + } +} struct ExampleCharger<'a> { sender: channel::DynamicSender<'a, power_policy_interface::charger::event::EventData>, @@ -228,20 +286,20 @@ async fn run(spawner: Spawner) { StaticCell::new(); let device0_event_channel = DEVICE0_EVENT_CHANNEL.init(Channel::new()); static DEVICE0: StaticCell = StaticCell::new(); - let device0 = DEVICE0.init(Mutex::new(ExampleDevice::new( - "Device 0", - device0_event_channel.dyn_sender(), - ))); + let device0 = DEVICE0.init(DeviceType::new( + 0, + ExampleDevice::new("Device 0", device0_event_channel.dyn_sender()), + )); info!("Creating device 1"); static DEVICE1_EVENT_CHANNEL: StaticCell> = StaticCell::new(); let device1_event_channel = DEVICE1_EVENT_CHANNEL.init(Channel::new()); static DEVICE1: StaticCell = StaticCell::new(); - let device1 = DEVICE1.init(Mutex::new(ExampleDevice::new( - "Device 1", - device1_event_channel.dyn_sender(), - ))); + let device1 = DEVICE1.init(DeviceType::new( + 1, + ExampleDevice::new("Device 1", device1_event_channel.dyn_sender()), + )); info!("Creating charger 0"); static CHARGER0_EVENT_CHANNEL: StaticCell< @@ -251,9 +309,14 @@ async fn run(spawner: Spawner) { static CHARGER0: StaticCell = StaticCell::new(); let charger0 = CHARGER0.init(Mutex::new(ExampleCharger::new(charger0_event_channel.dyn_sender()))); + static SERVICE_EVENT_CHANNEL: StaticCell> = StaticCell::new(); + let service_event_channel = SERVICE_EVENT_CHANNEL.init(Channel::new()); + spawner.spawn(battery_task(service_event_channel.dyn_receiver()).expect("Failed to create battery task")); + + let service_notifier = ServiceNotifier::new(service_event_channel.dyn_sender()); let registration = ArrayRegistration { psus: [device0, device1], - service_notifiers: [NoopSender.into()], + service_notifiers: [service_notifier], chargers: [charger0], }; @@ -530,6 +593,56 @@ async fn run(spawner: Spawner) { Timer::after_millis(PER_CALL_DELAY_MS).await; } +/// Polls fuel gauges and routes power-policy notifications in the same task. +#[embassy_executor::task] +async fn battery_task(events: channel::DynamicReceiver<'static, ServiceEvent>) { + let mut events = battery_service::EventReceiver::<_, ServiceEvent>::new(events); + let first = Mutex::::new(battery_service::mock::MockFuelGauge::new()); + let second = Mutex::::new(battery_service::mock::MockFuelGauge::new_2s()); + let battery = battery_service::Service::new(battery_service::ArrayRegistration { + fuel_gauges: [&first, &second], + routing: BatteryPowerRouting, + }); + for &gauge in battery.fuel_gauges() { + battery_service::mock::init_state_machine(gauge) + .await + .expect("Failed to initialize fuel gauge"); + } + let mut ticker = Ticker::every(Duration::from_secs(1)); + loop { + let event = match events.wait_next_with_poll(ticker.next()).await { + BatteryEvent::Poll => { + for &gauge in battery.fuel_gauges() { + if let Err(e) = gauge.lock().await.update_dynamic_data().await { + error!("Fuel gauge polling error: {:?}", e); + } + } + continue; + } + BatteryEvent::PowerPolicy(event) => event, + }; + info!("battery event {:?}", PowerPolicyEventData::from(event)); + battery + .process_power_policy_event(&event) + .await + .expect("Failed to route power-policy event to fuel gauges"); + for id in [DeviceId(0), DeviceId(1)] { + let status = BatteryService::is_psu_in_use(&battery, id) + .await + .expect("Failed to read fuel-gauge power status"); + let information = BatteryService::power_source_information(&battery, id) + .await + .expect("Failed to read fuel-gauge power information"); + info!( + "Battery ACPI: gauge={}, external power online={}, available output={} mW (0xFFFFFFFF means unknown)", + id.0, + status.power_source == embedded_batteries_async::acpi::PowerSource::Online, + information.max_output_power, + ); + } + } +} + #[embassy_executor::task] async fn power_policy_task( psu_events: PsuEventReceivers< diff --git a/power-policy-interface/src/psu/mod.rs b/power-policy-interface/src/psu/mod.rs index 300e1ef41..2dddead92 100644 --- a/power-policy-interface/src/psu/mod.rs +++ b/power-policy-interface/src/psu/mod.rs @@ -47,7 +47,7 @@ pub enum StateKind { } /// Current state of the power device -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum PsuState { /// Device is attached, but is not currently providing or consuming power @@ -57,6 +57,7 @@ pub enum PsuState { /// Device is attached and is currently consuming power ConnectedConsumer(ConsumerPowerCapability), /// No device attached + #[default] Detached, } diff --git a/power-policy-interface/src/service/event.rs b/power-policy-interface/src/service/event.rs index 659a148a0..31199b712 100644 --- a/power-policy-interface/src/service/event.rs +++ b/power-policy-interface/src/service/event.rs @@ -1,7 +1,7 @@ use core::{future::ready, marker::PhantomData}; use embedded_services::{ - event::{NonBlockingSender, Sender}, + event::{NonBlockingSender, Receiver, Sender}, sync::Lockable, }; @@ -47,6 +47,35 @@ where } } +/// Receives power-policy notifications. +/// +/// Wraps any [`Receiver`] and forwards its events unchanged. +pub struct PowerPolicySubscriber> { + receiver: R, + // Track E without owning it or imposing its Send/Sync requirements. + _phantom: PhantomData E>, +} + +impl> PowerPolicySubscriber { + /// Wrap an event receiver. + pub fn new(receiver: R) -> Self { + Self { + receiver, + _phantom: PhantomData, + } + } +} + +impl> Receiver for PowerPolicySubscriber { + fn try_next(&mut self) -> Option { + self.receiver.try_next() + } + + fn wait_next(&mut self) -> impl Future { + self.receiver.wait_next() + } +} + /// Events broadcast from the service. #[derive(Debug, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] diff --git a/tcpm-service/src/service/event_receiver.rs b/tcpm-service/src/service/event_receiver.rs index b78a468c5..82d69cffa 100644 --- a/tcpm-service/src/service/event_receiver.rs +++ b/tcpm-service/src/service/event_receiver.rs @@ -3,20 +3,9 @@ use core::pin::pin; use crate::service::Event; use embassy_futures::select::{Either, select, select_slice}; use embedded_services::{event::Receiver, sync::Lockable}; -use power_policy_interface::service::event::EventData as PowerPolicyEventData; +use power_policy_interface::service::event::{EventData as PowerPolicyEventData, PowerPolicySubscriber}; use tcpm_interface::{port::pd::Pd, service::event::PortEvent}; -struct PowerPolicySubscriber> { - receiver: PowerReceiver, -} - -impl> PowerPolicySubscriber { - /// Wait for a power policy event - async fn wait_next(&mut self) -> PowerPolicyEventData { - self.receiver.wait_next().await - } -} - pub struct ArrayPortReceivers< 'port, const N: usize, @@ -61,7 +50,7 @@ pub struct ArrayEventReceiver< PowerReceiver: Receiver, > { /// Power policy event subscriber - power_policy_event_subscriber: PowerPolicySubscriber, + power_policy_event_subscriber: PowerPolicySubscriber, /// Port event receivers and corresponding ports port_receivers: ArrayPortReceivers<'a, N, Port, PortReceiver>, } @@ -82,9 +71,7 @@ impl< ) -> Self { Self { port_receivers: ArrayPortReceivers { ports, port_receivers }, - power_policy_event_subscriber: PowerPolicySubscriber { - receiver: power_policy_event_receiver, - }, + power_policy_event_subscriber: PowerPolicySubscriber::new(power_policy_event_receiver), } } From f9fe87a29eb7cfc2db609f712ef74e63f69a310b Mon Sep 17 00:00:00 2001 From: Matteo Tullo Date: Thu, 22 Oct 2026 13:08:56 -0700 Subject: [PATCH 2/5] test(battery): cover power-policy integration Exercise cached ACPI power reporting, custom routing, validation, backpressure, and cancellation. Cover subscriber forwarding and fair polling, and enable the mock dependencies needed by these tests. Assisted-by: GitHub Copilot:gpt-6-astra --- Cargo.lock | 2 + battery-service/Cargo.toml | 4 + battery-service/src/acpi.rs | 960 +++++++++++++++++++- battery-service/src/event_receiver.rs | 116 +++ battery-service/src/lib.rs | 2 +- battery-service/src/routing.rs | 90 ++ power-policy-interface/src/service/event.rs | 207 +++++ 7 files changed, 1378 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2a0783cac..043539aae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -119,11 +119,13 @@ dependencies = [ "battery-service-interface", "defmt 0.3.100", "embassy-futures", + "embassy-sync", "embassy-time", "embedded-batteries-async", "embedded-services", "log", "power-policy-interface", + "power-policy-interface-test-mocks", ] [[package]] diff --git a/battery-service/Cargo.toml b/battery-service/Cargo.toml index 4e34b8b00..f677835f6 100644 --- a/battery-service/Cargo.toml +++ b/battery-service/Cargo.toml @@ -23,6 +23,10 @@ embedded-services.workspace = true log = { workspace = true, optional = true } power-policy-interface.workspace = true +[dev-dependencies] +embassy-sync.workspace = true +power-policy-interface-test-mocks.workspace = true + [features] default = [] defmt = [ diff --git a/battery-service/src/acpi.rs b/battery-service/src/acpi.rs index 374fb9a8f..983ebf48b 100644 --- a/battery-service/src/acpi.rs +++ b/battery-service/src/acpi.rs @@ -391,15 +391,971 @@ impl<'hw, Reg: crate::registration::Registration<'hw>> crate::Service<'hw, Reg> #[cfg(test)] mod tests { - #![allow(clippy::expect_used)] + #![allow(clippy::expect_used, clippy::unwrap_used)] + + use core::{ + future::Future, + pin::pin, + task::{Context, Waker}, + }; use embedded_batteries_async::smart_battery::CapacityModeValue; - use super::{compute_bix, compute_bpc, compute_bst}; + use embassy_futures::block_on; + use embassy_sync::{blocking_mutex::raw::NoopRawMutex, mutex::Mutex}; + use power_policy_interface::{ + capability::{ConsumerFlags, ConsumerPowerCapability, DisconnectFlags, DisconnectReason, PowerCapability}, + service::{UnconstrainedState, event::EventData as PowerPolicyEventData}, + }; + + use super::*; + use crate::{ + ArrayRegistration, BatteryService, DefaultPowerPolicyRouting, PowerPolicyRouting, PowerPolicyUpdate, + Registration, Service, State, mock::MockFuelGauge, + }; use battery_service_interface::fuel_gauge::{ DynamicBatteryData, DynamicBatteryMsgs, StaticBatteryData, StaticBatteryMsgs, }; + struct RouteTo([DeviceId; N]); + + struct InheritedRouting; + + impl PowerPolicyRouting for InheritedRouting { + type Event = SourceEvent; + } + + impl PowerPolicyRouting for RouteTo { + type Event = PowerPolicyEventData; + + fn route(&self, event: &Self::Event) -> impl Iterator + Clone { + DefaultPowerPolicyRouting + .route(event) + .flat_map(move |(_, state)| self.0.into_iter().map(move |id| (id, state))) + } + } + + struct UpdateRouting([(DeviceId, PsuState); N]); + + impl PowerPolicyRouting for UpdateRouting { + type Event = PowerPolicyEventData; + + fn route(&self, _event: &Self::Event) -> impl Iterator + Clone { + self.0 + .into_iter() + .map(|(id, state)| (id, PowerPolicyUpdate::PsuState(state))) + } + } + + #[derive(Clone)] + struct SourceEvent { + source: DeviceId, + data: PowerPolicyEventData, + } + + impl From for PowerPolicyEventData { + fn from(event: SourceEvent) -> Self { + event.data + } + } + + struct SourceRouting; + + impl PowerPolicyRouting for SourceRouting { + type Event = SourceEvent; + + fn route(&self, event: &Self::Event) -> impl Iterator + Clone { + DefaultPowerPolicyRouting + .route(&event.data) + .filter_map(move |(_, state)| match event.source { + DeviceId(0) | DeviceId(1) => Some((event.source, state)), + _ => None, + }) + } + } + + async fn assert_power_source<'hw, Reg: Registration<'hw>>( + service: &Service<'hw, Reg>, + id: DeviceId, + power_mw: Option, + ) { + let status = BatteryService::is_psu_in_use(service, id).await.unwrap(); + assert!( + status.power_source + == if power_mw.is_some() { + PowerSource::Online + } else { + PowerSource::Offline + } + ); + let information = BatteryService::power_source_information(service, id).await.unwrap(); + assert_eq!(information.max_output_power, power_mw.unwrap_or(u32::MAX)); + assert_eq!(information.max_input_power, u32::MAX); + assert!(information.power_source_state.is_empty()); + assert_eq!(information.model_number, [0; STD_PIF_MODEL_SIZE]); + assert_eq!(information.serial_number, [0; STD_PIF_SERIAL_SIZE]); + assert_eq!(information.oem_info, [0; STD_PIF_OEM_SIZE]); + let mut gauge = service.get_fuel_gauge(id).unwrap().lock().await; + assert!(service.is_psu_in_use(&mut *gauge).unwrap().power_source == status.power_source); + assert!(service.power_source_information(&mut *gauge).unwrap() == information); + } + + #[test] + fn shared_references_forward_battery_queries() { + async fn query(service: impl BatteryService) { + for (id, voltage) in [(DeviceId(0), 11_100), (DeviceId(1), 7400)] { + assert_eq!(service.battery_info(id).await.unwrap().design_voltage, voltage); + } + assert_eq!( + service.battery_info(DeviceId(2)).await.err(), + Some(BatteryError::UnknownDeviceId) + ); + assert!(service.is_psu_in_use(DeviceId(0)).await.unwrap().power_source == PowerSource::Offline); + assert!(service.is_psu_in_use(DeviceId(1)).await.unwrap().power_source == PowerSource::Online); + assert_eq!( + service + .power_source_information(DeviceId(1)) + .await + .unwrap() + .max_output_power, + 65_000 + ); + } + + block_on(async { + let first = Mutex::::new(MockFuelGauge::new()); + let second = Mutex::::new(MockFuelGauge::new_2s()); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&first, &second], + routing: SourceRouting, + }); + service + .process_power_policy_event(&SourceEvent { + source: DeviceId(1), + data: PowerPolicyEventData::ConsumerConnected( + PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(), + ), + }) + .await + .unwrap(); + + query(&service).await; + query(&&service).await; + }); + } + + #[test] + fn default_routing_updates_only_gauge_zero_for_all_sources() { + block_on(async { + let first = Mutex::::new(MockFuelGauge::new()); + let second = Mutex::::new(MockFuelGauge::new()); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&first, &second], + routing: InheritedRouting, + }); + + for source in 0..=u8::MAX { + let capability = PowerCapability { + voltage_mv: 5000, + current_ma: 1000 + u16::from(source), + }; + service + .process_power_policy_event(&SourceEvent { + source: DeviceId(source), + data: PowerPolicyEventData::ConsumerConnected(capability.into()), + }) + .await + .unwrap(); + assert_power_source(&service, DeviceId(0), Some(capability.max_power_mw())).await; + assert_power_source(&service, DeviceId(1), None).await; + + service + .process_power_policy_event(&SourceEvent { + source: DeviceId(source), + data: PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags::default()), + }) + .await + .unwrap(); + assert_power_source(&service, DeviceId(0), None).await; + assert_power_source(&service, DeviceId(1), None).await; + } + }); + } + + #[test] + fn routed_events_drive_cached_psu_queries() { + block_on(async { + let gauge = Mutex::::new(MockFuelGauge::new()); + let unrelated = Mutex::::new(MockFuelGauge::new()); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&gauge, &unrelated], + routing: DefaultPowerPolicyRouting, + }); + assert!(!gauge.lock().await.state().is_present()); + assert_eq!(gauge.lock().await.state().psu_state(), PsuState::Detached); + assert_power_source(&service, DeviceId(0), None).await; + + for (capability, expected_mw) in [ + ( + PowerCapability { + voltage_mv: 5000, + current_ma: 1500, + }, + 7500, + ), + ( + PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + }, + 65_000, + ), + ( + PowerCapability { + voltage_mv: u16::MAX, + current_ma: u16::MAX, + }, + 4_294_836, + ), + ( + PowerCapability { + voltage_mv: 5000, + current_ma: 0, + }, + 0, + ), + ] { + service + .process_power_policy_event(&PowerPolicyEventData::ConsumerConnected(capability.into())) + .await + .unwrap(); + assert_eq!( + gauge.lock().await.state().psu_state(), + PsuState::ConnectedConsumer(capability.into()) + ); + assert_power_source(&service, DeviceId(0), Some(expected_mw)).await; + assert_power_source(&service, DeviceId(1), None).await; + } + + for reason in [ + None, + Some(DisconnectReason::Detached), + Some(DisconnectReason::Switching), + Some(DisconnectReason::AutoRenegotiation), + Some(DisconnectReason::ManualRenegotiation), + Some(DisconnectReason::RoleSwap), + Some(DisconnectReason::Reset), + ] { + service + .process_power_policy_event(&PowerPolicyEventData::ConsumerConnected( + PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(), + )) + .await + .unwrap(); + let event = PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags { reason }); + let expected_state = if reason == Some(DisconnectReason::Detached) { + PsuState::Detached + } else { + PsuState::Idle + }; + for _ in 0..2 { + service.process_power_policy_event(&event).await.unwrap(); + assert_eq!(gauge.lock().await.state().psu_state(), expected_state); + assert_power_source(&service, DeviceId(0), None).await; + assert_power_source(&service, DeviceId(1), None).await; + assert_eq!(unrelated.lock().await.state().psu_state(), PsuState::Detached); + } + } + }); + } + + #[test] + fn unconstrained_notifications_override_and_restore_cached_input() { + block_on(async { + let capability = PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + }; + for (psu_state, restored_power) in [ + (PsuState::Detached, None), + (PsuState::Idle, None), + (PsuState::ConnectedProvider(capability.into()), None), + (PsuState::ConnectedConsumer(capability.into()), Some(65_000)), + ] { + let gauge = Mutex::::new(MockFuelGauge::new()); + let unrelated = Mutex::::new(MockFuelGauge::new()); + gauge.lock().await.state_mut().set_psu_state(psu_state); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&gauge, &unrelated], + routing: DefaultPowerPolicyRouting, + }); + assert_power_source(&service, DeviceId(0), restored_power).await; + + for notification in [ + UnconstrainedState::new(true, 1), + UnconstrainedState::new(true, 2), + UnconstrainedState::new(false, 2), + UnconstrainedState::new(false, 0), + UnconstrainedState::new(true, 0), + UnconstrainedState::new(false, 0), + ] { + service + .process_power_policy_event(&PowerPolicyEventData::Unconstrained(notification)) + .await + .unwrap(); + assert_eq!(gauge.lock().await.state().psu_state(), psu_state); + assert_eq!( + gauge.lock().await.state().is_psu_unconstrained(), + notification.unconstrained + ); + assert_power_source( + &service, + DeviceId(0), + if notification.unconstrained { + Some(0xffff_fffe) + } else { + restored_power + }, + ) + .await; + assert_power_source(&service, DeviceId(1), None).await; + } + } + }); + } + + #[test] + fn consumer_notifications_refresh_unconstrained_power() { + block_on(async { + let gauge = Mutex::::new(MockFuelGauge::new()); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&gauge], + routing: DefaultPowerPolicyRouting, + }); + let constrained: ConsumerPowerCapability = PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(); + let unconstrained = ConsumerPowerCapability { + flags: ConsumerFlags { + unconstrained_power: true, + ..Default::default() + }, + ..constrained + }; + for (event, expected_power) in [ + ( + PowerPolicyEventData::Unconstrained(UnconstrainedState::new(true, 1)), + Some(0xffff_fffe), + ), + (PowerPolicyEventData::ConsumerConnected(constrained), Some(65_000)), + ( + PowerPolicyEventData::ConsumerConnected(unconstrained), + Some(0xffff_fffe), + ), + ( + PowerPolicyEventData::Unconstrained(UnconstrainedState::new(false, 1)), + Some(65_000), + ), + ( + PowerPolicyEventData::ConsumerConnected(unconstrained), + Some(0xffff_fffe), + ), + ( + PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags::default()), + None, + ), + ( + PowerPolicyEventData::Unconstrained(UnconstrainedState::new(true, 1)), + Some(0xffff_fffe), + ), + ( + PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags { + reason: Some(DisconnectReason::Detached), + }), + None, + ), + ] { + service.process_power_policy_event(&event).await.unwrap(); + assert_power_source(&service, DeviceId(0), expected_power).await; + assert_eq!( + gauge.lock().await.state().is_psu_unconstrained(), + expected_power == Some(0xffff_fffe) + ); + } + }); + } + + #[test] + fn attached_psu_without_input_power_reports_offline_and_unknown_output() { + block_on(async { + let gauge = Mutex::::new(MockFuelGauge::new()); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&gauge], + routing: DefaultPowerPolicyRouting, + }); + for state in [ + PsuState::Idle, + PsuState::ConnectedProvider( + PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(), + ), + ] { + gauge.lock().await.state_mut().set_psu_state(state); + assert_power_source(&service, DeviceId(0), None).await; + assert_eq!(gauge.lock().await.state().psu_state(), state); + } + }); + } + + #[test] + fn psu_queries_are_independent_per_fuel_gauge() { + block_on(async { + let first = Mutex::::new(MockFuelGauge::new()); + let second = Mutex::::new(MockFuelGauge::new_2s()); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&first, &second], + routing: SourceRouting, + }); + + for (source, capability) in [ + ( + DeviceId(0), + PowerCapability { + voltage_mv: 5000, + current_ma: 1500, + }, + ), + ( + DeviceId(1), + PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + }, + ), + ] { + service + .process_power_policy_event(&SourceEvent { + source, + data: PowerPolicyEventData::ConsumerConnected(capability.into()), + }) + .await + .unwrap(); + } + assert_power_source(&service, DeviceId(0), Some(7500)).await; + assert_power_source(&service, DeviceId(1), Some(65_000)).await; + + for unconstrained in [true, false] { + service + .process_power_policy_event(&SourceEvent { + source: DeviceId(0), + data: PowerPolicyEventData::Unconstrained(UnconstrainedState::new(unconstrained, 2)), + }) + .await + .unwrap(); + assert_power_source( + &service, + DeviceId(0), + Some(if unconstrained { 0xffff_fffe } else { 7500 }), + ) + .await; + assert_power_source(&service, DeviceId(1), Some(65_000)).await; + } + + service + .process_power_policy_event(&SourceEvent { + source: DeviceId(0), + data: PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags::default()), + }) + .await + .unwrap(); + assert_power_source(&service, DeviceId(0), None).await; + assert_power_source(&service, DeviceId(1), Some(65_000)).await; + service + .process_power_policy_event(&SourceEvent { + source: DeviceId(1), + data: PowerPolicyEventData::ConsumerConnected( + PowerCapability { + voltage_mv: 20_000, + current_ma: 5000, + } + .into(), + ), + }) + .await + .unwrap(); + assert_power_source(&service, DeviceId(0), None).await; + assert_power_source(&service, DeviceId(1), Some(100_000)).await; + + for (id, voltage) in [(DeviceId(0), 11_100), (DeviceId(1), 7400)] { + assert_eq!( + BatteryService::battery_info(&service, id).await.unwrap().design_voltage, + voltage + ); + } + }); + } + + #[test] + fn routing_can_fan_out_a_shared_source() { + block_on(async { + let first = Mutex::::new(MockFuelGauge::new()); + let second = Mutex::::new(MockFuelGauge::new()); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&first, &second], + routing: RouteTo([DeviceId(0), DeviceId(1)]), + }); + + service + .process_power_policy_event(&PowerPolicyEventData::ConsumerConnected( + PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(), + )) + .await + .unwrap(); + for id in [DeviceId(0), DeviceId(1)] { + assert_power_source(&service, id, Some(65_000)).await; + } + for (unconstrained, expected_power) in [(true, 0xffff_fffe), (false, 65_000)] { + service + .process_power_policy_event(&PowerPolicyEventData::Unconstrained(UnconstrainedState::new( + unconstrained, + 1, + ))) + .await + .unwrap(); + for id in [DeviceId(0), DeviceId(1)] { + assert_power_source(&service, id, Some(expected_power)).await; + } + } + service + .process_power_policy_event(&PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags::default())) + .await + .unwrap(); + for id in [DeviceId(0), DeviceId(1)] { + assert_power_source(&service, id, None).await; + } + }); + } + + #[test] + fn custom_routing_controls_event_filtering_and_each_gauge_state() { + block_on(async { + let first = Mutex::::new(MockFuelGauge::new()); + let second = Mutex::::new(MockFuelGauge::new()); + let updates = [ + (DeviceId(0), PsuState::Idle), + ( + DeviceId(1), + PsuState::ConnectedConsumer( + PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(), + ), + ), + ]; + let service = Service::new(ArrayRegistration { + fuel_gauges: [&first, &second], + routing: UpdateRouting(updates), + }); + for event in [ + PowerPolicyEventData::ConsumerConnected( + PowerCapability { + voltage_mv: 5000, + current_ma: 1500, + } + .into(), + ), + PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags { + reason: Some(DisconnectReason::Detached), + }), + PowerPolicyEventData::ProviderDisconnected, + PowerPolicyEventData::Unconstrained(UnconstrainedState::new(true, 2)), + ] { + for gauge in [&first, &second] { + gauge.lock().await.state_mut().set_psu_state(PsuState::Detached); + } + service.process_power_policy_event(&event).await.unwrap(); + for (id, expected) in updates { + assert_eq!( + service.get_fuel_gauge(id).unwrap().lock().await.state().psu_state(), + expected + ); + } + assert_power_source(&service, DeviceId(0), None).await; + assert_power_source(&service, DeviceId(1), Some(65_000)).await; + } + }); + } + + #[test] + fn unrouted_gauges_remain_offline() { + block_on(async { + let first = Mutex::::new(MockFuelGauge::new()); + let second = Mutex::::new(MockFuelGauge::new()); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&first, &second], + routing: SourceRouting, + }); + assert_power_source(&service, DeviceId(0), None).await; + assert_power_source(&service, DeviceId(1), None).await; + service + .process_power_policy_event(&SourceEvent { + source: DeviceId(1), + data: PowerPolicyEventData::ConsumerConnected( + PowerCapability { + voltage_mv: 5000, + current_ma: 1500, + } + .into(), + ), + }) + .await + .unwrap(); + assert_power_source(&service, DeviceId(0), None).await; + assert_power_source(&service, DeviceId(1), Some(7500)).await; + + service + .process_power_policy_event(&SourceEvent { + source: DeviceId(2), + data: PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags::default()), + }) + .await + .unwrap(); + assert_power_source(&service, DeviceId(0), None).await; + assert_power_source(&service, DeviceId(1), Some(7500)).await; + }); + } + + #[test] + fn psu_queries_reject_unregistered_gauge_ids() { + block_on(async { + type Gauge = Mutex; + let empty = Service::new(ArrayRegistration:: { + fuel_gauges: [], + routing: RouteTo([]), + }); + for id in 0..=u8::MAX { + assert!(matches!( + BatteryService::is_psu_in_use(&empty, DeviceId(id)).await, + Err(BatteryError::UnknownDeviceId) + )); + assert!(matches!( + BatteryService::power_source_information(&empty, DeviceId(id)).await, + Err(BatteryError::UnknownDeviceId) + )); + } + let first = Mutex::::new(MockFuelGauge::new()); + let second = Mutex::::new(MockFuelGauge::new()); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&first, &second], + routing: RouteTo([]), + }); + for id in 2..=u8::MAX { + assert!(matches!( + BatteryService::is_psu_in_use(&service, DeviceId(id)).await, + Err(BatteryError::UnknownDeviceId) + )); + assert!(matches!( + BatteryService::power_source_information(&service, DeviceId(id)).await, + Err(BatteryError::UnknownDeviceId) + )); + } + }); + } + + #[test] + fn cancelled_fanout_can_be_retried_without_losing_the_event() { + block_on(async { + let first = Mutex::::new(MockFuelGauge::new()); + let second = Mutex::::new(MockFuelGauge::new()); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&first, &second], + routing: RouteTo([DeviceId(0), DeviceId(1)]), + }); + for (event, expected_power) in [ + ( + PowerPolicyEventData::ConsumerConnected( + PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(), + ), + 65_000, + ), + ( + PowerPolicyEventData::Unconstrained(UnconstrainedState::new(true, 1)), + 0xffff_fffe, + ), + ( + PowerPolicyEventData::Unconstrained(UnconstrainedState::new(false, 0)), + 65_000, + ), + ] { + let second_guard = second.lock().await; + { + let mut update = pin!(service.process_power_policy_event(&event)); + assert!( + update + .as_mut() + .poll(&mut Context::from_waker(Waker::noop())) + .is_pending() + ); + assert_power_source(&service, DeviceId(0), Some(expected_power)).await; + } + drop(second_guard); + service.process_power_policy_event(&event).await.unwrap(); + for id in [DeviceId(0), DeviceId(1)] { + assert_power_source(&service, id, Some(expected_power)).await; + } + } + }); + } + + #[test] + fn backpressured_notifications_and_cached_queries_do_not_lock_the_psu() { + use crate::{Event, EventReceiver}; + use core::future::{pending, ready}; + use embassy_sync::channel::Channel; + use embedded_services::event::{MapSender, NoopSender, Receiver}; + use power_policy_interface::{ + psu::event::NonBlockingSenderNotifier as PsuNotifier, + service::{event::SenderNotifier, notification::Notifier}, + }; + use power_policy_interface_test_mocks::psu::Mock as MockPsu; + + block_on(async { + type TestPsu = Mutex>>; + let psu = TestPsu::new(MockPsu::new("PSU", PsuNotifier(NoopSender))); + let gauge = Mutex::::new(MockFuelGauge::new()); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&gauge], + routing: RouteTo([DeviceId(0)]), + }); + let events = Channel::::new(); + let mut event_receiver = EventReceiver::new(events.dyn_receiver()); + let mut notifier = SenderNotifier::new(MapSender::new(events.dyn_sender(), PowerPolicyEventData::from)); + let capability = PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(); + + let mut psu_guard = psu.lock().await; + psu_guard.state.attach().unwrap(); + psu_guard.state.connect_consumer(capability).unwrap(); + notifier.notify_consumer_connected(&psu, capability).await.unwrap(); + psu_guard.state.disconnect(false).unwrap(); + + let mut send = pin!(notifier.notify_consumer_disconnected(&psu, DisconnectFlags::default())); + assert!(send.as_mut().poll(&mut Context::from_waker(Waker::noop())).is_pending()); + assert_eq!(event_receiver.wait_next_with_poll(ready(())).await, Event::Poll); + assert!(send.as_mut().poll(&mut Context::from_waker(Waker::noop())).is_pending()); + let connected = event_receiver.wait_next_with_poll(pending()).await; + assert_eq!( + connected, + Event::PowerPolicy(PowerPolicyEventData::ConsumerConnected(capability)) + ); + if let Event::PowerPolicy(event) = connected { + service.process_power_policy_event(&event).await.unwrap(); + } + assert_eq!(psu_guard.state.psu_state, PsuState::Idle); + assert_eq!( + gauge.lock().await.state().psu_state(), + PsuState::ConnectedConsumer(capability) + ); + assert_power_source(&service, DeviceId(0), Some(65_000)).await; + send.await.unwrap(); + let disconnected = event_receiver.wait_next_with_poll(pending()).await; + assert_eq!( + disconnected, + Event::PowerPolicy(PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags::default())) + ); + if let Event::PowerPolicy(event) = disconnected { + service.process_power_policy_event(&event).await.unwrap(); + } + assert_eq!(gauge.lock().await.state().psu_state(), PsuState::Idle); + assert_power_source(&service, DeviceId(0), None).await; + assert!(psu.try_lock().is_err()); + assert!(event_receiver.try_next().is_none()); + }); + } + + #[test] + fn invalid_routes_are_rejected_before_any_cache_is_changed() { + block_on(async { + let gauge = Mutex::::new(MockFuelGauge::new()); + gauge + .lock() + .await + .state_mut() + .set_psu_state(PsuState::ConnectedConsumer( + PowerCapability { + voltage_mv: 5000, + current_ma: 1500, + } + .into(), + )); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&gauge], + routing: UpdateRouting([(DeviceId(0), PsuState::Idle), (DeviceId(1), PsuState::Detached)]), + }); + for event in [ + PowerPolicyEventData::ConsumerConnected( + PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(), + ), + PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags::default()), + PowerPolicyEventData::ProviderDisconnected, + ] { + assert!(matches!( + service.process_power_policy_event(&event).await, + Err(BatteryError::UnknownDeviceId) + )); + assert_power_source(&service, DeviceId(0), Some(7500)).await; + } + }); + } + + #[test] + fn unconstrained_routes_validate_all_ids_before_updating_caches() { + block_on(async { + let gauge = Mutex::::new(MockFuelGauge::new()); + gauge + .lock() + .await + .state_mut() + .set_psu_state(PsuState::ConnectedConsumer( + PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(), + )); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&gauge], + routing: RouteTo([DeviceId(0), DeviceId(1)]), + }); + for unconstrained in [true, false] { + gauge.lock().await.state_mut().set_psu_unconstrained(!unconstrained); + assert!(matches!( + service + .process_power_policy_event(&PowerPolicyEventData::Unconstrained(UnconstrainedState::new( + unconstrained, + 1, + ))) + .await, + Err(BatteryError::UnknownDeviceId) + )); + assert_power_source( + &service, + DeviceId(0), + Some(if unconstrained { 65_000 } else { 0xffff_fffe }), + ) + .await; + } + }); + } + + #[test] + fn provider_events_do_not_change_cached_input() { + block_on(async { + let capability = ConsumerPowerCapability { + capability: PowerCapability { + voltage_mv: 5000, + current_ma: 1500, + }, + flags: ConsumerFlags { + unconstrained_power: true, + psu_type: None, + }, + }; + for state in [ + PsuState::Detached, + PsuState::Idle, + PsuState::ConnectedConsumer(capability), + ] { + let gauge = Mutex::::new(MockFuelGauge::new()); + gauge.lock().await.state_mut().set_psu_state(state); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&gauge], + routing: RouteTo([DeviceId(0)]), + }); + for event in [ + PowerPolicyEventData::ProviderConnected( + PowerCapability { + voltage_mv: 20_000, + current_ma: 5000, + } + .into(), + ), + PowerPolicyEventData::ProviderDisconnected, + ] { + service.process_power_policy_event(&event).await.unwrap(); + assert_eq!(gauge.lock().await.state().psu_state(), state); + assert_power_source( + &service, + DeviceId(0), + matches!(state, PsuState::ConnectedConsumer(_)).then_some(0xffff_fffe), + ) + .await; + } + } + }); + } + + #[test] + fn cached_power_is_independent_of_fuel_gauge_state_transitions() { + let capability = PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + }; + for psu_state in [ + PsuState::Detached, + PsuState::Idle, + PsuState::ConnectedConsumer(capability.into()), + PsuState::ConnectedProvider(capability.into()), + ] { + let mut state: State = State::default(); + state.set_psu_state(psu_state); + assert!(!state.is_psu_unconstrained()); + state.set_psu_unconstrained(true); + assert!(!state.is_present()); + state.on_initialized(); + state.on_static_data(|_| {}); + state.on_dynamic_data(|_| {}); + state.on_timeout(); + state.on_recovered(); + assert_eq!(state.psu_state(), psu_state); + assert!(state.is_psu_unconstrained()); + state.set_psu_state(PsuState::Detached); + assert_eq!(state.psu_state(), PsuState::Detached); + assert!(!state.is_psu_unconstrained()); + assert!(state.is_operational()); + } + } + /// An OEM dynamic data type that embeds the standard messages and extends /// them with extra fields. struct OemDynamic { diff --git a/battery-service/src/event_receiver.rs b/battery-service/src/event_receiver.rs index 1e438797a..4a6cf7f88 100644 --- a/battery-service/src/event_receiver.rs +++ b/battery-service/src/event_receiver.rs @@ -77,3 +77,119 @@ impl, E> Receiver for EventReceiver::new(); + let mut receiver = EventReceiver::new(channel.receiver()); + assert_eq!(block_on(receiver.wait_next_with_poll(ready(()))), Event::Poll); + assert_eq!(receiver.try_next(), None); + } + + #[test] + fn ready_poll_preserves_queued_notifications_in_order() { + let channel = Channel::::new(); + let mut receiver = EventReceiver::new(channel.receiver()); + let connected = PowerPolicyEventData::ConsumerConnected( + PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(), + ); + let disconnected = PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags::default()); + assert!(channel.try_send(connected.clone()).is_ok()); + assert!(channel.try_send(disconnected.clone()).is_ok()); + + assert_eq!(block_on(receiver.wait_next_with_poll(ready(()))), Event::Poll); + assert_eq!( + block_on(receiver.wait_next_with_poll(pending())), + Event::PowerPolicy(connected) + ); + assert_eq!( + block_on(receiver.wait_next_with_poll(pending())), + Event::PowerPolicy(disconnected) + ); + assert_eq!(receiver.try_next(), None); + } + + #[test] + fn overdue_poll_does_not_starve_queued_notifications() { + let channel = Channel::::new(); + let mut receiver = EventReceiver::new(channel.receiver()); + let events = [ + PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags::default()), + PowerPolicyEventData::ProviderDisconnected, + ]; + for event in &events { + assert!(channel.try_send(event.clone()).is_ok()); + } + + for event in events { + assert_eq!(block_on(receiver.wait_next_with_poll(ready(()))), Event::Poll); + assert_eq!( + block_on(receiver.wait_next_with_poll(ready(()))), + Event::PowerPolicy(event) + ); + } + assert_eq!(receiver.try_next(), None); + } + + #[test] + fn combined_wait_preserves_notifications_after_cancellation() { + let channel = Channel::::new(); + let mut receiver = EventReceiver::new(channel.receiver()); + { + let mut wait = pin!(receiver.wait_next_with_poll(pending())); + assert!(wait.as_mut().poll(&mut Context::from_waker(Waker::noop())).is_pending()); + } + + let disconnected = PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags::default()); + assert!(channel.try_send(disconnected.clone()).is_ok()); + assert_eq!( + block_on(receiver.wait_next_with_poll(pending())), + Event::PowerPolicy(disconnected) + ); + assert_eq!(block_on(receiver.wait_next_with_poll(ready(()))), Event::Poll); + assert_eq!(receiver.try_next(), None); + } + + #[test] + fn receiver_preserves_events_after_cancelled_wait() { + let channel = Channel::::new(); + let mut receiver = EventReceiver::new(channel.receiver()); + { + let mut wait = pin!(receiver.wait_next()); + assert!(wait.as_mut().poll(&mut Context::from_waker(Waker::noop())).is_pending()); + } + + let connected = PowerPolicyEventData::ConsumerConnected( + PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(), + ); + assert!(channel.try_send(connected.clone()).is_ok()); + assert_eq!(block_on(receiver.wait_next()), connected); + + let disconnected = PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags::default()); + assert!(channel.try_send(disconnected.clone()).is_ok()); + assert_eq!(receiver.try_next(), Some(disconnected)); + assert_eq!(receiver.try_next(), None); + } +} diff --git a/battery-service/src/lib.rs b/battery-service/src/lib.rs index 68751a080..a1c7c8d58 100644 --- a/battery-service/src/lib.rs +++ b/battery-service/src/lib.rs @@ -10,7 +10,7 @@ use embedded_services::{info, trace}; mod acpi; pub mod event_receiver; -#[cfg(feature = "mock")] +#[cfg(any(feature = "mock", test))] pub mod mock; pub mod registration; pub mod routing; diff --git a/battery-service/src/routing.rs b/battery-service/src/routing.rs index 7459b326b..f17d9f6c9 100644 --- a/battery-service/src/routing.rs +++ b/battery-service/src/routing.rs @@ -62,3 +62,93 @@ pub struct DefaultPowerPolicyRouting; impl PowerPolicyRouting for DefaultPowerPolicyRouting { type Event = PowerPolicyEventData; } + +#[cfg(test)] +mod tests { + use power_policy_interface::{ + capability::{DisconnectFlags, PowerCapability}, + service::UnconstrainedState, + }; + + use super::*; + + #[test] + fn default_routing_connects_only_gauge_zero() { + let routing = DefaultPowerPolicyRouting; + let capability = PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(); + let event = PowerPolicyEventData::ConsumerConnected(capability); + let expected = ( + DeviceId(0), + PowerPolicyUpdate::PsuState(PsuState::ConnectedConsumer(capability)), + ); + let mut updates = routing.route(&event); + let cloned = updates.clone(); + assert_eq!(updates.next(), Some(expected)); + assert_eq!(updates.next(), None); + assert!(cloned.eq([expected])); + } + + #[test] + fn default_routing_distinguishes_detachment_from_other_disconnects() { + let routing = DefaultPowerPolicyRouting; + for reason in [ + None, + Some(DisconnectReason::Detached), + Some(DisconnectReason::Switching), + Some(DisconnectReason::AutoRenegotiation), + Some(DisconnectReason::ManualRenegotiation), + Some(DisconnectReason::RoleSwap), + Some(DisconnectReason::Reset), + ] { + let event = PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags { reason }); + let expected = if reason == Some(DisconnectReason::Detached) { + PsuState::Detached + } else { + PsuState::Idle + }; + assert!( + routing + .route(&event) + .eq([(DeviceId(0), PowerPolicyUpdate::PsuState(expected))]) + ); + } + } + + #[test] + fn default_routing_ignores_provider_events() { + let routing = DefaultPowerPolicyRouting; + for event in [ + PowerPolicyEventData::ProviderConnected( + PowerCapability { + voltage_mv: 5000, + current_ma: 1500, + } + .into(), + ), + PowerPolicyEventData::ProviderDisconnected, + ] { + assert!(routing.route(&event).next().is_none()); + } + } + + #[test] + fn default_routing_updates_unconstrained_override() { + let routing = DefaultPowerPolicyRouting; + for state in [ + UnconstrainedState::new(true, 2), + UnconstrainedState::new(true, 0), + UnconstrainedState::new(false, 2), + UnconstrainedState::new(false, 0), + ] { + let event = PowerPolicyEventData::Unconstrained(state); + let expected = [(DeviceId(0), PowerPolicyUpdate::Unconstrained(state.unconstrained))]; + let updates = routing.route(&event); + assert!(updates.clone().eq(expected)); + assert!(updates.eq(expected)); + } + } +} diff --git a/power-policy-interface/src/service/event.rs b/power-policy-interface/src/service/event.rs index 31199b712..39572d380 100644 --- a/power-policy-interface/src/service/event.rs +++ b/power-policy-interface/src/service/event.rs @@ -287,3 +287,210 @@ impl<'device, PSU: Lockable, S: Sender>> From } } } + +#[cfg(test)] +mod tests { + use core::{ + future::{Future, poll_fn}, + pin::pin, + ptr, + task::{Context, Poll, Waker}, + }; + + use embassy_sync::{blocking_mutex::raw::NoopRawMutex, channel::Channel, mutex::Mutex}; + + use super::{Event, EventData, PowerPolicySubscriber, Receiver, UnconstrainedState}; + use crate::{ + capability::{ + ConsumerPowerCapability, DisconnectFlags, DisconnectReason, PowerCapability, ProviderPowerCapability, + }, + psu, + }; + + #[derive(Default)] + struct TestPsu(psu::State); + + impl psu::Psu for TestPsu { + async fn disconnect(&mut self) -> Result<(), psu::Error> { + self.0.disconnect(false) + } + + async fn connect_provider(&mut self, capability: ProviderPowerCapability) -> Result<(), psu::Error> { + self.0.connect_provider(capability) + } + + async fn connect_consumer(&mut self, capability: ConsumerPowerCapability) -> Result<(), psu::Error> { + self.0.connect_consumer(capability) + } + + fn state(&self) -> &psu::State { + &self.0 + } + + fn state_mut(&mut self) -> &mut psu::State { + &mut self.0 + } + } + + impl embedded_services::named::Named for TestPsu { + fn name(&self) -> &'static str { + "Test PSU" + } + } + + fn poll_once(receiver: &mut impl Receiver) -> Poll { + let mut future = pin!(receiver.wait_next()); + future.as_mut().poll(&mut Context::from_waker(Waker::noop())) + } + + #[test] + fn subscriber_forwards_event_payloads_in_order() { + let events = [ + EventData::ConsumerConnected( + PowerCapability { + voltage_mv: 5000, + current_ma: 1500, + } + .into(), + ), + EventData::ConsumerDisconnected(DisconnectFlags { + reason: Some(DisconnectReason::Switching), + }), + EventData::ProviderConnected( + PowerCapability { + voltage_mv: 20_000, + current_ma: 5000, + } + .into(), + ), + EventData::ProviderDisconnected, + EventData::Unconstrained(UnconstrainedState::new(true, 2)), + ]; + let channel = Channel::::new(); + for event in &events { + assert!(channel.try_send(event.clone()).is_ok()); + } + + let mut subscriber = PowerPolicySubscriber::new(channel.dyn_receiver()); + for event in events { + assert_eq!(poll_once(&mut subscriber), Poll::Ready(event)); + } + assert_eq!(poll_once(&mut subscriber), Poll::Pending); + } + + #[test] + fn subscriber_forwards_full_events_with_psu_references() { + type Psu = Mutex; + let first_psu = Psu::new(TestPsu::default()); + let second_psu = Psu::new(TestPsu::default()); + let channel = Channel::, 2>::new(); + let capability = PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + } + .into(); + let disconnect = DisconnectFlags { + reason: Some(DisconnectReason::Switching), + }; + assert!( + channel + .try_send(Event::ConsumerConnected(&first_psu, capability)) + .is_ok() + ); + assert!( + channel + .try_send(Event::ConsumerDisconnected(&second_psu, disconnect)) + .is_ok() + ); + + let mut subscriber = PowerPolicySubscriber::new(channel.dyn_receiver()); + assert!(matches!( + poll_once(&mut subscriber), + Poll::Ready(Event::ConsumerConnected(psu, received)) + if ptr::eq(psu, &first_psu) && received == capability + )); + assert!(matches!( + subscriber.try_next(), + Some(Event::ConsumerDisconnected(psu, received)) + if ptr::eq(psu, &second_psu) && received == disconnect + )); + assert!(subscriber.try_next().is_none()); + } + + #[test] + fn subscriber_forwards_non_clone_events() { + struct NonCloneEvent(u8); + + let channel = Channel::::new(); + let mut subscriber = PowerPolicySubscriber::new(channel.receiver()); + assert!(channel.try_send(NonCloneEvent(42)).is_ok()); + assert!(matches!(poll_once(&mut subscriber), Poll::Ready(NonCloneEvent(42)))); + assert!(subscriber.try_next().is_none()); + } + + #[test] + fn subscriber_forwards_nonblocking_receives() { + let channel = Channel::::new(); + let mut subscriber = PowerPolicySubscriber::new(channel.receiver()); + assert_eq!(subscriber.try_next(), None); + + let event = EventData::ConsumerDisconnected(DisconnectFlags::default()); + assert!(channel.try_send(event.clone()).is_ok()); + assert_eq!(subscriber.try_next(), Some(event)); + assert_eq!(subscriber.try_next(), None); + } + + #[test] + fn subscriber_preserves_events_after_cancelled_pending_wait() { + let channel = Channel::::new(); + let mut subscriber = PowerPolicySubscriber::new(channel.receiver()); + assert_eq!(poll_once(&mut subscriber), Poll::Pending); + + let event = EventData::ConsumerDisconnected(DisconnectFlags::default()); + assert!(channel.try_send(event.clone()).is_ok()); + assert_eq!(poll_once(&mut subscriber), Poll::Ready(event)); + assert_eq!(subscriber.try_next(), None); + } + + #[test] + fn subscriber_preserves_receiver_send_and_sync() { + struct EmptyReceiver; + + impl Receiver<*const ()> for EmptyReceiver { + fn try_next(&mut self) -> Option<*const ()> { + None + } + + fn wait_next(&mut self) -> impl Future { + core::future::pending() + } + } + + fn assert_send_sync() {} + + assert_send_sync::>(); + } + + #[test] + fn subscriber_accepts_custom_receivers() { + struct CustomReceiver(Option); + + impl Receiver for CustomReceiver { + fn try_next(&mut self) -> Option { + self.0.take() + } + + fn wait_next(&mut self) -> impl Future { + poll_fn(|_| match self.try_next() { + Some(event) => Poll::Ready(event), + None => Poll::Pending, + }) + } + } + + let event = EventData::ProviderDisconnected; + let mut subscriber = PowerPolicySubscriber::new(CustomReceiver(Some(event.clone()))); + assert_eq!(poll_once(&mut subscriber), Poll::Ready(event)); + assert_eq!(subscriber.try_next(), None); + } +} From 3e5bf02da4bdea07d4fb978981f9c2b6f60196d4 Mon Sep 17 00:00:00 2001 From: Matteo Tullo Date: Thu, 22 Oct 2026 13:08:56 -0700 Subject: [PATCH 3/5] docs(battery): document power-policy routing Describe cache semantics, routing options, delivery requirements, and polling integration. Clarify shared-reference support in the relay. Assisted-by: GitHub Copilot:gpt-6-astra --- battery-service-relay/src/lib.rs | 2 + battery-service/README.md | 143 +++++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+) create mode 100644 battery-service/README.md diff --git a/battery-service-relay/src/lib.rs b/battery-service-relay/src/lib.rs index d7e7dbc87..7e00fd8f6 100644 --- a/battery-service-relay/src/lib.rs +++ b/battery-service-relay/src/lib.rs @@ -7,6 +7,8 @@ mod serialization; pub use serialization::{AcpiBatteryError, AcpiBatteryRequest, AcpiBatteryResponse, AcpiBatteryResult}; /// Relays messages to and from a battery service implementation over MCTP. +/// +/// The handler accepts shared references as well as owned services: pub struct BatteryServiceRelayHandler { service: S, } diff --git a/battery-service/README.md b/battery-service/README.md new file mode 100644 index 000000000..8f7641fd8 --- /dev/null +++ b/battery-service/README.md @@ -0,0 +1,143 @@ +# Battery service + +The [battery service](src/lib.rs) answers ACPI queries from +caller-owned fuel-gauge caches. The caller drives the +[FuelGauge interface](../battery-service-interface/src/fuel_gauge.rs); the driver +caches successful reads in its `State`. + +## Per-gauge power sources + +Each fuel gauge's `State` stores a `PsuState` and an unconstrained-power override +from power-policy notifications. +It distinguishes an attached but idle source from an active consumer or a +detached source. PSU state is kept per gauge, with no service-wide copy. +Fuel gauges and ACPI queries do not hold PSU references or acquire PSU locks. +Fuel-gauge drivers need no extra associated type. Without power-policy +integration, the cache remains offline with unknown output capability. + +`_PSR` and `_PIF` use the same `DeviceId` as the fuel gauge and read its cache. +Different gauges can track independent sources, share a source, or receive no +power updates. Reference-based queries are synchronous, like the battery queries: + +```rust +let mut gauge = fuel_gauge.lock().await; +let status = service.is_psu_in_use(&mut *gauge)?; +``` + +- With default routing, `ConsumerConnected` records `ConnectedConsumer` with the + routed source's capability. `ConsumerDisconnected` records `Detached` when the + reason is `Detached`, or `Idle` otherwise. A source can stop supplying power + while still attached. Provider events leave the cache unchanged. +- Unconstrained notifications set or clear the override without replacing the + cached PSU state. Consumer events refresh it from the capability flags; + disconnects clear it. +- `_PSR` reports online for `ConnectedConsumer` or unconstrained power. +- `_PIF` reports unlimited output as `0xFFFFFFFE`, a service-specific convention. + Clearing unconstrained restores the cached consumer's negotiated output in mW, + which may be below the adapter's rated maximum, or offline status if disconnected. +- Without a consumer or unconstrained override, output power is unknown + (`0xFFFFFFFF`). Input rating is always unknown. Unassociated or not-yet-reported + gauges start offline with unknown output. A consumer disconnect clears the + previous negotiated capability. +- Notifications do not provide identity or redundancy/shared-source information, + so identity strings are empty and those flags are clear. +- Unregistered IDs return `UnknownDeviceId`, including ID `0` when no gauges are + registered. Cached external power can still be queried before the battery is + present or initialized. + +## Routing power-policy notifications + +Use [`DefaultPowerPolicyRouting`](src/routing.rs) to route +payload-only power-policy notifications from every PSU to fuel gauge `0` without +defining a local routing type: + +```rust +use battery_service::{ArrayRegistration, DefaultPowerPolicyRouting, Service}; + +let service = Service::new(ArrayRegistration { + fuel_gauges: [fuel_gauge], + routing: DefaultPowerPolicyRouting, +}); +service.process_power_policy_event(&event).await?; +``` + +Every registration must define +[`Registration::Routing`](src/registration.rs) and provide its +router through `routing()`. `ArrayRegistration` owns the caller-provided `routing` +value. The service uses this router for every notification, so its event handler +accepts only the router's associated `Event` type and takes no per-notification +routing argument. The built-in router accepts `PowerPolicyEventData`; custom +routers can use another notification representation. + +With default routing, fuel gauge `0` must be registered. The router applies +notifications to its cache in delivery order and does not aggregate +simultaneous PSU connections. + +For custom mappings or event types, implement +[`battery_service::PowerPolicyRouting`](src/routing.rs). Define +only its associated `Event` type to use the trait's consumer-event policy and +gauge-`0` mapping. Override `route` to return `(DeviceId, PowerPolicyUpdate)` updates. +Return no updates to ignore an event, one for an independent source, or multiple +for gauges sharing a source. Each gauge can receive a different state. +Updates can replace a PSU state or set its unconstrained override. The router +chooses which gauges receive each update. The service validates the destinations, +then locks and updates each gauge. + +To route by source identity, receive full power-policy `Event` notifications or +an application-tagged payload. The default implementation does not need source +identity, so payload-only `EventData` is enough. The associated event type must +implement `Clone + Into` for the trait's default implementation. Route +using immutable source metadata without locking a PSU or the policy. Only state +and capability payloads go into gauge caches. The caches do not retain event +references. + +[`battery_service::EventReceiver`](src/event_receiver.rs) wraps +the shared [`PowerPolicySubscriber`](../power-policy-interface/src/service/event.rs) +and supports either event representation. Await +`service.process_power_policy_event(&event)` for each event before processing the +next. The service rejects invalid target IDs before changing any cache. + +Updates to multiple gauges run sequentially and are not atomic. Let processing +finish, or retain a cancelled event and retry it before applying newer events. +Cloned routing iterators must yield the same destination/update pairs without +side effects. Stateful routers must reproduce updates on retry without consuming +pending updates or applying an event twice to their policy state. + +Power-policy routing does not need its own Embassy task. Like TCPM's event +receiver, `EventReceiver::wait_next_with_poll` can combine power notifications +with the existing battery task's polling deadline: + +```rust +let mut events = battery_service::EventReceiver::new(power_policy_receiver); +let mut ticker = embassy_time::Ticker::every(embassy_time::Duration::from_secs(1)); +loop { + match events.wait_next_with_poll(ticker.next()).await { + battery_service::Event::Poll => { + // Run the existing fuel-gauge polling here. + } + battery_service::Event::PowerPolicy(event) => { + service.process_power_policy_event(&event).await?; + } + } +} +``` + +Keep the ticker outside the loop so incoming notifications do not restart the +polling interval. When both sources are ready, priority alternates, starting with +polling, so neither overdue polling nor notification traffic starves the other. +The method selects only the two waits. Finish hardware polling or notification +processing before waiting again to avoid cancelling either operation. +The [standalone battery example](../examples/std/src/bin/battery.rs) uses this +combined loop. Its power-policy channel is ready for a platform notifier, but +the example does not start a power-policy producer. + +Register the notifier before starting power policy and deliver every connection +and disconnection in order: notifications are not replayed and the caches start +offline. Do not use a lossy immediate publisher for cache updates. + +The [power-policy example](../examples/std/src/bin/power_policy.rs) stores immutable +source IDs alongside PSU mutexes and routes notifications to two independent +gauge caches in the task that polls them. Its `SenderNotifier` uses a bounded +channel; a full queue applies backpressure without dropping events. Routing, +cache updates, and ACPI reads do not acquire a PSU lock, avoiding a dependency +between that lock and the notification queue. From 3668d82a0f7a187fec357820027cccdfc635ceab Mon Sep 17 00:00:00 2001 From: Matteo Tullo Date: Tue, 27 Oct 2026 09:43:09 -0700 Subject: [PATCH 4/5] fix(battery): report _PIF ratings as unavailable ACPI defines the _PIF power fields as the source's rated maximums and reserves only 0xFFFFFFFF, meaning unavailable. Power-policy notifications carry the negotiated contract instead, and unconstrained_power marks an outlet-powered source, not unlimited wattage. Reporting the contract, or 0xFFFFFFFE for unconstrained sources, misstated the rating to the host. Report both ratings as unavailable and remove UNLIMITED_POWER_MW. The unconstrained override now only affects _PSR. Tests check the cached contract and unconstrained flag directly, so routing coverage no longer depends on _PIF wattage. Assisted-by: GitHub Copilot:claude-opus-5.5 --- battery-service-interface/src/fuel_gauge.rs | 7 +- battery-service-interface/src/lib.rs | 13 +- battery-service/README.md | 17 +- battery-service/src/acpi.rs | 219 +++++++++++++------- examples/std/src/bin/power_policy.rs | 6 +- 5 files changed, 158 insertions(+), 104 deletions(-) diff --git a/battery-service-interface/src/fuel_gauge.rs b/battery-service-interface/src/fuel_gauge.rs index 3b3b842ba..36f0ed44e 100644 --- a/battery-service-interface/src/fuel_gauge.rs +++ b/battery-service-interface/src/fuel_gauge.rs @@ -446,15 +446,16 @@ impl State { matches!(state, PsuState::ConnectedConsumer(capability) if capability.flags.unconstrained_power); } - /// Whether cached external power is online with unlimited output. + /// Whether the cached external source is unconstrained, i.e. outlet-powered rather than + /// battery-powered. This says nothing about its wattage. pub fn is_psu_unconstrained(&self) -> bool { self.psu_unconstrained } /// Set the unconstrained override without changing the cached PSU state. /// - /// Clearing it restores the cached consumer's negotiated wattage, or offline - /// status if no consumer is connected. + /// While set, `_PSR` reports online. Clearing it restores the status implied by the + /// cached PSU state. pub fn set_psu_unconstrained(&mut self, unconstrained: bool) { self.psu_unconstrained = unconstrained; } diff --git a/battery-service-interface/src/lib.rs b/battery-service-interface/src/lib.rs index 4a72a9404..55e478514 100644 --- a/battery-service-interface/src/lib.rs +++ b/battery-service-interface/src/lib.rs @@ -23,11 +23,6 @@ pub const STD_PIF_SERIAL_SIZE: usize = 8; /// Standard Power Policy Service OEM Info String Size pub const STD_PIF_OEM_SIZE: usize = 8; -/// Service-specific sentinel for unlimited output power (mW). -/// -/// `0xFFFFFFFF` remains reserved for unavailable power. -pub const UNLIMITED_POWER_MW: u32 = u32::MAX - 1; - #[derive(PartialEq, Clone, Copy, Default)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub struct BixFixedStrings { @@ -82,7 +77,7 @@ pub struct PifFixedStrings { pub power_source_state: PowerSourceState, /// Maximum rated output power in milliwatts (mW). /// - /// [`UNLIMITED_POWER_MW`] indicates unconstrained output; `0xFFFFFFFF` means unavailable. + /// 0xFFFFFFFF indicates the value is unavailable. pub max_output_power: u32, /// Maximum rated input power in milliwatts (mW). /// @@ -194,9 +189,9 @@ pub trait BatteryService { /// Queries information about the battery's power source. Corresponds to ACPI's _PIF method. /// - /// The query reads the cached external-power state of the fuel gauge selected by `power_source_id`. - /// Unconstrained output is [`UNLIMITED_POWER_MW`]; otherwise it is the cached consumer's - /// negotiated wattage, or unknown without a consumer. An unregistered ID returns an error. + /// Output and input ratings are unavailable (`0xFFFFFFFF`). ACPI defines them as the source's + /// rated maximums, but power-policy notifications carry only the negotiated contract. + /// An unregistered `power_source_id` returns an error. fn power_source_information( &self, power_source_id: DeviceId, diff --git a/battery-service/README.md b/battery-service/README.md index 8f7641fd8..c79a60ee1 100644 --- a/battery-service/README.md +++ b/battery-service/README.md @@ -13,9 +13,9 @@ It distinguishes an attached but idle source from an active consumer or a detached source. PSU state is kept per gauge, with no service-wide copy. Fuel gauges and ACPI queries do not hold PSU references or acquire PSU locks. Fuel-gauge drivers need no extra associated type. Without power-policy -integration, the cache remains offline with unknown output capability. +integration, the cache remains offline. -`_PSR` and `_PIF` use the same `DeviceId` as the fuel gauge and read its cache. +`_PSR` and `_PIF` use the same `DeviceId` as the fuel gauge; `_PSR` reads its cache. Different gauges can track independent sources, share a source, or receive no power updates. Reference-based queries are synchronous, like the battery queries: @@ -32,13 +32,12 @@ let status = service.is_psu_in_use(&mut *gauge)?; cached PSU state. Consumer events refresh it from the capability flags; disconnects clear it. - `_PSR` reports online for `ConnectedConsumer` or unconstrained power. -- `_PIF` reports unlimited output as `0xFFFFFFFE`, a service-specific convention. - Clearing unconstrained restores the cached consumer's negotiated output in mW, - which may be below the adapter's rated maximum, or offline status if disconnected. -- Without a consumer or unconstrained override, output power is unknown - (`0xFFFFFFFF`). Input rating is always unknown. Unassociated or not-yet-reported - gauges start offline with unknown output. A consumer disconnect clears the - previous negotiated capability. +- `_PIF` reports output and input ratings as unavailable (`0xFFFFFFFF`). ACPI + defines them as the source's rated maximums, but notifications carry only the + negotiated contract, which can be lower and can change while the same source + stays attached. Unconstrained means outlet-powered, not unlimited wattage. +- Unassociated or not-yet-reported gauges start offline. A consumer disconnect + clears the previous negotiated capability. - Notifications do not provide identity or redundancy/shared-source information, so identity strings are empty and those flags are clear. - Unregistered IDs return `UnknownDeviceId`, including ID `0` when no gauges are diff --git a/battery-service/src/acpi.rs b/battery-service/src/acpi.rs index 983ebf48b..8b5c66645 100644 --- a/battery-service/src/acpi.rs +++ b/battery-service/src/acpi.rs @@ -10,7 +10,7 @@ use embedded_services::{error, info, trace}; use battery_service_interface::{ BctReturnResult, BixFixedStrings, Bmd, Bpc, Bps, BstReturn, BtmReturnResult, DeviceId, PifFixedStrings, PsrReturn, STD_BIX_BATTERY_SIZE, STD_BIX_MODEL_SIZE, STD_BIX_OEM_SIZE, STD_BIX_SERIAL_SIZE, STD_PIF_MODEL_SIZE, - STD_PIF_OEM_SIZE, STD_PIF_SERIAL_SIZE, StaReturn, UNLIMITED_POWER_MW, + STD_PIF_OEM_SIZE, STD_PIF_SERIAL_SIZE, StaReturn, }; use power_policy_interface::psu::PsuState; @@ -175,18 +175,12 @@ pub(crate) fn compute_psr(psu_state: PsuState, unconstrained: bool) -> PsrReturn } } -pub(crate) fn compute_pif(psu_state: PsuState, unconstrained: bool) -> PifFixedStrings { +pub(crate) fn compute_pif() -> PifFixedStrings { PifFixedStrings { power_source_state: PowerSourceState::empty(), - // Policy reports the available or negotiated output, which may differ from the adapter's nameplate rating. - max_output_power: if unconstrained { - UNLIMITED_POWER_MW - } else { - match psu_state { - PsuState::ConnectedConsumer(capability) => capability.capability.max_power_mw(), - _ => u32::MAX, - } - }, + // `_PIF` reports rated maximums. Notifications carry only the negotiated contract, and + // unconstrained means outlet-powered rather than unlimited, so both ratings are unavailable. + max_output_power: u32::MAX, max_input_power: u32::MAX, model_number: [0; STD_PIF_MODEL_SIZE], serial_number: [0; STD_PIF_SERIAL_SIZE], @@ -365,18 +359,17 @@ impl<'hw, Reg: crate::registration::Registration<'hw>> crate::Service<'hw, Reg> /// Queries information about the battery's power source. Corresponds to ACPI's _PIF method. /// - /// Reports [`UNLIMITED_POWER_MW`] for unconstrained power, otherwise the cached - /// consumer's negotiated wattage or `u32::MAX` without a consumer. - /// The negotiated value may be below the rated maximum. Input rating is always unknown. + /// Reports output and input ratings as unavailable (`0xFFFFFFFF`). ACPI defines them as the + /// source's rated maximums, but notifications carry only the negotiated contract. /// Notifications do not include adapter identity or redundancy/shared-source /// flags, so the strings are empty and those flags are clear. + /// TODO: Investigate how we can add more detailed power source information. pub fn power_source_information( &self, - fuel_gauge: &mut ::Inner, + _fuel_gauge: &mut ::Inner, ) -> Result { trace!("Battery service: got PIF command!"); - let state = fuel_gauge.state(); - Ok(compute_pif(state.psu_state(), state.is_psu_unconstrained())) + Ok(compute_pif()) } /// Queries the battery's status. Corresponds to ACPI's _STA method. @@ -474,28 +467,50 @@ mod tests { } } + /// Expected external-power cache of a fuel gauge. + #[derive(Clone, Copy, Debug)] + enum Expected { + /// No consumer contract and no unconstrained flag. + Offline, + /// A consumer contract of this many mW, without the unconstrained flag. + Contract(u32), + /// The unconstrained flag, with the cached consumer contract in mW, if any. + Unconstrained(Option), + } + async fn assert_power_source<'hw, Reg: Registration<'hw>>( service: &Service<'hw, Reg>, id: DeviceId, - power_mw: Option, + expected: Expected, ) { + let (online, contract_mw, unconstrained) = match expected { + Expected::Offline => (false, None, false), + Expected::Contract(mw) => (true, Some(mw), false), + Expected::Unconstrained(contract_mw) => (true, contract_mw, true), + }; let status = BatteryService::is_psu_in_use(service, id).await.unwrap(); assert!( status.power_source - == if power_mw.is_some() { + == if online { PowerSource::Online } else { PowerSource::Offline } ); let information = BatteryService::power_source_information(service, id).await.unwrap(); - assert_eq!(information.max_output_power, power_mw.unwrap_or(u32::MAX)); + assert_eq!(information.max_output_power, u32::MAX); assert_eq!(information.max_input_power, u32::MAX); assert!(information.power_source_state.is_empty()); assert_eq!(information.model_number, [0; STD_PIF_MODEL_SIZE]); assert_eq!(information.serial_number, [0; STD_PIF_SERIAL_SIZE]); assert_eq!(information.oem_info, [0; STD_PIF_OEM_SIZE]); let mut gauge = service.get_fuel_gauge(id).unwrap().lock().await; + let cached_mw = match gauge.state().psu_state() { + PsuState::ConnectedConsumer(capability) => Some(capability.capability.max_power_mw()), + _ => None, + }; + assert_eq!(cached_mw, contract_mw); + assert_eq!(gauge.state().is_psu_unconstrained(), unconstrained); assert!(service.is_psu_in_use(&mut *gauge).unwrap().power_source == status.power_source); assert!(service.power_source_information(&mut *gauge).unwrap() == information); } @@ -518,7 +533,7 @@ mod tests { .await .unwrap() .max_output_power, - 65_000 + u32::MAX ); } @@ -570,8 +585,8 @@ mod tests { }) .await .unwrap(); - assert_power_source(&service, DeviceId(0), Some(capability.max_power_mw())).await; - assert_power_source(&service, DeviceId(1), None).await; + assert_power_source(&service, DeviceId(0), Expected::Contract(capability.max_power_mw())).await; + assert_power_source(&service, DeviceId(1), Expected::Offline).await; service .process_power_policy_event(&SourceEvent { @@ -580,8 +595,8 @@ mod tests { }) .await .unwrap(); - assert_power_source(&service, DeviceId(0), None).await; - assert_power_source(&service, DeviceId(1), None).await; + assert_power_source(&service, DeviceId(0), Expected::Offline).await; + assert_power_source(&service, DeviceId(1), Expected::Offline).await; } }); } @@ -597,7 +612,7 @@ mod tests { }); assert!(!gauge.lock().await.state().is_present()); assert_eq!(gauge.lock().await.state().psu_state(), PsuState::Detached); - assert_power_source(&service, DeviceId(0), None).await; + assert_power_source(&service, DeviceId(0), Expected::Offline).await; for (capability, expected_mw) in [ ( @@ -637,8 +652,8 @@ mod tests { gauge.lock().await.state().psu_state(), PsuState::ConnectedConsumer(capability.into()) ); - assert_power_source(&service, DeviceId(0), Some(expected_mw)).await; - assert_power_source(&service, DeviceId(1), None).await; + assert_power_source(&service, DeviceId(0), Expected::Contract(expected_mw)).await; + assert_power_source(&service, DeviceId(1), Expected::Offline).await; } for reason in [ @@ -669,8 +684,8 @@ mod tests { for _ in 0..2 { service.process_power_policy_event(&event).await.unwrap(); assert_eq!(gauge.lock().await.state().psu_state(), expected_state); - assert_power_source(&service, DeviceId(0), None).await; - assert_power_source(&service, DeviceId(1), None).await; + assert_power_source(&service, DeviceId(0), Expected::Offline).await; + assert_power_source(&service, DeviceId(1), Expected::Offline).await; assert_eq!(unrelated.lock().await.state().psu_state(), PsuState::Detached); } } @@ -684,12 +699,13 @@ mod tests { voltage_mv: 20_000, current_ma: 3250, }; - for (psu_state, restored_power) in [ + for (psu_state, contract_mw) in [ (PsuState::Detached, None), (PsuState::Idle, None), (PsuState::ConnectedProvider(capability.into()), None), (PsuState::ConnectedConsumer(capability.into()), Some(65_000)), ] { + let restored = contract_mw.map_or(Expected::Offline, Expected::Contract); let gauge = Mutex::::new(MockFuelGauge::new()); let unrelated = Mutex::::new(MockFuelGauge::new()); gauge.lock().await.state_mut().set_psu_state(psu_state); @@ -697,7 +713,7 @@ mod tests { fuel_gauges: [&gauge, &unrelated], routing: DefaultPowerPolicyRouting, }); - assert_power_source(&service, DeviceId(0), restored_power).await; + assert_power_source(&service, DeviceId(0), restored).await; for notification in [ UnconstrainedState::new(true, 1), @@ -720,13 +736,13 @@ mod tests { &service, DeviceId(0), if notification.unconstrained { - Some(0xffff_fffe) + Expected::Unconstrained(contract_mw) } else { - restored_power + restored }, ) .await; - assert_power_source(&service, DeviceId(1), None).await; + assert_power_source(&service, DeviceId(1), Expected::Offline).await; } } }); @@ -755,46 +771,78 @@ mod tests { for (event, expected_power) in [ ( PowerPolicyEventData::Unconstrained(UnconstrainedState::new(true, 1)), - Some(0xffff_fffe), + Expected::Unconstrained(None), + ), + ( + PowerPolicyEventData::ConsumerConnected(constrained), + Expected::Contract(65_000), ), - (PowerPolicyEventData::ConsumerConnected(constrained), Some(65_000)), ( PowerPolicyEventData::ConsumerConnected(unconstrained), - Some(0xffff_fffe), + Expected::Unconstrained(Some(65_000)), ), ( PowerPolicyEventData::Unconstrained(UnconstrainedState::new(false, 1)), - Some(65_000), + Expected::Contract(65_000), ), ( PowerPolicyEventData::ConsumerConnected(unconstrained), - Some(0xffff_fffe), + Expected::Unconstrained(Some(65_000)), ), ( PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags::default()), - None, + Expected::Offline, ), ( PowerPolicyEventData::Unconstrained(UnconstrainedState::new(true, 1)), - Some(0xffff_fffe), + Expected::Unconstrained(None), ), ( PowerPolicyEventData::ConsumerDisconnected(DisconnectFlags { reason: Some(DisconnectReason::Detached), }), - None, + Expected::Offline, ), ] { service.process_power_policy_event(&event).await.unwrap(); assert_power_source(&service, DeviceId(0), expected_power).await; - assert_eq!( - gauge.lock().await.state().is_psu_unconstrained(), - expected_power == Some(0xffff_fffe) - ); } }); } + #[test] + fn outlet_powered_consumer_reports_unavailable_ratings() { + block_on(async { + let gauge = Mutex::::new(MockFuelGauge::new()); + let service = Service::new(ArrayRegistration { + fuel_gauges: [&gauge], + routing: DefaultPowerPolicyRouting, + }); + // A 65 W contract from an outlet-powered adapter is neither unlimited nor the adapter's rating. + let outlet_powered = ConsumerPowerCapability { + capability: PowerCapability { + voltage_mv: 20_000, + current_ma: 3250, + }, + flags: ConsumerFlags { + unconstrained_power: true, + ..Default::default() + }, + }; + service + .process_power_policy_event(&PowerPolicyEventData::ConsumerConnected(outlet_powered)) + .await + .unwrap(); + let status = BatteryService::is_psu_in_use(&service, DeviceId(0)).await.unwrap(); + assert!(status.power_source == PowerSource::Online); + let information = BatteryService::power_source_information(&service, DeviceId(0)) + .await + .unwrap(); + assert_eq!(information.max_output_power, u32::MAX); + assert_eq!(information.max_input_power, u32::MAX); + }); + } + #[test] fn attached_psu_without_input_power_reports_offline_and_unknown_output() { block_on(async { @@ -814,7 +862,7 @@ mod tests { ), ] { gauge.lock().await.state_mut().set_psu_state(state); - assert_power_source(&service, DeviceId(0), None).await; + assert_power_source(&service, DeviceId(0), Expected::Offline).await; assert_eq!(gauge.lock().await.state().psu_state(), state); } }); @@ -854,8 +902,8 @@ mod tests { .await .unwrap(); } - assert_power_source(&service, DeviceId(0), Some(7500)).await; - assert_power_source(&service, DeviceId(1), Some(65_000)).await; + assert_power_source(&service, DeviceId(0), Expected::Contract(7500)).await; + assert_power_source(&service, DeviceId(1), Expected::Contract(65_000)).await; for unconstrained in [true, false] { service @@ -868,10 +916,14 @@ mod tests { assert_power_source( &service, DeviceId(0), - Some(if unconstrained { 0xffff_fffe } else { 7500 }), + if unconstrained { + Expected::Unconstrained(Some(7500)) + } else { + Expected::Contract(7500) + }, ) .await; - assert_power_source(&service, DeviceId(1), Some(65_000)).await; + assert_power_source(&service, DeviceId(1), Expected::Contract(65_000)).await; } service @@ -881,8 +933,8 @@ mod tests { }) .await .unwrap(); - assert_power_source(&service, DeviceId(0), None).await; - assert_power_source(&service, DeviceId(1), Some(65_000)).await; + assert_power_source(&service, DeviceId(0), Expected::Offline).await; + assert_power_source(&service, DeviceId(1), Expected::Contract(65_000)).await; service .process_power_policy_event(&SourceEvent { source: DeviceId(1), @@ -896,8 +948,8 @@ mod tests { }) .await .unwrap(); - assert_power_source(&service, DeviceId(0), None).await; - assert_power_source(&service, DeviceId(1), Some(100_000)).await; + assert_power_source(&service, DeviceId(0), Expected::Offline).await; + assert_power_source(&service, DeviceId(1), Expected::Contract(100_000)).await; for (id, voltage) in [(DeviceId(0), 11_100), (DeviceId(1), 7400)] { assert_eq!( @@ -929,9 +981,12 @@ mod tests { .await .unwrap(); for id in [DeviceId(0), DeviceId(1)] { - assert_power_source(&service, id, Some(65_000)).await; + assert_power_source(&service, id, Expected::Contract(65_000)).await; } - for (unconstrained, expected_power) in [(true, 0xffff_fffe), (false, 65_000)] { + for (unconstrained, expected_power) in [ + (true, Expected::Unconstrained(Some(65_000))), + (false, Expected::Contract(65_000)), + ] { service .process_power_policy_event(&PowerPolicyEventData::Unconstrained(UnconstrainedState::new( unconstrained, @@ -940,7 +995,7 @@ mod tests { .await .unwrap(); for id in [DeviceId(0), DeviceId(1)] { - assert_power_source(&service, id, Some(expected_power)).await; + assert_power_source(&service, id, expected_power).await; } } service @@ -948,7 +1003,7 @@ mod tests { .await .unwrap(); for id in [DeviceId(0), DeviceId(1)] { - assert_power_source(&service, id, None).await; + assert_power_source(&service, id, Expected::Offline).await; } }); } @@ -999,8 +1054,8 @@ mod tests { expected ); } - assert_power_source(&service, DeviceId(0), None).await; - assert_power_source(&service, DeviceId(1), Some(65_000)).await; + assert_power_source(&service, DeviceId(0), Expected::Offline).await; + assert_power_source(&service, DeviceId(1), Expected::Contract(65_000)).await; } }); } @@ -1014,8 +1069,8 @@ mod tests { fuel_gauges: [&first, &second], routing: SourceRouting, }); - assert_power_source(&service, DeviceId(0), None).await; - assert_power_source(&service, DeviceId(1), None).await; + assert_power_source(&service, DeviceId(0), Expected::Offline).await; + assert_power_source(&service, DeviceId(1), Expected::Offline).await; service .process_power_policy_event(&SourceEvent { source: DeviceId(1), @@ -1029,8 +1084,8 @@ mod tests { }) .await .unwrap(); - assert_power_source(&service, DeviceId(0), None).await; - assert_power_source(&service, DeviceId(1), Some(7500)).await; + assert_power_source(&service, DeviceId(0), Expected::Offline).await; + assert_power_source(&service, DeviceId(1), Expected::Contract(7500)).await; service .process_power_policy_event(&SourceEvent { @@ -1039,8 +1094,8 @@ mod tests { }) .await .unwrap(); - assert_power_source(&service, DeviceId(0), None).await; - assert_power_source(&service, DeviceId(1), Some(7500)).await; + assert_power_source(&service, DeviceId(0), Expected::Offline).await; + assert_power_source(&service, DeviceId(1), Expected::Contract(7500)).await; }); } @@ -1099,15 +1154,15 @@ mod tests { } .into(), ), - 65_000, + Expected::Contract(65_000), ), ( PowerPolicyEventData::Unconstrained(UnconstrainedState::new(true, 1)), - 0xffff_fffe, + Expected::Unconstrained(Some(65_000)), ), ( PowerPolicyEventData::Unconstrained(UnconstrainedState::new(false, 0)), - 65_000, + Expected::Contract(65_000), ), ] { let second_guard = second.lock().await; @@ -1119,12 +1174,12 @@ mod tests { .poll(&mut Context::from_waker(Waker::noop())) .is_pending() ); - assert_power_source(&service, DeviceId(0), Some(expected_power)).await; + assert_power_source(&service, DeviceId(0), expected_power).await; } drop(second_guard); service.process_power_policy_event(&event).await.unwrap(); for id in [DeviceId(0), DeviceId(1)] { - assert_power_source(&service, id, Some(expected_power)).await; + assert_power_source(&service, id, expected_power).await; } } }); @@ -1182,7 +1237,7 @@ mod tests { gauge.lock().await.state().psu_state(), PsuState::ConnectedConsumer(capability) ); - assert_power_source(&service, DeviceId(0), Some(65_000)).await; + assert_power_source(&service, DeviceId(0), Expected::Contract(65_000)).await; send.await.unwrap(); let disconnected = event_receiver.wait_next_with_poll(pending()).await; assert_eq!( @@ -1193,7 +1248,7 @@ mod tests { service.process_power_policy_event(&event).await.unwrap(); } assert_eq!(gauge.lock().await.state().psu_state(), PsuState::Idle); - assert_power_source(&service, DeviceId(0), None).await; + assert_power_source(&service, DeviceId(0), Expected::Offline).await; assert!(psu.try_lock().is_err()); assert!(event_receiver.try_next().is_none()); }); @@ -1233,7 +1288,7 @@ mod tests { service.process_power_policy_event(&event).await, Err(BatteryError::UnknownDeviceId) )); - assert_power_source(&service, DeviceId(0), Some(7500)).await; + assert_power_source(&service, DeviceId(0), Expected::Contract(7500)).await; } }); } @@ -1271,7 +1326,11 @@ mod tests { assert_power_source( &service, DeviceId(0), - Some(if unconstrained { 65_000 } else { 0xffff_fffe }), + if unconstrained { + Expected::Contract(65_000) + } else { + Expected::Unconstrained(Some(65_000)) + }, ) .await; } @@ -1317,7 +1376,11 @@ mod tests { assert_power_source( &service, DeviceId(0), - matches!(state, PsuState::ConnectedConsumer(_)).then_some(0xffff_fffe), + if matches!(state, PsuState::ConnectedConsumer(_)) { + Expected::Unconstrained(Some(7500)) + } else { + Expected::Offline + }, ) .await; } diff --git a/examples/std/src/bin/power_policy.rs b/examples/std/src/bin/power_policy.rs index 36d1b8004..174bfd753 100644 --- a/examples/std/src/bin/power_policy.rs +++ b/examples/std/src/bin/power_policy.rs @@ -630,14 +630,10 @@ async fn battery_task(events: channel::DynamicReceiver<'static, ServiceEvent>) { let status = BatteryService::is_psu_in_use(&battery, id) .await .expect("Failed to read fuel-gauge power status"); - let information = BatteryService::power_source_information(&battery, id) - .await - .expect("Failed to read fuel-gauge power information"); info!( - "Battery ACPI: gauge={}, external power online={}, available output={} mW (0xFFFFFFFF means unknown)", + "Battery ACPI: gauge={}, external power online={}", id.0, status.power_source == embedded_batteries_async::acpi::PowerSource::Online, - information.max_output_power, ); } } From 3d10c3d857c6e94c60e45dadbcb73fc650a97dc4 Mon Sep 17 00:00:00 2001 From: Matteo Tullo Date: Tue, 27 Oct 2026 09:43:15 -0700 Subject: [PATCH 5/5] fixup! feat(battery)!: route power-policy events Explain why PowerPolicySubscriber is a separate field type: composite receivers can await it alongside other fields because each future borrows only its own field, while &mut self helper methods on the composite would each borrow the whole struct. Assisted-by: GitHub Copilot:claude-opus-5.5 --- power-policy-interface/src/service/event.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/power-policy-interface/src/service/event.rs b/power-policy-interface/src/service/event.rs index 39572d380..73a2cbef8 100644 --- a/power-policy-interface/src/service/event.rs +++ b/power-policy-interface/src/service/event.rs @@ -50,6 +50,11 @@ where /// Receives power-policy notifications. /// /// Wraps any [`Receiver`] and forwards its events unchanged. +/// +/// Keeping this in its own field lets us wait on multiple receivers with `select`, +/// since each wait borrows only its own field. With `&mut self` helper methods on +/// the parent struct, each wait would borrow the whole struct instead, so the +/// borrow checker wouldn't allow both waits at once. pub struct PowerPolicySubscriber> { receiver: R, // Track E without owning it or imposing its Send/Sync requirements.