From 26268479fba465687c85123b982b7f0ef5324aba Mon Sep 17 00:00:00 2001
From: Thomas Basler <thomas@familie-basler.net>
Date: Wed, 24 Aug 2022 02:53:34 +0200
Subject: [PATCH] Fix #96 and #95 : Reworked handling of inverter editing

---
 lib/Hoymiles/src/Hoymiles.cpp      |  9 ++++++---
 lib/Hoymiles/src/Hoymiles.h        |  2 +-
 lib/Hoymiles/src/HoymilesRadio.cpp |  5 +++++
 src/WebApi_inverter.cpp            | 22 +++++++++++++++-------
 4 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/lib/Hoymiles/src/Hoymiles.cpp b/lib/Hoymiles/src/Hoymiles.cpp
index aef62059..5ec88dc2 100644
--- a/lib/Hoymiles/src/Hoymiles.cpp
+++ b/lib/Hoymiles/src/Hoymiles.cpp
@@ -110,10 +110,13 @@ std::shared_ptr<InverterAbstract> HoymilesClass::getInverterByFragment(fragment_
     return nullptr;
 }
 
-void HoymilesClass::removeInverterByPos(uint8_t pos)
+void HoymilesClass::removeInverterBySerial(uint64_t serial)
 {
-    if (pos < _inverters.size()) {
-        _inverters.erase(_inverters.begin() + pos);
+    for (uint8_t i = 0; i < _inverters.size(); i++) {
+        if (_inverters[i]->serial() == serial) {
+            _inverters.erase(_inverters.begin() + i);
+            return;
+        }
     }
 }
 
diff --git a/lib/Hoymiles/src/Hoymiles.h b/lib/Hoymiles/src/Hoymiles.h
index 4064690f..57979553 100644
--- a/lib/Hoymiles/src/Hoymiles.h
+++ b/lib/Hoymiles/src/Hoymiles.h
@@ -16,7 +16,7 @@ public:
     std::shared_ptr<InverterAbstract> getInverterByPos(uint8_t pos);
     std::shared_ptr<InverterAbstract> getInverterBySerial(uint64_t serial);
     std::shared_ptr<InverterAbstract> getInverterByFragment(fragment_t* fragment);
-    void removeInverterByPos(uint8_t pos);
+    void removeInverterBySerial(uint64_t serial);
     size_t getNumInverters();
 
     HoymilesRadio* getRadio();
diff --git a/lib/Hoymiles/src/HoymilesRadio.cpp b/lib/Hoymiles/src/HoymilesRadio.cpp
index 40cb972e..f9c34105 100644
--- a/lib/Hoymiles/src/HoymilesRadio.cpp
+++ b/lib/Hoymiles/src/HoymilesRadio.cpp
@@ -123,6 +123,11 @@ void HoymilesRadio::loop()
                 _commandQueue.pop();
                 _busyFlag = false;
             }
+        } else {
+            // If inverter was not found, assume the command is invalid
+            Serial.println(F("Invalid inverter found"));
+            _commandQueue.pop();
+            _busyFlag = false;
         }
     } else if (!_busyFlag) {
         // Currently in idle mode --> send packet if one is in the queue
diff --git a/src/WebApi_inverter.cpp b/src/WebApi_inverter.cpp
index e82a5c5b..db2d0344 100644
--- a/src/WebApi_inverter.cpp
+++ b/src/WebApi_inverter.cpp
@@ -225,8 +225,11 @@ void WebApiInverterClass::onInverterEdit(AsyncWebServerRequest* request)
     INVERTER_CONFIG_T& inverter = Configuration.get().Inverter[root[F("id")].as<uint8_t>()];
 
     char* t;
+    uint64_t new_serial = strtoll(root[F("serial")].as<String>().c_str(), &t, 16);
+    uint64_t old_serial = inverter.Serial;
+
     // Interpret the string as a hex value and convert it to uint64_t
-    inverter.Serial = strtoll(root[F("serial")].as<String>().c_str(), &t, 16);
+    inverter.Serial = new_serial;
     strncpy(inverter.Name, root[F("name")].as<String>().c_str(), INV_MAX_NAME_STRLEN);
 
     arrayCount = 0;
@@ -243,13 +246,17 @@ void WebApiInverterClass::onInverterEdit(AsyncWebServerRequest* request)
     response->setLength();
     request->send(response);
 
-    std::shared_ptr<InverterAbstract> inv = Hoymiles.getInverterByPos(root[F("id")].as<uint64_t>());
+    std::shared_ptr<InverterAbstract> inv = Hoymiles.getInverterBySerial(old_serial);
 
-    if (inv != nullptr && inv->serial() != inverter.Serial) {
-        // Serial was changed
-        Hoymiles.removeInverterByPos(root[F("id")].as<uint64_t>());
+    if (inv != nullptr && new_serial != old_serial) {
+        // Valid inverter exists but serial changed --> remove it and insert new one
+        Hoymiles.removeInverterBySerial(old_serial);
         inv = Hoymiles.addInverter(inverter.Name, inverter.Serial);
+    } else if (inv != nullptr && new_serial == old_serial) {
+        // Valid inverter exists and serial stays the same --> update name
+        inv->setName(inverter.Name);
     } else if (inv == nullptr) {
+        // Valid inverter did not exist --> try to create one
         inv = Hoymiles.addInverter(inverter.Name, inverter.Serial);
     }
 
@@ -310,6 +317,9 @@ void WebApiInverterClass::onInverterDelete(AsyncWebServerRequest* request)
 
     uint8_t inverter_id = root[F("id")].as<uint8_t>();
     INVERTER_CONFIG_T& inverter = Configuration.get().Inverter[inverter_id];
+
+    Hoymiles.removeInverterBySerial(inverter.Serial);
+
     inverter.Serial = 0;
     strncpy(inverter.Name, "", 0);
     Configuration.write();
@@ -320,7 +330,5 @@ void WebApiInverterClass::onInverterDelete(AsyncWebServerRequest* request)
     response->setLength();
     request->send(response);
 
-    Hoymiles.removeInverterByPos(inverter_id);
-
     MqttHassPublishing.forceUpdate();
 }
\ No newline at end of file