From b4601d4b7f91d7e0ecc09c2c372fa1dd54590b84 Mon Sep 17 00:00:00 2001
From: Stuart Pittaway <1201909+stuartpittaway@users.noreply.github.com>
Date: Tue, 21 Feb 2023 08:52:34 +0000
Subject: [PATCH] Additional changes for #175 - Victron don't use zero volt for
 CVL

---
 ESPController/include/Rules.h        | 10 +++++-----
 ESPController/include/defines.h      |  1 +
 ESPController/src/victron_canbus.cpp | 30 ++++++++++++++++------------
 3 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/ESPController/include/Rules.h b/ESPController/include/Rules.h
index 892ad24a..29a4d1bc 100644
--- a/ESPController/include/Rules.h
+++ b/ESPController/include/Rules.h
@@ -72,7 +72,7 @@ class Rules
     // Number of TRUE values in array rule_outcome
     uint8_t active_rule_count;
 
-    // Actual bank voltage reported by the modules (sum of voltage reported by modules)
+    // Actual bank voltage reported by the modules (sum of voltage reported by modules) (millivolts)
     uint32_t bankvoltage[maximum_number_of_banks];
     // As above, but each voltage reading limited to "cellmaxmv" setting (used for charge voltage calc)
     uint32_t limitedbankvoltage[maximum_number_of_banks];
@@ -83,14 +83,14 @@ class Rules
     // Number of modules who have reported zero volts (bad!)
     uint8_t zeroVoltageModuleCount;
 
-    // Highest pack voltage
+    // Highest pack voltage (millivolts)
     uint32_t highestBankVoltage;
-    // Lowest pack voltage
+    // Lowest pack voltage (millivolts)
     uint32_t lowestBankVoltage;
 
-    // Highest cell voltage in the whole system
+    // Highest cell voltage in the whole system (millivolts)
     uint16_t highestCellVoltage;
-    // Lowest cell voltage in the whole system
+    // Lowest cell voltage in the whole system (millivolts)
     uint16_t lowestCellVoltage;
 
     // Highest cell voltage range (mV) across all banks
diff --git a/ESPController/include/defines.h b/ESPController/include/defines.h
index 1fd28c20..a0783af8 100644
--- a/ESPController/include/defines.h
+++ b/ESPController/include/defines.h
@@ -164,6 +164,7 @@ struct diybms_eeprom_settings
   // Maximum charge current - scale 0.1
   uint16_t chargecurrent;
   uint16_t dischargecurrent;
+  // Scale 0.1
   uint16_t dischargevolt;
   int16_t cellminmv;
   int16_t cellmaxmv;
diff --git a/ESPController/src/victron_canbus.cpp b/ESPController/src/victron_canbus.cpp
index 5c93e4c1..cf991480 100644
--- a/ESPController/src/victron_canbus.cpp
+++ b/ESPController/src/victron_canbus.cpp
@@ -4,12 +4,11 @@
  )(_) )_)(_  \  /  ) _ < )    ( \__ \
 (____/(____) (__) (____/(_/\/\_)(___/
 
-  (c) 2021 Stuart Pittaway
+(c) 2021-2023 Stuart Pittaway
 
 This code communicates with VICTRON CERBO GX style devices using CANBUS @ 500kbps and 11 bit addresses.
 
 The code supports the VICTRON CAN BUS BMS style messages.
-
 */
 
 #define USE_ESP_IDF_LOG 1
@@ -127,29 +126,31 @@ void victron_message_351()
 
   struct data351
   {
-    // CVL
+    // CVL - 0.1V scale
     uint16_t chargevoltagelimit;
-    // CCL
+    // CCL - 0.1A scale
     int16_t maxchargecurrent;
-    // DCL
+    // DCL - 0.1A scale
     int16_t maxdischargecurrent;
     // Not currently used by Victron
+    // 0.1V scale
     uint16_t dischargevoltage;
   };
 
   data351 data;
 
-
-  //  Defaults (do nothing)
-  data.chargevoltagelimit = 0;
+  // Defaults (do nothing)
+  // Don't use zero for voltage - this indicates to Victron an over voltage situation, and Victron gear attempts to dump
+  // the whole battery contents!  (feedback from end users)
+  data.chargevoltagelimit = rules.lowestBankVoltage / 100;
   data.maxchargecurrent = 0;
 
   if (rules.IsChargeAllowed(&mysettings))
   {
     if (rules.numberOfBalancingModules > 0 && mysettings.stopchargebalance == true)
     {
-      // Balancing, stop charge, allow discharge
-      data.chargevoltagelimit = 0;
+      // Balancing, stop charge
+      data.chargevoltagelimit = rules.lowestBankVoltage / 100;
       data.maxchargecurrent = 0;
     }
     else
@@ -160,6 +161,7 @@ void victron_message_351()
     }
   }
 
+  // Discharge settings....
   data.maxdischargecurrent = 0;
   data.dischargevoltage = mysettings.dischargevolt;
 
@@ -206,6 +208,7 @@ void victron_message_356()
   data356 data;
 
   // Use highest bank voltage calculated by controller and modules
+  // Scale 0.01V
   data.voltage = rules.highestBankVoltage / 10;
 
   // If current shunt is installed, use the voltage from that as it should be more accurate
@@ -218,10 +221,11 @@ void victron_message_356()
   // If current shunt is installed, use it
   if (mysettings.currentMonitoringEnabled && currentMonitor.validReadings)
   {
+    // Scale 0.1A
     data.current = currentMonitor.modbus.current * 10;
   }
 
-  // Temperature 0.1 C using external temperature sensor
+  // Temperature 0.1C using external temperature sensor
   if (rules.moduleHasExternalTempSensor)
   {
     data.temperature = (int16_t)rules.highestExternalTemp * (int16_t)10;
@@ -361,8 +365,8 @@ void victron_message_35a()
   // ESP_LOGI(TAG, "numberOfBalancingModules=%u", rules.numberOfBalancingModules);
 
   // 7 (bit 0+1) Cell imbalance warning
-  //data.byte7 |= (rules.numberOfBalancingModules > 0 ? BIT01_ALARM : BIT01_OK);
-  
+  // data.byte7 |= (rules.numberOfBalancingModules > 0 ? BIT01_ALARM : BIT01_OK);
+
   // 7 (bit 2+3) System status (online/offline) [1]
   data.byte7 |= ((_controller_state != ControllerState::Running) ? BIT23_ALARM : BIT23_OK);
   // 7 (rest) Reserved