polish support for second VE.Direct MPPT charge controller

* fix compiler warning in SerialPortManager.cpp: function must not return void * clean up and simplify implementation of usesHwPort2() * make const * overrides are final * default implementation returns false * implement in header, as the implementation is very simple * rename PortManager to SerialPortManager. as "PortManager" is too generic, the static instance of the serial port manager is renamed to "SerialPortManager". the class is therefore renamed to SerialPortManagerClass, which is in line with other (static) classes withing OpenDTU(-OnBattery). * implement separate data ages for MPPT charge controllers * make sure MPPT data and live data time out * do not use invalid data of MPPT controlers for calculations * add :key binding to v-for iterating over MPPT instances
hoylabs · Mar 17, 2024 · 7d6b725 · 7d6b725
1 parent 75541be
commit 7d6b725
Show file tree

Hide file tree

Showing 19 changed files with 103 additions and 87 deletions.
diff --git a/include/Battery.h b/include/Battery.h
@@ -14,7 +14,7 @@ class BatteryProvider {
     virtual void deinit() = 0;
     virtual void loop() = 0;
     virtual std::shared_ptr<BatteryStats> getStats() const = 0;
-    virtual bool usesHwPort2() = 0;
+    virtual bool usesHwPort2() const { return false; }
 };
 
 class BatteryClass {

diff --git a/include/JkBmsController.h b/include/JkBmsController.h
@@ -19,7 +19,7 @@ class Controller : public BatteryProvider {
         void deinit() final;
         void loop() final;
         std::shared_ptr<BatteryStats> getStats() const final { return _stats; }
-        bool usesHwPort2() override;
+        bool usesHwPort2() const final { return true; }
 
     private:
         enum class Status : unsigned {

diff --git a/include/MqttBattery.h b/include/MqttBattery.h
@@ -12,7 +12,6 @@ class MqttBattery : public BatteryProvider {
     void deinit() final;
     void loop() final { return; } // this class is event-driven
     std::shared_ptr<BatteryStats> getStats() const final { return _stats; }
-    bool usesHwPort2() override;
 
 private:
     bool _verboseLogging = false;

diff --git a/include/PylontechCanReceiver.h b/include/PylontechCanReceiver.h
@@ -14,7 +14,6 @@ class PylontechCanReceiver : public BatteryProvider {
     void deinit() final;
     void loop() final;
     std::shared_ptr<BatteryStats> getStats() const final { return _stats; }
-    bool usesHwPort2() override;
 
 private:
     uint16_t readUnsignedInt16(uint8_t *data);

diff --git a/include/SerialPortManager.h b/include/SerialPortManager.h
@@ -3,7 +3,7 @@
 
 #include <map>
 
-class SerialPortManager {
+class SerialPortManagerClass {
 public:
     bool allocateMpptPort(int port);
     bool allocateBatteryPort(int port);
@@ -24,4 +24,4 @@ class SerialPortManager {
     static const char* print(Owner owner);
 };
 
-extern SerialPortManager PortManager;
+extern SerialPortManagerClass SerialPortManager;
diff --git a/include/VictronMppt.h b/include/VictronMppt.h
@@ -22,6 +22,7 @@ class VictronMpptClass {
     // returns the data age of all controllers,
     // i.e, the youngest data's age is returned.
     uint32_t getDataAgeMillis() const;
+    uint32_t getDataAgeMillis(size_t idx) const;
 
     std::optional<VeDirectMpptController::spData_t> getData(size_t idx = 0) const;
 

diff --git a/include/VictronSmartShunt.h b/include/VictronSmartShunt.h
@@ -9,7 +9,7 @@ class VictronSmartShunt : public BatteryProvider {
     void deinit() final { }
     void loop() final;
     std::shared_ptr<BatteryStats> getStats() const final { return _stats; }
-    bool usesHwPort2() override;
+    bool usesHwPort2() const final { return true; }
 
 private:
     uint32_t _lastUpdate = 0;

diff --git a/include/WebApi_ws_vedirect_live.h b/include/WebApi_ws_vedirect_live.h
@@ -14,16 +14,16 @@ class WebApiWsVedirectLiveClass {
     void init(AsyncWebServer& server, Scheduler& scheduler);
 
 private:
-    void generateJsonResponse(JsonVariant& root);
+    void generateJsonResponse(JsonVariant& root, bool fullUpdate);
     static void populateJson(const JsonObject &root, const VeDirectMpptController::spData_t &spMpptData);
     void onLivedataStatus(AsyncWebServerRequest* request);
     void onWebsocketEvent(AsyncWebSocket* server, AsyncWebSocketClient* client, AwsEventType type, void* arg, uint8_t* data, size_t len);
 
     AsyncWebServer* _server;
     AsyncWebSocket _ws;
 
-    uint32_t _lastWsPublish = 0;
-    uint32_t _dataAgeMillis = 0;
+    uint32_t _lastFullPublish = 0;
+    uint32_t _dataAgeMillis[VICTRON_MAX_COUNT] = { 0 };
     static constexpr uint16_t _responseSize = VICTRON_MAX_COUNT * (1024 + 128);
 
     std::mutex _mutex;

diff --git a/lib/VeDirectFrameHandler/VeDirectFrameHandler.cpp b/lib/VeDirectFrameHandler/VeDirectFrameHandler.cpp
@@ -281,13 +281,7 @@ int VeDirectFrameHandler::hexRxEvent(uint8_t inbyte) {
 }
 
 bool VeDirectFrameHandler::isDataValid(veStruct const& frame) const {
-	if (_lastUpdate == 0) {
-		return false;
-	}
-	if (strlen(frame.SER) == 0) {
-		return false;
-	}
-	return true;
+	return strlen(frame.SER) > 0 && _lastUpdate > 0 && (millis() - _lastUpdate) < (10 * 1000);
 }
 
 uint32_t VeDirectFrameHandler::getLastUpdate() const

diff --git a/src/Battery.cpp b/src/Battery.cpp
@@ -39,7 +39,7 @@ void BatteryClass::updateSettings()
         _upProvider->deinit();
         _upProvider = nullptr;
     }
-    PortManager.invalidateBatteryPort();
+    SerialPortManager.invalidateBatteryPort();
 
     CONFIG_T& config = Configuration.get();
     if (!config.Battery.Enabled) { return; }
@@ -65,15 +65,15 @@ void BatteryClass::updateSettings()
     }
 
     if(_upProvider->usesHwPort2()) {
-        if (!PortManager.allocateBatteryPort(2)) {
+        if (!SerialPortManager.allocateBatteryPort(2)) {
             MessageOutput.printf("[Battery] Serial port %d already in use. Initialization aborted!\r\n", 2);
             _upProvider = nullptr;
             return;
         }
     }
 
     if (!_upProvider->init(verboseLogging)) {
-        PortManager.invalidateBatteryPort();
+        SerialPortManager.invalidateBatteryPort();
         _upProvider = nullptr;
     }
 }

diff --git a/src/JkBmsController.cpp b/src/JkBmsController.cpp
@@ -427,8 +427,4 @@ void Controller::processDataPoints(DataPointContainer const& dataPoints)
     }
 }
 
-bool Controller::usesHwPort2() {
-    return true;
-}
-
 } /* namespace JkBms */
diff --git a/src/MqttBattery.cpp b/src/MqttBattery.cpp
@@ -112,7 +112,3 @@ void MqttBattery::onMqttMessageVoltage(espMqttClientTypes::MessageProperties con
                 *voltage, topic);
     }
 }
-
-bool MqttBattery::usesHwPort2() {
-    return false;
-}
diff --git a/src/PylontechCanReceiver.cpp b/src/PylontechCanReceiver.cpp
@@ -266,10 +266,6 @@ bool PylontechCanReceiver::getBit(uint8_t value, uint8_t bit)
     return (value & (1 << bit)) >> bit;
 }
 
-bool PylontechCanReceiver::usesHwPort2() {
-    return false;
-}
-
 #ifdef PYLONTECH_DUMMY
 void PylontechCanReceiver::dummyData()
 {

diff --git a/src/SerialPortManager.cpp b/src/SerialPortManager.cpp
@@ -4,19 +4,19 @@
 
 #define MAX_CONTROLLERS 3
 
-SerialPortManager PortManager;
+SerialPortManagerClass SerialPortManager;
 
-bool SerialPortManager::allocateBatteryPort(int port)
+bool SerialPortManagerClass::allocateBatteryPort(int port)
 {
     return allocatePort(port, Owner::BATTERY);
 }
 
-bool SerialPortManager::allocateMpptPort(int port)
+bool SerialPortManagerClass::allocateMpptPort(int port)
 {
     return allocatePort(port, Owner::MPPT);
 }
 
-bool SerialPortManager::allocatePort(uint8_t port, Owner owner)
+bool SerialPortManagerClass::allocatePort(uint8_t port, Owner owner)
 {
     if (port >= MAX_CONTROLLERS) {
         MessageOutput.printf("[SerialPortManager] Invalid serial port = %d \r\n", port);
@@ -26,17 +26,17 @@ bool SerialPortManager::allocatePort(uint8_t port, Owner owner)
     return allocatedPorts.insert({port, owner}).second;
 }
 
-void SerialPortManager::invalidateBatteryPort()
+void SerialPortManagerClass::invalidateBatteryPort()
 {
     invalidate(Owner::BATTERY);
 }
 
-void SerialPortManager::invalidateMpptPorts()
+void SerialPortManagerClass::invalidateMpptPorts()
 {
     invalidate(Owner::MPPT);
 }
 
-void SerialPortManager::invalidate(Owner owner)
+void SerialPortManagerClass::invalidate(Owner owner)
 {
     for (auto it = allocatedPorts.begin(); it != allocatedPorts.end();) {
         if (it->second == owner) {
@@ -48,12 +48,13 @@ void SerialPortManager::invalidate(Owner owner)
     }
 }
 
-const char* SerialPortManager::print(Owner owner)
+const char* SerialPortManagerClass::print(Owner owner)
 {
     switch (owner) {
         case BATTERY:
             return "BATTERY";
         case MPPT:
             return "MPPT";
     }
+    return "unknown";
 }
diff --git a/src/VictronMppt.cpp b/src/VictronMppt.cpp
@@ -22,7 +22,7 @@ void VictronMpptClass::updateSettings()
     std::lock_guard<std::mutex> lock(_mutex);
 
     _controllers.clear();
-    PortManager.invalidateMpptPorts();
+    SerialPortManager.invalidateMpptPorts();
 
     CONFIG_T& config = Configuration.get();
     if (!config.Vedirect.Enabled) { return; }
@@ -47,7 +47,7 @@ bool VictronMpptClass::initController(int8_t rx, int8_t tx, bool logging, int hw
         return false;
     }
 
-    if (!PortManager.allocateMpptPort(hwSerialPort)) {
+    if (!SerialPortManager.allocateMpptPort(hwSerialPort)) {
         MessageOutput.printf("[VictronMppt] Serial port %d already in use. Initialization aborted!\r\n",
                              hwSerialPort);
         return false;
@@ -110,6 +110,15 @@ uint32_t VictronMpptClass::getDataAgeMillis() const
     return age;
 }
 
+uint32_t VictronMpptClass::getDataAgeMillis(size_t idx) const
+{
+    std::lock_guard<std::mutex> lock(_mutex);
+
+    if (_controllers.empty() || idx >= _controllers.size()) { return 0; }
+
+    return millis() - _controllers[idx]->getLastUpdate();
+}
+
 std::optional<VeDirectMpptController::spData_t> VictronMpptClass::getData(size_t idx) const
 {
     std::lock_guard<std::mutex> lock(_mutex);
@@ -128,6 +137,7 @@ int32_t VictronMpptClass::getPowerOutputWatts() const
     int32_t sum = 0;
 
     for (const auto& upController : _controllers) {
+        if (!upController->isDataValid()) { continue; }
         sum += upController->getData()->P;
     }
 
@@ -139,6 +149,7 @@ int32_t VictronMpptClass::getPanelPowerWatts() const
     int32_t sum = 0;
 
     for (const auto& upController : _controllers) {
+        if (!upController->isDataValid()) { continue; }
         sum += upController->getData()->PPV;
     }
 
@@ -150,6 +161,7 @@ double VictronMpptClass::getYieldTotal() const
     double sum = 0;
 
     for (const auto& upController : _controllers) {
+        if (!upController->isDataValid()) { continue; }
         sum += upController->getData()->H19;
     }
 
@@ -161,6 +173,7 @@ double VictronMpptClass::getYieldDay() const
     double sum = 0;
 
     for (const auto& upController : _controllers) {
+        if (!upController->isDataValid()) { continue; }
         sum += upController->getData()->H20;
     }
 
@@ -172,6 +185,7 @@ double VictronMpptClass::getOutputVoltage() const
     double min = -1;
 
     for (const auto& upController : _controllers) {
+        if (!upController->isDataValid()) { continue; }
         double volts = upController->getData()->V;
         if (min == -1) { min = volts; }
         min = std::min(min, volts);

diff --git a/src/VictronSmartShunt.cpp b/src/VictronSmartShunt.cpp
@@ -34,7 +34,3 @@ void VictronSmartShunt::loop()
     _stats->updateFrom(VeDirectShunt.veFrame);
     _lastUpdate = VeDirectShunt.getLastUpdate();
 }
-
-bool VictronSmartShunt::usesHwPort2() {
-    return true;
-}
diff --git a/src/WebApi_ws_vedirect_live.cpp b/src/WebApi_ws_vedirect_live.cpp
@@ -55,25 +55,28 @@ void WebApiWsVedirectLiveClass::wsCleanupTaskCb()
 void WebApiWsVedirectLiveClass::sendDataTaskCb()
 {
     // do nothing if no WS client is connected
-    if (_ws.count() == 0) {
-        return;
-    }
-
-    // we assume this loop to be running at least twice for every
-    // update from a VE.Direct MPPT data producer, so _dataAgeMillis
-    // actually grows in between updates.
-    auto lastDataAgeMillis = _dataAgeMillis;
-    _dataAgeMillis = VictronMppt.getDataAgeMillis();
+    if (_ws.count() == 0) { return; }
 
     // Update on ve.direct change or at least after 10 seconds
-    if (millis() - _lastWsPublish > (10 * 1000) || lastDataAgeMillis > _dataAgeMillis) {
+    bool fullUpdate = (millis() - _lastFullPublish > (10 * 1000));
+    bool updateAvailable = false;
+    if (!fullUpdate) {
+        for (int idx = 0; idx < VICTRON_MAX_COUNT; ++idx) {
+            auto currentAgeMillis = VictronMppt.getDataAgeMillis(idx);
+            if (currentAgeMillis > 0 && currentAgeMillis < _dataAgeMillis[idx]) {
+                updateAvailable = true;
+                break;
+            }
+        }
+    }
 
+    if (fullUpdate || updateAvailable) {
         try {
             std::lock_guard<std::mutex> lock(_mutex);
             DynamicJsonDocument root(_responseSize);
             if (Utils::checkJsonAlloc(root, __FUNCTION__, __LINE__)) {
                 JsonVariant var = root;
-                generateJsonResponse(var);
+                generateJsonResponse(var, fullUpdate);
 
                 String buffer;
                 serializeJson(root, buffer);
@@ -92,26 +95,34 @@ void WebApiWsVedirectLiveClass::sendDataTaskCb()
         } catch (const std::exception& exc) {
             MessageOutput.printf("Unknown exception in /api/vedirectlivedata/status. Reason: \"%s\".\r\n", exc.what());
         }
+    }
 
-        _lastWsPublish = millis();
+    if (fullUpdate) {
+        _lastFullPublish = millis();
     }
 }
 
-void WebApiWsVedirectLiveClass::generateJsonResponse(JsonVariant& root)
+void WebApiWsVedirectLiveClass::generateJsonResponse(JsonVariant& root, bool fullUpdate)
 {
-    root["vedirect"]["data_age"] = VictronMppt.getDataAgeMillis() / 1000;
-    const JsonArray &array = root["vedirect"].createNestedArray("devices");
+    const JsonObject &array = root["vedirect"].createNestedObject("instances");
+    root["vedirect"]["full_update"] = fullUpdate;
 
     for (int idx = 0; idx < VICTRON_MAX_COUNT; ++idx) {
         std::optional<VeDirectMpptController::spData_t> spOptMpptData = VictronMppt.getData(idx);
         if (!spOptMpptData.has_value()) {
             continue;
         }
 
+        auto lastDataAgeMillis = _dataAgeMillis[idx];
+        _dataAgeMillis[idx] = VictronMppt.getDataAgeMillis(idx);
+        bool validAge = _dataAgeMillis[idx] > 0;
+        bool updateAvailable = _dataAgeMillis[idx] < lastDataAgeMillis;
+        if (!fullUpdate && !(validAge && updateAvailable)) { continue; }
+
         VeDirectMpptController::spData_t &spMpptData = spOptMpptData.value();
 
-        const JsonObject &nested = array.createNestedObject();
-        nested["age_critical"] = !VictronMppt.isDataValid(idx);
+        const JsonObject &nested = array.createNestedObject(spMpptData->SER);
+        nested["data_age_ms"] = _dataAgeMillis[idx];
         populateJson(nested, spMpptData);
     }
 
@@ -122,8 +133,7 @@ void WebApiWsVedirectLiveClass::generateJsonResponse(JsonVariant& root)
     root["dpl"]["PLLIMIT"] = PowerLimiter.getLastRequestedPowerLimit();
 }
 
-void
-WebApiWsVedirectLiveClass::populateJson(const JsonObject &root, const VeDirectMpptController::spData_t &spMpptData) {
+void WebApiWsVedirectLiveClass::populateJson(const JsonObject &root, const VeDirectMpptController::spData_t &spMpptData) {
     // device info
     root["device"]["PID"] = spMpptData->getPidAsString();
     root["device"]["SER"] = spMpptData->SER;
@@ -202,7 +212,7 @@ void WebApiWsVedirectLiveClass::onLivedataStatus(AsyncWebServerRequest* request)
         AsyncJsonResponse* response = new AsyncJsonResponse(false, _responseSize);
         auto& root = response->getRoot();
 
-        generateJsonResponse(root);
+        generateJsonResponse(root, true/*fullUpdate*/);
 
         response->setLength();
         request->send(response);