From b28f39667da0a6b196105ae13e58839fcca23704 Mon Sep 17 00:00:00 2001
From: Alex Janka <alex@alexjanka.com>
Date: Mon, 13 Jan 2025 13:28:52 +1100
Subject: [PATCH] ccs: pull settings timer out of tristar

---
 .../src/controller.rs                         | 53 +++++++++++--------
 .../src/controller/tristar.rs                 | 14 -----
 2 files changed, 32 insertions(+), 35 deletions(-)

diff --git a/charge-controller-supervisor/src/controller.rs b/charge-controller-supervisor/src/controller.rs
index 89cf739..6c21b66 100644
--- a/charge-controller-supervisor/src/controller.rs
+++ b/charge-controller-supervisor/src/controller.rs
@@ -10,6 +10,7 @@ pub struct Controller {
     data: std::sync::Arc<ControllerData>,
     voltage_rx: Option<tokio::sync::mpsc::UnboundedReceiver<VoltageCommand>>,
     voltage_tx: Option<MultiTx>,
+    settings_last_read: Option<std::time::Instant>,
 }
 
 #[derive(Default, serde::Serialize, Clone)]
@@ -94,6 +95,7 @@ impl Controller {
                 data,
                 voltage_rx,
                 voltage_tx: None,
+                settings_last_read: None,
             },
             voltage_tx,
         ))
@@ -104,7 +106,7 @@ impl Controller {
     }
 
     pub async fn refresh(&mut self) -> eyre::Result<()> {
-        let (data, settings) = self.inner.refresh().await?;
+        let data = self.inner.refresh().await?;
 
         if let Some(tx) = self.voltage_tx.as_mut() {
             if crate::config::access_config()
@@ -123,8 +125,16 @@ impl Controller {
         }
 
         *self.data.write_state().await = Some(data);
-        if let Some(settings) = settings {
-            *self.data.write_settings().await = Some(settings);
+        if self.needs_new_settings() {
+            match self.inner.get_settings().await {
+                Ok(s) => {
+                    *self.data.write_settings().await = s;
+                    self.settings_last_read = Some(std::time::Instant::now());
+                }
+                Err(e) => {
+                    log::error!("couldn't read config from {}: {e:?}", self.name);
+                }
+            }
         }
 
         Ok(())
@@ -159,6 +169,12 @@ impl Controller {
             }
         }
     }
+
+    pub fn needs_new_settings(&self) -> bool {
+        self.settings_last_read.is_none_or(|t| {
+            std::time::Instant::now().duration_since(t) >= std::time::Duration::from_secs(60 * 60)
+        })
+    }
 }
 
 #[derive(Clone)]
@@ -174,38 +190,33 @@ impl MultiTx {
     }
 }
 
-#[expect(clippy::large_enum_variant)]
 pub enum ControllerInner {
     Pl(pl::Pli),
     Tristar(tristar::Tristar),
 }
 
 impl ControllerInner {
-    pub async fn refresh(&mut self) -> eyre::Result<(ControllerState, Option<ControllerSettings>)> {
+    pub async fn refresh(&mut self) -> eyre::Result<ControllerState> {
         match self {
             ControllerInner::Pl(pli) => {
                 let pl_data = pli.refresh().await?;
 
-                Ok((ControllerState::Pl(pl_data), None))
+                Ok(ControllerState::Pl(pl_data))
             }
             ControllerInner::Tristar(tristar) => {
-                let settings = if tristar.needs_new_settings() {
-                    match tristar.read_settings().await {
-                        Ok(v) => Some(ControllerSettings::Tristar(v)),
-                        Err(e) => {
-                            log::error!(
-                                "couldn't read config from tristar {}: {e:?}",
-                                tristar.name()
-                            );
-                            None
-                        }
-                    }
-                } else {
-                    None
-                };
                 let tristar_data = tristar.refresh().await?;
 
-                Ok((ControllerState::Tristar(tristar_data), settings))
+                Ok(ControllerState::Tristar(tristar_data))
+            }
+        }
+    }
+
+    pub async fn get_settings(&mut self) -> eyre::Result<Option<ControllerSettings>> {
+        match self {
+            ControllerInner::Pl(_) => Ok(None),
+            ControllerInner::Tristar(tristar) => {
+                let settings = tristar.read_settings().await?;
+                Ok(Some(ControllerSettings::Tristar(settings)))
             }
         }
     }
diff --git a/charge-controller-supervisor/src/controller/tristar.rs b/charge-controller-supervisor/src/controller/tristar.rs
index 5b533d6..a28ad13 100644
--- a/charge-controller-supervisor/src/controller/tristar.rs
+++ b/charge-controller-supervisor/src/controller/tristar.rs
@@ -54,7 +54,6 @@ pub struct Tristar {
     charge_state_gauges: ChargeStateGauges,
     consecutive_errors: usize,
     scaling: Scaling,
-    settings_last_read: Option<std::time::Instant>,
     transport_settings: crate::config::Transport,
 }
 
@@ -818,7 +817,6 @@ impl Tristar {
             charge_state_gauges,
             consecutive_errors: 0,
             scaling,
-            settings_last_read: None,
             transport_settings: transport.clone(),
         })
     }
@@ -906,17 +904,9 @@ impl Tristar {
 
         let charge = ChargeSettings::from_buf(&charge_data, &self.scaling)?;
 
-        self.settings_last_read = Some(std::time::Instant::now());
-
         Ok(TristarSettings { network, charge })
     }
 
-    pub fn needs_new_settings(&self) -> bool {
-        self.settings_last_read.is_none_or(|t| {
-            std::time::Instant::now().duration_since(t) >= std::time::Duration::from_secs(60 * 60)
-        })
-    }
-
     pub async fn set_target_voltage(&mut self, target_voltage: f64) -> eyre::Result<()> {
         let scaled_voltage: u16 = self.scale_voltage(target_voltage);
         self.modbus
@@ -931,10 +921,6 @@ impl Tristar {
         Ok(())
     }
 
-    pub fn name(&self) -> &str {
-        &self.friendly_name
-    }
-
     fn scale_voltage(&self, voltage: f64) -> u16 {
         self.scaling.inverse_voltage(voltage)
     }