diff --git a/Cargo.lock b/Cargo.lock index dcab087b..538958ef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -168,6 +168,7 @@ dependencies = [ "embedded-hal 1.0.0", "embedded-hal-async", "embedded-services", + "heapless", "log", "mctp-rs", "zerocopy", diff --git a/battery-service/Cargo.toml b/battery-service/Cargo.toml index 2e12930a..87d36baf 100644 --- a/battery-service/Cargo.toml +++ b/battery-service/Cargo.toml @@ -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 = [] diff --git a/battery-service/src/context.rs b/battery-service/src/context.rs index 73edb63d..b43a03fe 100644 --- a/battery-service/src/context.rs +++ b/battery-service/src/context.rs @@ -402,8 +402,11 @@ impl Context { } } - pub(super) async fn process_acpi_cmd(&self, acpi_msg: &AcpiBatteryRequest) { - let response: Result = match *acpi_msg { + pub(super) async fn process_acpi_cmd( + &self, + acpi_msg: &AcpiBatteryRequest, + ) -> Result { + 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, @@ -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> { diff --git a/battery-service/src/lib.rs b/battery-service/src/lib.rs index 6a94e11a..993f0ff8 100644 --- a/battery-service/src/lib.rs +++ b/battery-service/src/lib.rs @@ -7,7 +7,7 @@ use context::BatteryEvent; use embassy_futures::select::select; use embedded_services::{ comms::{self, EndpointID}, - trace, + error, trace, }; mod acpi; @@ -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) } @@ -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)] @@ -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 -} diff --git a/battery-service/src/task.rs b/battery-service/src/task.rs index 9e63bc3b..8f38285a 100644 --- a/battery-service/src/task.rs +++ b/battery-service/src/task.rs @@ -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 { + DeviceRegistrationFailed(heapless::Vec), + CommsRegistrationFailed, +} /// Battery service task. -pub async fn task() { +pub async fn task( + service: &'static Service, + devices: [&'static Device; N], +) -> Result<(), InitError> { 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; } } diff --git a/examples/pico-de-gallo/Cargo.lock b/examples/pico-de-gallo/Cargo.lock index ed341cdb..6bfd1029 100644 --- a/examples/pico-de-gallo/Cargo.lock +++ b/examples/pico-de-gallo/Cargo.lock @@ -178,6 +178,7 @@ dependencies = [ "embedded-hal 1.0.0", "embedded-hal-async", "embedded-services", + "heapless 0.8.0", "log", "mctp-rs", "zerocopy", diff --git a/examples/pico-de-gallo/src/bin/battery.rs b/examples/pico-de-gallo/src/bin/battery.rs index 64ee1f39..e79b2edd 100644 --- a/examples/pico-de-gallo/src/bin/battery.rs +++ b/examples/pico-de-gallo/src/bin/battery.rs @@ -151,7 +151,11 @@ 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; @@ -159,8 +163,6 @@ async fn init_and_run_service(i2c: pico_de_gallo_hal::I2c, delay: pico_de_gallo_ static BATTERY_WRAPPER: StaticCell> = 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 { @@ -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!"); @@ -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); @@ -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); @@ -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; } @@ -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; } diff --git a/examples/rt633/Cargo.lock b/examples/rt633/Cargo.lock index 0695fa9b..59a783c8 100644 --- a/examples/rt633/Cargo.lock +++ b/examples/rt633/Cargo.lock @@ -111,6 +111,7 @@ dependencies = [ "embedded-hal 1.0.0", "embedded-hal-async", "embedded-services", + "heapless", "mctp-rs", "zerocopy", ] diff --git a/examples/rt633/src/bin/espi_battery.rs b/examples/rt633/src/bin/espi_battery.rs index e19f1644..eb735976 100644 --- a/examples/rt633/src/bin/espi_battery.rs +++ b/examples/rt633/src/bin/espi_battery.rs @@ -100,19 +100,20 @@ unsafe extern "C" { } #[embassy_executor::task] -async fn battery_publish_task(fg_device: &'static Device) { +async fn battery_publish_task(battery_service: &'static battery_service::Service, fg_device: &'static Device) { loop { Timer::after_secs(1).await; // Get dynamic cache let cache = fg_device.get_dynamic_battery_cache().await; // Send cache data to eSpi service - battery_service::comms_send( - embedded_services::comms::EndpointID::External(embedded_services::comms::External::Host), - &embedded_services::ec_type::message::BatteryMessage::CycleCount(cache.cycle_count.into()), - ) - .await - .unwrap(); + battery_service + .comms_send( + embedded_services::comms::EndpointID::External(embedded_services::comms::External::Host), + &embedded_services::ec_type::message::BatteryMessage::CycleCount(cache.cycle_count.into()), + ) + .await + .unwrap(); } } @@ -131,9 +132,13 @@ async fn espi_service_task(espi: embassy_imxrt::espi::Espi<'static>, memory_map_ } #[embassy_executor::task] -async fn battery_service_task() -> ! { - battery_service::task::task().await; - unreachable!() +async fn battery_service_task( + service: &'static battery_service::Service, + device: [&'static battery_service::device::Device; 1], +) { + if battery_service::task::task(service, device).await.is_err() { + error!("Failed to start battery service") + } } #[embassy_executor::main] @@ -210,6 +215,8 @@ async fn main(spawner: Spawner) { let fg_bus = I2cDevice::new(i2c_bus_fg); + static BATTERY_SERVICE: battery_service::Service = battery_service::Service::new(); + let fg = FG_DEVICE.init(Device::new(DeviceId(0))); let wrap = Wrapper::new( @@ -220,17 +227,16 @@ async fn main(spawner: Spawner) { ); spawner.must_spawn(wrapper_task(wrap)); - spawner.must_spawn(battery_service_task()); + spawner.must_spawn(battery_service_task(&BATTERY_SERVICE, [fg])); - battery_service::register_fuel_gauge(fg).unwrap(); + spawner.must_spawn(battery_publish_task(&BATTERY_SERVICE, fg)); - spawner.must_spawn(battery_publish_task(fg)); - - if let Err(e) = battery_service::execute_event(BatteryEvent { - device_id: DeviceId(0), - event: battery_service::context::BatteryEventInner::DoInit, - }) - .await + if let Err(e) = BATTERY_SERVICE + .execute_event(BatteryEvent { + device_id: DeviceId(0), + event: battery_service::context::BatteryEventInner::DoInit, + }) + .await { error!("Error initializing fuel gauge, error: {:?}", e); } @@ -249,11 +255,12 @@ async fn main(spawner: Spawner) { info!("Memory map contents: {:?}", data[..64]); - if let Err(e) = battery_service::execute_event(BatteryEvent { - device_id: DeviceId(0), - event: battery_service::context::BatteryEventInner::PollDynamicData, - }) - .await + if let Err(e) = BATTERY_SERVICE + .execute_event(BatteryEvent { + device_id: DeviceId(0), + event: battery_service::context::BatteryEventInner::PollDynamicData, + }) + .await { error!("Error getting dynamic fuel gauge data, error: {:?}", e); } diff --git a/examples/std/Cargo.lock b/examples/std/Cargo.lock index 818a34e4..7bd330cb 100644 --- a/examples/std/Cargo.lock +++ b/examples/std/Cargo.lock @@ -124,6 +124,7 @@ dependencies = [ "embedded-hal 1.0.0", "embedded-hal-async", "embedded-services", + "heapless", "log", "mctp-rs", "zerocopy", diff --git a/examples/std/src/bin/battery.rs b/examples/std/src/bin/battery.rs index 4ff2430f..605416e1 100644 --- a/examples/std/src/bin/battery.rs +++ b/examples/std/src/bin/battery.rs @@ -12,7 +12,7 @@ use embedded_batteries_async::smart_battery::{ ManufactureDate, MilliAmpsSigned, Minutes, Percent, SmartBattery, SpecificationInfoFields, }; use embedded_hal_mock::eh1::i2c::Mock; -use embedded_services::info; +use embedded_services::{error, info}; mod espi_service { use battery_service::context::{BatteryEvent, BatteryEventInner}; @@ -67,7 +67,7 @@ mod espi_service { } #[embassy_executor::task] - pub async fn task() { + pub async fn task(battery_service: &'static battery_service::Service) { let espi_service = ESPI_SERVICE.get().await; espi_service @@ -81,14 +81,9 @@ mod espi_service { ) .await .unwrap(); - info!("Sent init request"); - match battery_service::wait_for_battery_response().await { - Ok(_) => { - info!("Init request succeeded!") - } - Err(e) => { - error!("Init request failed with {:?}", e); - } + + if let Err(e) = battery_service.wait_for_battery_response().await { + error!("Init request failed with {:?}", e); } Timer::after_secs(5).await; @@ -104,6 +99,10 @@ mod espi_service { .await .unwrap(); + if let Err(e) = battery_service.wait_for_battery_response().await { + error!("static data request failed with {:?}", e); + } + loop { espi_service .endpoint @@ -116,14 +115,9 @@ mod espi_service { ) .await .unwrap(); - info!("Sent dynamic data request"); - match battery_service::wait_for_battery_response().await { - Ok(_) => { - info!("dynamic data request succeeded!") - } - Err(e) => { - error!("dynamic data request failed with {:?}", e); - } + + if let Err(e) = battery_service.wait_for_battery_response().await { + error!("dynamic data request failed with {:?}", e); } Timer::after_secs(5).await; } @@ -470,14 +464,18 @@ async fn wrapper_task(wrapper: Wrapper<'static, FuelGaugeController>) { } #[embassy_executor::task] -async fn battery_service_task() -> ! { - battery_service::task::task().await; - unreachable!() +async fn battery_service_task( + service: &'static battery_service::Service, + device: [&'static battery_service::device::Device; 1], +) { + if let Err(e) = battery_service::task::task(service, device).await { + error!("Failed to start battery service with error {:?}", e) + } } #[embassy_executor::main] async fn main(spawner: Spawner) { - env_logger::builder().filter_level(log::LevelFilter::Trace).init(); + env_logger::builder().filter_level(log::LevelFilter::Info).init(); let expectations = vec![]; @@ -485,6 +483,8 @@ async fn main(spawner: Spawner) { let dev = DEV.get_or_init(|| Device::new(DeviceId(0))); + static SERVICE: battery_service::Service = battery_service::Service::new(); + let wrap = Wrapper::new( dev, FuelGaugeController { @@ -498,9 +498,7 @@ async fn main(spawner: Spawner) { espi_service::init().await; info!("espi service init'd"); - battery_service::register_fuel_gauge(dev).unwrap(); - - spawner.must_spawn(espi_service::task()); + spawner.must_spawn(espi_service::task(&SERVICE)); spawner.must_spawn(wrapper_task(wrap)); - spawner.must_spawn(battery_service_task()); + spawner.must_spawn(battery_service_task(&SERVICE, [dev])); }