refactor battery state

rust/src/hal/battery.rs

@@ -1,202 +1,157 @@
 use crate::to_string;
-use anyhow::bail;
+use anyhow::anyhow;
 use bq34z100::{Bq34Z100Error, Bq34z100g1, Bq34z100g1Driver};
 use embedded_hal_bus::i2c::MutexDevice;
 use esp_idf_hal::delay::Delay;
 use esp_idf_hal::i2c::{I2cDriver, I2cError};
 use measurements::Temperature;
 use serde::Serialize;
-use std::result::Result::Ok as OkStd;
 
 pub trait BatteryInteraction {
-    fn state_charge_percent(&mut self) -> anyhow::Result<u8>;
-    fn remaining_milli_ampere_hour(&mut self) -> anyhow::Result<u16>;
-    fn max_milli_ampere_hour(&mut self) -> anyhow::Result<u16>;
-    fn design_milli_ampere_hour(&mut self) -> anyhow::Result<u16>;
-    fn voltage_milli_volt(&mut self) -> anyhow::Result<u16>;
-    fn average_current_milli_ampere(&mut self) -> anyhow::Result<i16>;
-    fn cycle_count(&mut self) -> anyhow::Result<u16>;
-    fn state_health_percent(&mut self) -> anyhow::Result<u8>;
-    fn bat_temperature(&mut self) -> anyhow::Result<u16>;
-    fn get_battery_state(&mut self) -> String;
-}
-#[derive(Serialize)]
-pub struct BatteryState {
-    voltage_milli_volt: String,
-    current_milli_ampere: String,
-    cycle_count: String,
-    design_milli_ampere: String,
-    remaining_milli_ampere: String,
-    state_of_charge: String,
-    state_of_health: String,
-    temperature: String,
+    fn state_charge_percent(&mut self) -> Result<f32, BatteryError>;
+    fn remaining_milli_ampere_hour(&mut self) -> Result<u16, BatteryError>;
+    fn max_milli_ampere_hour(&mut self) -> Result<u16, BatteryError>;
+    fn design_milli_ampere_hour(&mut self) -> Result<u16, BatteryError>;
+    fn voltage_milli_volt(&mut self) -> Result<u16, BatteryError>;
+    fn average_current_milli_ampere(&mut self) -> Result<i16, BatteryError>;
+    fn cycle_count(&mut self) -> Result<u16, BatteryError>;
+    fn state_health_percent(&mut self) -> Result<u16, BatteryError>;
+    fn bat_temperature(&mut self) -> Result<u16, BatteryError>;
+    fn get_battery_state(&mut self) -> Result<BatteryState, BatteryError>;
 }
 
-pub enum BatteryMonitor<'a> {
-    Disabled {},
-    BQ34Z100G1 {
-        battery_driver: Bq34z100g1Driver<MutexDevice<'a, I2cDriver<'a>>, Delay>,
-    },
-    WchI2cSlave {},
+#[derive(Debug, Serialize)]
+pub struct BatteryInfo {
+    pub voltage_milli_volt: u16,
+    pub average_current_milli_ampere: i16,
+    pub cycle_count: u16,
+    pub design_milli_ampere_hour: u16,
+    pub remaining_milli_ampere_hour: u16,
+    pub state_of_charge: f32,
+    pub state_of_health: u16,
+    pub temperature: u16,
 }
 
-impl BatteryInteraction for BatteryMonitor<'_> {
-    fn state_charge_percent(&mut self) -> anyhow::Result<u8> {
-        match self {
-            BatteryMonitor::BQ34Z100G1 { battery_driver } => {
-                match battery_driver.state_of_charge() {
-                    OkStd(r) => anyhow::Ok(r),
-                    Err(err) => bail!("Error reading SoC {:?}", err),
-                }
-            }
-            BatteryMonitor::WchI2cSlave { .. } => {
-                bail!("Not implemented")
-            }
-            BatteryMonitor::Disabled {} => {
-                bail!("Battery monitor is disabled")
-            }
-        }
+#[derive(Debug, Serialize)]
+pub enum BatteryError {
+    NoBatteryMonitor,
+    CommunicationError(String),
+}
+
+impl From<Bq34Z100Error<esp_idf_hal::i2c::I2cError>> for BatteryError {
+    fn from(err: Bq34Z100Error<esp_idf_hal::i2c::I2cError>) -> Self {
+        BatteryError::CommunicationError(
+            anyhow!("failed to communicate with battery monitor: {:?}", err).to_string(),
+        )
+    }
+}
+
+#[derive(Debug, Serialize)]
+pub enum BatteryState {
+    Unknown,
+    Info(BatteryInfo),
+}
+
+/// If no battery monitor is installed this implementation will be used
+pub struct NoBatteryMonitor {}
+
+impl BatteryInteraction for NoBatteryMonitor {
+    fn state_charge_percent(&mut self) -> Result<f32, BatteryError> {
+        Err(BatteryError::NoBatteryMonitor)
     }
 
-    fn remaining_milli_ampere_hour(&mut self) -> anyhow::Result<u16> {
-        match self {
-            BatteryMonitor::BQ34Z100G1 { battery_driver } => {
-                match battery_driver.remaining_capacity() {
-                    OkStd(r) => anyhow::Ok(r),
-                    Err(err) => bail!("Error reading remaining_milli_ampere_hour {:?}", err),
-                }
-            }
-            BatteryMonitor::WchI2cSlave { .. } => {
-                bail!("Not implemented")
-            }
-            &mut BatteryMonitor::Disabled {} => {
-                bail!("Battery monitor is disabled")
-            }
-        }
-    }
-    fn max_milli_ampere_hour(&mut self) -> anyhow::Result<u16> {
-        match self {
-            BatteryMonitor::BQ34Z100G1 { battery_driver } => {
-                match battery_driver.full_charge_capacity() {
-                    OkStd(r) => anyhow::Ok(r),
-                    Err(err) => bail!("Error reading max_milli_ampere_hour {:?}", err),
-                }
-            }
-            BatteryMonitor::WchI2cSlave { .. } => {
-                bail!("Not implemented")
-            }
-            &mut BatteryMonitor::Disabled {} => {
-                bail!("Battery monitor is disabled")
-            }
-        }
-    }
-    fn design_milli_ampere_hour(&mut self) -> anyhow::Result<u16> {
-        match self {
-            BatteryMonitor::BQ34Z100G1 { battery_driver } => {
-                match battery_driver.design_capacity() {
-                    OkStd(r) => anyhow::Ok(r),
-                    Err(err) => bail!("Error reading design_milli_ampere_hour {:?}", err),
-                }
-            }
-            BatteryMonitor::WchI2cSlave { .. } => {
-                bail!("Not implemented")
-            }
-            &mut BatteryMonitor::Disabled {} => {
-                bail!("Battery monitor is disabled")
-            }
-        }
-    }
-    fn voltage_milli_volt(&mut self) -> anyhow::Result<u16> {
-        match self {
-            BatteryMonitor::BQ34Z100G1 { battery_driver } => match battery_driver.voltage() {
-                OkStd(r) => anyhow::Ok(r),
-                Err(err) => bail!("Error reading voltage_milli_volt {:?}", err),
-            },
-            BatteryMonitor::WchI2cSlave { .. } => {
-                bail!("Not implemented")
-            }
-            &mut BatteryMonitor::Disabled {} => {
-                bail!("Battery monitor is disabled")
-            }
-        }
-    }
-    fn average_current_milli_ampere(&mut self) -> anyhow::Result<i16> {
-        match self {
-            BatteryMonitor::BQ34Z100G1 { battery_driver } => {
-                match battery_driver.average_current() {
-                    OkStd(r) => anyhow::Ok(r),
-                    Err(err) => bail!("Error reading average_current_milli_ampere {:?}", err),
-                }
-            }
-            BatteryMonitor::WchI2cSlave { .. } => {
-                bail!("Not implemented")
-            }
-            &mut BatteryMonitor::Disabled {} => {
-                bail!("Battery monitor is disabled")
-            }
-        }
-    }
-    fn cycle_count(&mut self) -> anyhow::Result<u16> {
-        match self {
-            BatteryMonitor::BQ34Z100G1 { battery_driver } => match battery_driver.cycle_count() {
-                OkStd(r) => anyhow::Ok(r),
-                Err(err) => bail!("Error reading cycle_count {:?}", err),
-            },
-            BatteryMonitor::WchI2cSlave { .. } => {
-                bail!("Not implemented")
-            }
-            &mut BatteryMonitor::Disabled {} => {
-                bail!("Battery monitor is disabled")
-            }
-        }
-    }
-    fn state_health_percent(&mut self) -> anyhow::Result<u8> {
-        match self {
-            BatteryMonitor::BQ34Z100G1 { battery_driver } => {
-                match battery_driver.state_of_health() {
-                    OkStd(r) => anyhow::Ok(r as u8),
-                    Err(err) => bail!("Error reading state_health_percent {:?}", err),
-                }
-            }
-            BatteryMonitor::WchI2cSlave { .. } => {
-                bail!("Not implemented")
-            }
-            &mut BatteryMonitor::Disabled {} => {
-                bail!("Battery monitor is disabled")
-            }
-        }
-    }
-    fn bat_temperature(&mut self) -> anyhow::Result<u16> {
-        match self {
-            BatteryMonitor::BQ34Z100G1 { battery_driver } => match battery_driver.temperature() {
-                OkStd(r) => anyhow::Ok(r),
-                Err(err) => bail!("Error reading bat_temperature {:?}", err),
-            },
-            BatteryMonitor::WchI2cSlave { .. } => {
-                bail!("Not implemented")
-            }
-            &mut BatteryMonitor::Disabled {} => {
-                bail!("Battery monitor is disabled")
-            }
-        }
+    fn remaining_milli_ampere_hour(&mut self) -> Result<u16, BatteryError> {
+        Err(BatteryError::NoBatteryMonitor)
     }
 
-    fn get_battery_state(&mut self) -> String {
-        let bat = BatteryState {
-            voltage_milli_volt: to_string(self.voltage_milli_volt()),
-            current_milli_ampere: to_string(self.average_current_milli_ampere()),
-            cycle_count: to_string(self.cycle_count()),
-            design_milli_ampere: to_string(self.design_milli_ampere_hour()),
-            remaining_milli_ampere: to_string(self.remaining_milli_ampere_hour()),
-            state_of_charge: to_string(self.state_charge_percent()),
-            state_of_health: to_string(self.state_health_percent()),
-            temperature: to_string(self.bat_temperature()),
-        };
+    fn max_milli_ampere_hour(&mut self) -> Result<u16, BatteryError> {
+        Err(BatteryError::NoBatteryMonitor)
+    }
 
-        match serde_json::to_string(&bat) {
-            Ok(state) => state,
-            Err(err) => format!("{:?}", err).to_owned(),
-        }
+    fn design_milli_ampere_hour(&mut self) -> Result<u16, BatteryError> {
+        Err(BatteryError::NoBatteryMonitor)
+    }
+
+    fn voltage_milli_volt(&mut self) -> Result<u16, BatteryError> {
+        Err(BatteryError::NoBatteryMonitor)
+    }
+
+    fn average_current_milli_ampere(&mut self) -> Result<i16, BatteryError> {
+        Err(BatteryError::NoBatteryMonitor)
+    }
+
+    fn cycle_count(&mut self) -> Result<u16, BatteryError> {
+        Err(BatteryError::NoBatteryMonitor)
+    }
+
+    fn state_health_percent(&mut self) -> Result<u16, BatteryError> {
+        Err(BatteryError::NoBatteryMonitor)
+    }
+
+    fn bat_temperature(&mut self) -> Result<u16, BatteryError> {
+        Err(BatteryError::NoBatteryMonitor)
+    }
+
+    fn get_battery_state(&mut self) -> Result<BatteryState, BatteryError> {
+        Ok(BatteryState::Unknown)
+    }
+}
+
+//TODO implement this battery monitor kind once controller is complete
+pub struct WchI2cSlave {}
+
+pub struct BQ34Z100G1<'a> {
+    pub battery_driver: Bq34z100g1Driver<MutexDevice<'a, I2cDriver<'a>>, Delay>,
+}
+
+impl BatteryInteraction for BQ34Z100G1<'_> {
+    fn state_charge_percent(&mut self) -> Result<f32, BatteryError> {
+        Ok(self.battery_driver.state_of_charge().map(f32::from)?)
+    }
+
+    fn remaining_milli_ampere_hour(&mut self) -> Result<u16, BatteryError> {
+        Ok(self.battery_driver.remaining_capacity()?)
+    }
+
+    fn max_milli_ampere_hour(&mut self) -> Result<u16, BatteryError> {
+        Ok(self.battery_driver.full_charge_capacity()?)
+    }
+
+    fn design_milli_ampere_hour(&mut self) -> Result<u16, BatteryError> {
+        Ok(self.battery_driver.design_capacity()?)
+    }
+
+    fn voltage_milli_volt(&mut self) -> Result<u16, BatteryError> {
+        Ok(self.battery_driver.voltage()?)
+    }
+
+    fn average_current_milli_ampere(&mut self) -> Result<i16, BatteryError> {
+        Ok(self.battery_driver.average_current()?)
+    }
+
+    fn cycle_count(&mut self) -> Result<u16, BatteryError> {
+        Ok(self.battery_driver.cycle_count()?)
+    }
+
+    fn state_health_percent(&mut self) -> Result<u16, BatteryError> {
+        Ok(self.battery_driver.state_of_health()?)
+    }
+
+    fn bat_temperature(&mut self) -> Result<u16, BatteryError> {
+        Ok(self.battery_driver.temperature()?)
+    }
+
+    fn get_battery_state(&mut self) -> Result<BatteryState, BatteryError> {
+        Ok(BatteryState::Info(BatteryInfo {
+            voltage_milli_volt: self.voltage_milli_volt()?,
+            average_current_milli_ampere: self.average_current_milli_ampere()?,
+            cycle_count: self.cycle_count()?,
+            design_milli_ampere_hour: self.design_milli_ampere_hour()?,
+            remaining_milli_ampere_hour: self.remaining_milli_ampere_hour()?,
+            state_of_charge: self.state_charge_percent()?,
+            state_of_health: self.state_health_percent()?,
+            temperature: self.bat_temperature()?,
+        }))
     }
 }
 

rust/src/hal/initial_hal.rs

@@ -1,5 +1,5 @@
 use crate::config::{BoardHardware, PlantControllerConfig};
-use crate::hal::battery::{BatteryInteraction, BatteryMonitor};
+use crate::hal::battery::{BatteryInteraction, NoBatteryMonitor};
 use crate::hal::esp::ESP;
 use crate::hal::{deep_sleep, BackupHeader, BoardInteraction, FreePeripherals, Sensor};
 use anyhow::{bail, Result};
@@ -32,7 +32,7 @@ pub(crate) fn create_initial_board(
         general_fault,
         config,
         esp,
-        battery: Box::new(BatteryMonitor::Disabled {}),
+        battery: Box::new(NoBatteryMonitor{}),
     };
     Ok(Box::new(v))
 }

rust/src/hal/mod.rs

@@ -4,6 +4,7 @@ mod initial_hal;
 mod v3_hal;
 mod v4_hal;
 
+use battery::BQ34Z100G1;
 use bq34z100::Bq34z100g1Driver;
 
 use crate::log::LogMessage;
@@ -35,7 +36,7 @@ use std::sync::Mutex;
 use std::time::Duration;
 
 use crate::config::{BatteryBoardVersion, BoardVersion, PlantControllerConfig};
-use crate::hal::battery::{print_battery_bq34z100, BatteryInteraction, BatteryMonitor};
+use crate::hal::battery::{print_battery_bq34z100, BatteryInteraction, NoBatteryMonitor};
 use crate::hal::esp::ESP;
 use crate::hal::initial_hal::Initial;
 use crate::log::log;
@@ -280,8 +281,8 @@ impl PlantHal {
 
         let hal = match config {
             Result::Ok(config) => {
-                let battery_monitor: BatteryMonitor = match config.hardware.battery {
-                    BatteryBoardVersion::Disabled => BatteryMonitor::Disabled {},
+                let battery_interaction: Box<dyn BatteryInteraction + Send> = match config.hardware.battery {
+                    BatteryBoardVersion::Disabled => Box::new(NoBatteryMonitor{}),
                     BatteryBoardVersion::BQ34Z100G1 => {
                         let mut battery_driver = Bq34z100g1Driver {
                             i2c: MutexDevice::new(&I2C_DRIVER),
@@ -301,12 +302,13 @@ impl PlantHal {
                                 );
                             }
                         }
-                        BatteryMonitor::BQ34Z100G1 { battery_driver }
+                        Box::new(BQ34Z100G1 { battery_driver })
                     }
-                    BatteryBoardVersion::WchI2cSlave => BatteryMonitor::WchI2cSlave {},
+                    BatteryBoardVersion::WchI2cSlave => {
+                        // TODO use correct implementation once availible
+                        Box::new(NoBatteryMonitor{})
+                    },
                 };
-                let battery_interaction =
-                    Box::new(battery_monitor) as Box<dyn BatteryInteraction + Send>;
 
                 let board_hal: Box<dyn BoardInteraction + Send> = match config.hardware.board {
                     BoardVersion::INITIAL => {

rust/src/hal/v3_hal.rs

@@ -1,5 +1,5 @@
 use crate::config::PlantControllerConfig;
-use crate::hal::battery::{BatteryInteraction, BatteryMonitor};
+use crate::hal::battery::BatteryInteraction;
 use crate::hal::esp::ESP;
 use crate::hal::{
     deep_sleep, BackupHeader, BoardInteraction, FreePeripherals, Sensor, V3Constants, I2C_DRIVER,

rust/src/hal/v4_hal.rs

@@ -1,5 +1,5 @@
 use crate::config::PlantControllerConfig;
-use crate::hal::battery::{BatteryInteraction, BatteryMonitor};
+use crate::hal::battery::BatteryInteraction;
 use crate::hal::esp::ESP;
 use crate::hal::{
     deep_sleep, BackupHeader, BoardInteraction, FreePeripherals, Sensor, I2C_DRIVER, PLANT_COUNT,