Skip to content

Battery service refactor #698

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tullom merged 2 commits into from
Feb 3, 2026
Merged

Battery service refactor #698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14fc5f4
Battery service refactor
tullom Jan 30, 2026
9f52ddf
Fix clippy errors and make `process_next`, `wait_next`, and
tullom Feb 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions battery-service/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ embedded-services.workspace = true
log = { workspace = true, optional = true }
zerocopy.workspace = true
mctp-rs = { workspace = true, features = ["espi"] }
heapless.workspace = true

[features]
default = []
Expand Down
19 changes: 5 additions & 14 deletions battery-service/src/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,11 @@ impl Context {
}
}

pub(super) async fn process_acpi_cmd(&self, acpi_msg: &AcpiBatteryRequest) {
let response: Result<AcpiBatteryResponse, AcpiBatteryError> = match *acpi_msg {
pub(super) async fn process_acpi_cmd(
&self,
acpi_msg: &AcpiBatteryRequest,
) -> Result<AcpiBatteryResponse, AcpiBatteryError> {
match *acpi_msg {
AcpiBatteryRequest::BatteryGetBixRequest { battery_id } => self.bix_handler(DeviceId(battery_id)).await,
AcpiBatteryRequest::BatteryGetBstRequest { battery_id } => self.bst_handler(DeviceId(battery_id)).await,
AcpiBatteryRequest::BatteryGetPsrRequest { battery_id } => self.psr_handler(DeviceId(battery_id)).await,
Expand Down Expand Up @@ -434,19 +437,7 @@ impl Context {
self.bma_handler(DeviceId(battery_id), bma).await
}
AcpiBatteryRequest::BatteryGetStaRequest { battery_id } => self.sta_handler(DeviceId(battery_id)).await,
};

if let Err(e) = response {
error!("Battery service command failed: {:?}", e);
}

// TODO We should probably be responding to the requestor rather than just assuming the request came from the host
super::comms_send(
crate::EndpointID::External(embedded_services::comms::External::Host),
&response,
)
.await
.expect("comms_send is infallible");
}

pub(crate) fn get_fuel_gauge(&self, id: DeviceId) -> Option<&'static Device> {
Expand Down
92 changes: 52 additions & 40 deletions battery-service/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use context::BatteryEvent;
use embassy_futures::select::select;
use embedded_services::{
comms::{self, EndpointID},
trace,
error, trace,
};

mod acpi;
Expand All @@ -30,7 +30,7 @@ impl Service {
}

/// Create a new battery service instance with context configuration.
pub fn new_with_ctx_config(config: context::Config) -> Self {
pub const fn new_with_ctx_config(config: context::Config) -> Self {
Self::new_inner(config)
}

Expand Down Expand Up @@ -64,10 +64,59 @@ impl Service {
}
Event::AcpiRequest(acpi_msg) => {
trace!("Battery service: ACPI cmd recvd");
self.context.process_acpi_cmd(&acpi_msg).await
match self.context.process_acpi_cmd(&acpi_msg).await {
Ok(response) => {
// TODO We should probably be responding to the requestor rather than just assuming the request came from the host
self.comms_send(
crate::EndpointID::External(embedded_services::comms::External::Host),
&response,
)
.await
.expect("comms_send is infallible")
}
Err(e) => error!("Battery service command failed: {:?}", e),
}
}
}
}

/// Register fuel gauge device with the battery service.
///
/// Must be done before sending the battery service commands so that hardware device is visible
/// to the battery service.
pub(crate) fn register_fuel_gauge(
&self,
device: &'static device::Device,
) -> Result<(), embedded_services::intrusive_list::Error> {
self.context.register_fuel_gauge(device)?;

Ok(())
}

/// Use the battery service endpoint to send data to other subsystems and services.
pub async fn comms_send(&self, endpoint_id: EndpointID, data: &(impl Any + Send + Sync)) -> Result<(), Infallible> {
self.endpoint.send(endpoint_id, data).await
}

/// Send the battery service state machine an event and await a response.
///
/// This is an alternative method of interacting with the battery service (instead of using the comms service),
/// and is a useful fn if you want to send an event and await a response sequentially.
pub async fn execute_event(&self, event: BatteryEvent) -> context::BatteryResponse {
self.context.execute_event(event).await
}

/// Wait for a response from the battery service.
///
/// Use this function after sending the battery service a message via the comms system.
pub async fn wait_for_battery_response(&self) -> context::BatteryResponse {
self.context.wait_response().await
}

/// Asynchronously query the state from the state machine.
pub async fn get_state(&self) -> context::State {
self.context.get_state().await
}
}

#[derive(Clone, Copy)]
Expand Down Expand Up @@ -98,40 +147,3 @@ impl comms::MailboxDelegate for Service {
Ok(())
}
}

static SERVICE: Service = Service::new();

/// Register fuel gauge device with the battery service.
///
/// Must be done before sending the battery service commands so that hardware device is visible
/// to the battery service.
pub fn register_fuel_gauge(device: &'static device::Device) -> Result<(), embedded_services::intrusive_list::Error> {
SERVICE.context.register_fuel_gauge(device)?;

Ok(())
}

/// Use the battery service endpoint to send data to other subsystems and services.
pub async fn comms_send(endpoint_id: EndpointID, data: &(impl Any + Send + Sync)) -> Result<(), Infallible> {
SERVICE.endpoint.send(endpoint_id, data).await
}

/// Send the battery service state machine an event and await a response.
///
/// This is an alternative method of interacting with the battery service (instead of using the comms service),
/// and is a useful fn if you want to send an event and await a response sequentially.
pub async fn execute_event(event: BatteryEvent) -> context::BatteryResponse {
SERVICE.context.execute_event(event).await
}

/// Wait for a response from the battery service.
///
/// Use this function after sending the battery service a message via the comms system.
pub async fn wait_for_battery_response() -> context::BatteryResponse {
SERVICE.context.wait_response().await
}

/// Asynchronously query the state from the state machine.
pub async fn get_state() -> context::State {
SERVICE.context.get_state().await
}
35 changes: 30 additions & 5 deletions battery-service/src/task.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,42 @@
use battery_service_messages::DeviceId;
use embedded_services::{comms, error, info};

use crate::SERVICE;
use crate::{Service, device::Device};

/// Standard dynamic battery data cache
#[derive(Debug, Clone)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub enum InitError<const N: usize> {
DeviceRegistrationFailed(heapless::Vec<DeviceId, N>),
CommsRegistrationFailed,
}

/// Battery service task.
pub async fn task() {
pub async fn task<const N: usize>(
service: &'static Service,
devices: [&'static Device; N],
) -> Result<(), InitError<N>> {
info!("Starting battery-service task");

if comms::register_endpoint(&SERVICE, &SERVICE.endpoint).await.is_err() {
let mut failed_devices = heapless::Vec::new();
for device in devices {
if service.register_fuel_gauge(device).is_err() {
error!("Failed to register battery device with DeviceId {:?}", device.id());
// Infallible as the Vec is as large as the list of devices passed in.
let _ = failed_devices.push(device.id());
}
}

if !failed_devices.is_empty() {
return Err(InitError::DeviceRegistrationFailed(failed_devices));
}

if comms::register_endpoint(service, &service.endpoint).await.is_err() {
error!("Failed to register battery service endpoint");
return;
return Err(InitError::CommsRegistrationFailed);
}

loop {
SERVICE.process_next().await;
service.process_next().await;
}
}
1 change: 1 addition & 0 deletions examples/pico-de-gallo/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

102 changes: 56 additions & 46 deletions examples/pico-de-gallo/src/bin/battery.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,16 +151,18 @@ impl bs::controller::Controller for Battery {
}
}

async fn init_and_run_service(i2c: pico_de_gallo_hal::I2c, delay: pico_de_gallo_hal::Delay) -> ! {
async fn init_and_run_service(
battery_service: &'static battery_service::Service,
i2c: pico_de_gallo_hal::I2c,
delay: pico_de_gallo_hal::Delay,
) -> ! {
embedded_services::debug!("Initializing battery service");
embedded_services::init().await;

static BATTERY_DEVICE: StaticCell<bs::device::Device> = StaticCell::new();
static BATTERY_WRAPPER: StaticCell<bs::wrapper::Wrapper<'static, Battery>> = StaticCell::new();
let device = BATTERY_DEVICE.init(bs::device::Device::new(bs::device::DeviceId(0)));

battery_service::register_fuel_gauge(device).expect("Failed to register fuel gauge");

let wrapper = BATTERY_WRAPPER.init(bs::wrapper::Wrapper::new(
device,
Battery {
Expand All @@ -170,45 +172,49 @@ async fn init_and_run_service(i2c: pico_de_gallo_hal::I2c, delay: pico_de_gallo_

// Run battery service
let _ = embassy_futures::join::join(
tokio::spawn(battery_service::task::task()),
tokio::spawn(battery_service::task::task(battery_service, [device])),
tokio::spawn(wrapper.process()),
)
.await;
unreachable!()
}

async fn init_state_machine() -> Result<(), bs::context::ContextError> {
battery_service::execute_event(battery_service::context::BatteryEvent {
event: battery_service::context::BatteryEventInner::DoInit,
device_id: bs::device::DeviceId(0),
})
.await
.inspect_err(|f| embedded_services::debug!("Fuel gauge init error: {:?}", f))?;

battery_service::execute_event(battery_service::context::BatteryEvent {
event: battery_service::context::BatteryEventInner::PollStaticData,
device_id: bs::device::DeviceId(0),
})
.await
.inspect_err(|f| embedded_services::debug!("Fuel gauge static data error: {:?}", f))?;

battery_service::execute_event(battery_service::context::BatteryEvent {
event: battery_service::context::BatteryEventInner::PollDynamicData,
device_id: bs::device::DeviceId(0),
})
.await
.inspect_err(|f| embedded_services::debug!("Fuel gauge dynamic data error: {:?}", f))?;
async fn init_state_machine(battery_service: &'static bs::Service) -> Result<(), bs::context::ContextError> {
battery_service
.execute_event(battery_service::context::BatteryEvent {
event: battery_service::context::BatteryEventInner::DoInit,
device_id: bs::device::DeviceId(0),
})
.await
.inspect_err(|f| embedded_services::debug!("Fuel gauge init error: {:?}", f))?;

Ok(())
}
battery_service
.execute_event(battery_service::context::BatteryEvent {
event: battery_service::context::BatteryEventInner::PollStaticData,
device_id: bs::device::DeviceId(0),
})
.await
.inspect_err(|f| embedded_services::debug!("Fuel gauge static data error: {:?}", f))?;

async fn recover_state_machine() -> Result<(), ()> {
loop {
match battery_service::execute_event(battery_service::context::BatteryEvent {
event: battery_service::context::BatteryEventInner::Timeout,
battery_service
.execute_event(battery_service::context::BatteryEvent {
event: battery_service::context::BatteryEventInner::PollDynamicData,
device_id: bs::device::DeviceId(0),
})
.await
.inspect_err(|f| embedded_services::debug!("Fuel gauge dynamic data error: {:?}", f))?;

Ok(())
}

async fn recover_state_machine(battery_service: &'static battery_service::Service) -> Result<(), ()> {
loop {
match battery_service
.execute_event(battery_service::context::BatteryEvent {
event: battery_service::context::BatteryEventInner::Timeout,
device_id: bs::device::DeviceId(0),
})
.await
{
Ok(_) => {
embedded_services::info!("FG recovered!");
Expand All @@ -232,10 +238,10 @@ async fn recover_state_machine() -> Result<(), ()> {
}
}

pub async fn run_app() {
pub async fn run_app(battery_service: &'static battery_service::Service) {
// Initialize battery state machine.
let mut retries = 5;
while let Err(e) = init_state_machine().await {
while let Err(e) = init_state_machine(battery_service).await {
retries -= 1;
if retries <= 0 {
embedded_services::error!("Failed to initialize Battery: {:?}", e);
Expand All @@ -249,21 +255,23 @@ pub async fn run_app() {
loop {
tokio::time::sleep(std::time::Duration::from_secs(1)).await;
if count.is_multiple_of(const { 60 * 60 * 60 }) {
if let Err(e) = battery_service::execute_event(battery_service::context::BatteryEvent {
event: battery_service::context::BatteryEventInner::PollStaticData,
device_id: bs::device::DeviceId(0),
})
.await
if let Err(e) = battery_service
.execute_event(battery_service::context::BatteryEvent {
event: battery_service::context::BatteryEventInner::PollStaticData,
device_id: bs::device::DeviceId(0),
})
.await
{
failures += 1;
embedded_services::error!("Fuel gauge static data error: {:#?}", e);
}
}
if let Err(e) = battery_service::execute_event(battery_service::context::BatteryEvent {
event: battery_service::context::BatteryEventInner::PollDynamicData,
device_id: bs::device::DeviceId(0),
})
.await
if let Err(e) = battery_service
.execute_event(battery_service::context::BatteryEvent {
event: battery_service::context::BatteryEventInner::PollDynamicData,
device_id: bs::device::DeviceId(0),
})
.await
{
failures += 1;
embedded_services::error!("Fuel gauge dynamic data error: {:#?}", e);
Expand All @@ -273,7 +281,7 @@ pub async fn run_app() {
failures = 0;
count = 0;
embedded_services::error!("FG: Too many errors, timing out and starting recovery...");
if recover_state_machine().await.is_err() {
if recover_state_machine(battery_service).await.is_err() {
embedded_services::error!("FG: Fatal error");
return;
}
Expand All @@ -288,11 +296,13 @@ async fn main() {
env_logger::builder().filter_level(log::LevelFilter::Info).init();
embedded_services::info!("host: battery example started");

static BATTERY_SERVICE: bs::Service = bs::Service::new();

let p = pico_de_gallo_hal::Hal::new();

let _ = embassy_futures::join::join(
tokio::spawn(run_app()),
tokio::spawn(init_and_run_service(p.i2c(), p.delay())),
tokio::spawn(run_app(&BATTERY_SERVICE)),
tokio::spawn(init_and_run_service(&BATTERY_SERVICE, p.i2c(), p.delay())),
)
.await;
}
Loading