Update battery-service device object to use Mutexes and SyncCell instead of Cell

Author: tullom

battery-service/src/device.rs

@@ -1,8 +1,6 @@
-use core::cell::Cell;
-
-use embassy_sync::channel::Channel;
+use embassy_sync::{channel::Channel, mutex::Mutex};
 use embassy_time::Duration;
-use embedded_services::{GlobalRawMutex, Node, NodeContainer};
+use embedded_services::{GlobalRawMutex, Node, NodeContainer, SyncCell};
 
 #[derive(Debug, Clone, Copy)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
@@ -116,9 +114,9 @@ pub struct Device {
     id: DeviceId,
     command: Channel<GlobalRawMutex, Command, 1>,
     response: Channel<GlobalRawMutex, Response, 1>,
-    dynamic_battery_cache: Cell<DynamicBatteryMsgs>,
-    static_battery_cache: Cell<StaticBatteryMsgs>,
-    timeout: Cell<Duration>,
+    dynamic_battery_cache: Mutex<GlobalRawMutex, DynamicBatteryMsgs>,
+    static_battery_cache: Mutex<GlobalRawMutex, StaticBatteryMsgs>,
+    timeout: SyncCell<Duration>,
 }
 
 impl Device {
@@ -128,9 +126,9 @@ impl Device {
             id,
             command: Channel::new(),
             response: Channel::new(),
-            dynamic_battery_cache: Cell::default(),
-            static_battery_cache: Cell::default(),
-            timeout: Cell::new(Duration::from_secs(60)),
+            dynamic_battery_cache: Mutex::default(),
+            static_battery_cache: Mutex::default(),
+            timeout: SyncCell::new(Duration::from_secs(60)),
         }
     }
 
@@ -166,23 +164,23 @@ impl Device {
     }
 
     /// Set dynamic battery cache with updated values.
-    pub fn set_dynamic_battery_cache(&self, new_values: DynamicBatteryMsgs) {
-        self.dynamic_battery_cache.set(new_values);
+    pub async fn set_dynamic_battery_cache(&self, new_values: DynamicBatteryMsgs) {
+        *self.dynamic_battery_cache.lock().await = new_values;
     }
 
     /// Set static battery cache with updated values.
-    pub fn set_static_battery_cache(&self, new_values: StaticBatteryMsgs) {
-        self.static_battery_cache.set(new_values);
+    pub async fn set_static_battery_cache(&self, new_values: StaticBatteryMsgs) {
+        *self.static_battery_cache.lock().await = new_values;
     }
 
     /// Get dynamic battery cache.
-    pub fn get_dynamic_battery_cache(&self) -> DynamicBatteryMsgs {
-        self.dynamic_battery_cache.get()
+    pub async fn get_dynamic_battery_cache(&self) -> DynamicBatteryMsgs {
+        *self.dynamic_battery_cache.lock().await
     }
 
     /// Get static battery cache.
-    pub fn get_static_battery_cache(&self) -> StaticBatteryMsgs {
-        self.static_battery_cache.get()
+    pub async fn get_static_battery_cache(&self) -> StaticBatteryMsgs {
+        *self.static_battery_cache.lock().await
     }
 
     /// Set device timeout.

battery-service/src/wrapper.rs

@@ -76,7 +76,7 @@ impl<'a, C: Controller> Wrapper<'a, C> {
             },
             Command::UpdateStaticCache => match controller.get_static_data().await {
                 Ok(static_data) => {
-                    device.set_static_battery_cache(static_data);
+                    device.set_static_battery_cache(static_data).await;
                     device
                         .send_response(Ok(crate::device::InternalResponse::Complete))
                         .await;
@@ -88,7 +88,7 @@ impl<'a, C: Controller> Wrapper<'a, C> {
             },
             Command::UpdateDynamicCache => match controller.get_dynamic_data().await {
                 Ok(dynamic_data) => {
-                    device.set_dynamic_battery_cache(dynamic_data);
+                    device.set_dynamic_battery_cache(dynamic_data).await;
                     device
                         .send_response(Ok(crate::device::InternalResponse::Complete))
                         .await;

examples/rt633/src/bin/espi_battery.rs

@@ -217,7 +217,7 @@ async fn battery_publish_task(fg_device: &'static Device) {
     loop {
         Timer::after_secs(1).await;
         // Get dynamic cache
-        let cache = fg_device.get_dynamic_battery_cache();
+        let cache = fg_device.get_dynamic_battery_cache().await;
 
         // Send cache data to eSpi service
         battery_service::comms_send(

examples/std/src/bin/battery.rs

@@ -18,18 +18,17 @@ use static_cell::StaticCell;
 mod espi_service {
     use battery_service::context::{BatteryEvent, BatteryEventInner};
     use battery_service::device::DeviceId;
-    use embassy_sync::blocking_mutex::raw::NoopRawMutex;
     use embassy_sync::once_lock::OnceLock;
     use embassy_sync::signal::Signal;
     use embassy_time::Timer;
     use embedded_services::comms::{self, EndpointID, External};
     use embedded_services::ec_type::message::BatteryMessage;
-    use embedded_services::error;
+    use embedded_services::{GlobalRawMutex, error};
     use log::info;
 
     pub struct Service {
         endpoint: comms::Endpoint,
-        _signal: Signal<NoopRawMutex, BatteryMessage>,
+        _signal: Signal<GlobalRawMutex, BatteryMessage>,
     }
 
     impl Service {

examples/std/src/bin/type_c/service.rs

@@ -4,11 +4,11 @@ use embassy_time::Timer;
 use embedded_cfu_protocol::protocol_definitions::{FwUpdateOfferResponse, HostToken};
 use embedded_services::comms;
 use embedded_services::power::{self, policy};
-use embedded_services::type_c::{controller, ControllerId};
-use embedded_usb_pd::type_c::Current;
+use embedded_services::type_c::{ControllerId, controller};
 use embedded_usb_pd::Error;
 use embedded_usb_pd::GlobalPortId;
 use embedded_usb_pd::PortId as LocalPortId;
+use embedded_usb_pd::type_c::Current;
 use log::*;
 use static_cell::StaticCell;
 
@@ -19,30 +19,33 @@ const POWER0: power::policy::DeviceId = power::policy::DeviceId(0);
 mod test_controller {
     use std::cell::Cell;
 
-    use embassy_sync::{blocking_mutex::raw::NoopRawMutex, signal::Signal};
-    use embedded_services::type_c::{
-        controller::{Contract, ControllerStatus, PortStatus, RetimerFwUpdateState},
-        event::PortEventKind,
+    use embassy_sync::{mutex::Mutex, signal::Signal};
+    use embedded_services::{
+        GlobalRawMutex,
+        type_c::{
+            controller::{Contract, ControllerStatus, PortStatus, RetimerFwUpdateState},
+            event::PortEventKind,
+        },
     };
 
     use super::*;
 
     pub struct ControllerState {
-        events: Signal<NoopRawMutex, PortEventKind>,
-        status: Cell<PortStatus>,
+        events: Signal<GlobalRawMutex, PortEventKind>,
+        status: Mutex<GlobalRawMutex, PortStatus>,
     }
 
     impl ControllerState {
         pub fn new() -> Self {
             Self {
                 events: Signal::new(),
-                status: Cell::new(PortStatus::default()),
+                status: Mutex::new(PortStatus::default()),
             }
         }
 
         /// Simulate a connection
-        pub fn connect(&self, _contract: Contract) {
-            self.status.set(PortStatus::new());
+        pub async fn connect(&self, _contract: Contract) {
+            *self.status.lock().await = PortStatus::new();
 
             let mut events = PortEventKind::none();
             events.set_plug_inserted_or_removed(true);
@@ -51,22 +54,22 @@ mod test_controller {
         }
 
         /// Simulate a sink connecting
-        pub fn connect_sink(&self, current: Current) {
-            self.connect(Contract::Sink(current.into()));
+        pub async fn connect_sink(&self, current: Current) {
+            self.connect(Contract::Sink(current.into())).await;
         }
 
         /// Simulate a disconnection
-        pub fn disconnect(&self) {
-            self.status.set(PortStatus::default());
+        pub async fn disconnect(&self) {
+            *self.status.lock().await = PortStatus::default();
 
             let mut events = PortEventKind::none();
             events.set_plug_inserted_or_removed(true);
             self.events.signal(events);
         }
 
         /// Simulate a debug accessory source connecting
-        pub fn connect_debug_accessory_source(&self, _current: Current) {
-            self.status.set(PortStatus::new());
+        pub async fn connect_debug_accessory_source(&self, _current: Current) {
+            *self.status.lock().await = PortStatus::new();
 
             let mut events = PortEventKind::none();
             events.set_plug_inserted_or_removed(true);
@@ -112,8 +115,8 @@ mod test_controller {
         }
 
         async fn get_port_status(&mut self, _port: LocalPortId) -> Result<PortStatus, Error<Self::BusError>> {
-            debug!("Get port status: {:#?}", self.state.status.get());
-            Ok(self.state.status.get())
+            debug!("Get port status: {:#?}", *self.state.status.lock().await);
+            Ok(*self.state.status.lock().await)
         }
 
         async fn enable_sink_path(&mut self, _port: LocalPortId, enable: bool) -> Result<(), Error<Self::BusError>> {
@@ -267,19 +270,19 @@ async fn task(spawner: Spawner) {
     Timer::after_secs(1).await;
 
     info!("Simulating connection");
-    state.connect_sink(Current::UsbDefault);
+    state.connect_sink(Current::UsbDefault).await;
     Timer::after_millis(250).await;
 
     info!("Simulating disconnection");
-    state.disconnect();
+    state.disconnect().await;
     Timer::after_millis(250).await;
 
     info!("Simulating debug accessory connection");
-    state.connect_debug_accessory_source(Current::UsbDefault);
+    state.connect_debug_accessory_source(Current::UsbDefault).await;
     Timer::after_millis(250).await;
 
     info!("Simulating debug accessory disconnection");
-    state.disconnect();
+    state.disconnect().await;
     Timer::after_millis(250).await;
 }