Merge branch 'development'

include/BatteryStats.h

@@ -95,4 +95,6 @@ class JkBmsBatteryStats : public BatteryStats {
 
     private:
         JkBms::DataPointContainer _dataPoints;
+        mutable uint32_t _lastMqttPublish = 0;
+        mutable uint32_t _lastFullMqttPublish = 0;
 };

include/HttpPowerMeter.h

@@ -2,6 +2,7 @@
 #pragma once
 
 #include <stdint.h>
+#include <Arduino.h>
 
 class HttpPowerMeterClass {
 public:

include/JkBmsDataPoints.h

@@ -185,6 +185,10 @@ class DataPoint {
         std::string const& getUnitText() const { return _strUnit; }
         uint32_t getTimestamp() const { return _timestamp; }
 
+        bool operator==(DataPoint const& other) const {
+            return _value == other._value;
+        }
+
     private:
         std::string _strLabel;
         std::string _strValue;

lib/VeDirectFrameHandler/VeDirectFrameHandler.cpp

@@ -2,23 +2,23 @@
  *
  * Arduino library to read from Victron devices using VE.Direct protocol.
  * Derived from Victron framehandler reference implementation.
- * 
+ *
  * The MIT License
- * 
+ *
  * Copyright (c) 2019 Victron Energy BV
  * Portions Copyright (C) 2020 Chris Terwilliger
  * https://github.com/cterwilliger/VeDirectFrameHandler
- * 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
  * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
  * copies of the Software, and to permit persons to whom the Software is
  * furnished to do so, subject to the following conditions:
- * 
+ *
  * The above copyright notice and this permission notice shall be included in
  * all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -26,13 +26,13 @@
  * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
- *  
+ *
  * 2020.05.05 - 0.2 - initial release
  * 2020.06.21 - 0.2 - add MIT license, no code changes
  * 2020.08.20 - 0.3 - corrected #include reference
- * 
+ *
  */
- 
+
 #include <Arduino.h>
 #include "VeDirectFrameHandler.h"
 
@@ -54,14 +54,13 @@ HardwareSerial VedirectSerial(1);
 VeDirectFrameHandler VeDirect;
 
 class Silent : public Print {
-    public:
+	public:
-        size_t write(uint8_t c) final { return 0; }
+		size_t write(uint8_t c) final { return 0; }
 };
 
 static Silent MessageOutputDummy;
 
 VeDirectFrameHandler::VeDirectFrameHandler() :
-	//mStop(false),	// don't know what Victron uses this for, not using
 	_msgOut(&MessageOutputDummy),
 	_state(IDLE),
 	_checksum(0),
@@ -79,6 +78,7 @@ VeDirectFrameHandler::VeDirectFrameHandler() :
 void VeDirectFrameHandler::setVerboseLogging(bool verboseLogging)
 {
 	_verboseLogging = verboseLogging;
+	if (!_verboseLogging) { _debugIn = 0; }
 }
 
 void VeDirectFrameHandler::init(int8_t rx, int8_t tx, Print* msgOut, bool verboseLogging)
@@ -121,22 +121,23 @@ void VeDirectFrameHandler::loop()
 }
 
 /*
- *	rxData
+ *  rxData
  *  This function is called by loop() which passes a byte of serial data
  *  Based on Victron's example code. But using String and Map instead of pointer and arrays
  */
 void VeDirectFrameHandler::rxData(uint8_t inbyte)
 {
-	_debugBuffer[_debugIn] = inbyte;
+	if (_verboseLogging) {
-	_debugIn = (_debugIn + 1) % _debugBuffer.size();
+		_debugBuffer[_debugIn] = inbyte;
-	if (0 == _debugIn) {
+		_debugIn = (_debugIn + 1) % _debugBuffer.size();
-		_msgOut->println("[VE.Direct] ERROR: debug buffer overrun!");
+		if (0 == _debugIn) {
+			_msgOut->println("[VE.Direct] ERROR: debug buffer overrun!");
+		}
 	}
 
-	//if (mStop) return;
 	if ( (inbyte == ':') && (_state != CHECKSUM) ) {
 		_prevState = _state; //hex frame can interrupt TEXT
-		_state = RECORD_HEX; 
+		_state = RECORD_HEX;
 		_hexSize = 0;
 	}
 	if (_state != RECORD_HEX) {
@@ -239,7 +240,7 @@ void VeDirectFrameHandler::textRxEvent(char * name, char * value) {
 	else if (strcmp(name, "LOAD") == 0) {
 		if (strcmp(value, "ON") == 0)
 			_tmpFrame.LOAD = true;
-		else	
+		else
 			_tmpFrame.LOAD = false;
 	}
 	else if (strcmp(name, "CS") == 0) {
@@ -284,12 +285,11 @@ void VeDirectFrameHandler::textRxEvent(char * name, char * value) {
 	else if (strcmp(name, "H23") == 0) {
 		_tmpFrame.H23 = atoi(value);
 	}
-	
 }
 
 
 /*
- *	frameEndEvent
+ *  frameEndEvent
  *  This function is called at the end of the received frame.  If the checksum is valid, the temp buffer is read line by line.
  *  If the name exists in the public buffer, the new value is copied to the public buffer.	If not, a new name/value entry
  *  is created in the public buffer.
@@ -316,8 +316,8 @@ void VeDirectFrameHandler::frameEndEvent(bool valid) {
 }
 
 /*
- *	hexRxEvent
+ *  hexRxEvent
- *  This function records hex answers or async messages	
+ *  This function records hex answers or async messages
  */
 int VeDirectFrameHandler::hexRxEvent(uint8_t inbyte) {
 	int ret=RECORD_HEX; // default - continue recording until end of frame
@@ -327,7 +327,7 @@ int VeDirectFrameHandler::hexRxEvent(uint8_t inbyte) {
 		// restore previous state
 		ret=_prevState;
 		break;
-		
+
 	default:
 		_hexSize++;
 		if (_hexSize>=VE_MAX_HEX_LEN) { // oops -buffer overflow - something went wrong, we abort
@@ -336,7 +336,7 @@ int VeDirectFrameHandler::hexRxEvent(uint8_t inbyte) {
 			ret=IDLE;
 		}
 	}
-	
+
 	return ret;
 }
 
@@ -352,7 +352,7 @@ bool VeDirectFrameHandler::isDataValid() {
 
 unsigned long VeDirectFrameHandler::getLastUpdate()
 {
-    return _lastUpdate;
+	return _lastUpdate;
 }
 
 /*
@@ -558,7 +558,7 @@ String VeDirectFrameHandler::getPidAsString(uint16_t pid)
 			break;
 		case 0XA110:
 			strPID =  "SmartSolar MPPT RS 450|100";
-			break; 
+			break;
 		case 0XA112:
 			strPID =  "BlueSolar MPPT VE.Can 250|70";
 			break;
@@ -764,4 +764,4 @@ String VeDirectFrameHandler::getMpptAsString(uint8_t mppt)
 			strMPPT = mppt;
 	}
 	return strMPPT;
-}
+}

lib/VeDirectFrameHandler/VeDirectFrameHandler.h

@@ -2,11 +2,11 @@
  *
  * Arduino library to read from Victron devices using VE.Direct protocol.
  * Derived from Victron framehandler reference implementation.
- * 
+ *
  * 2020.05.05 - 0.2 - initial release
  * 2021.02.23 - 0.3 - change frameLen to 22 per VE.Direct Protocol version 3.30
  * 2022.08.20 - 0.4 - changes for OpenDTU
- * 
+ *
  */
 
 #pragma once
@@ -14,19 +14,9 @@
 #include <Arduino.h>
 #include <array>
 
-#ifndef VICTRON_PIN_TX
-#define VICTRON_PIN_TX 21      // HardwareSerial TX Pin
-#endif
-
-#ifndef VICTRON_PIN_RX
-#define VICTRON_PIN_RX 22      // HardwareSerial RX Pin
-#endif
-
-#define VE_MAX_NAME_LEN 9   // VE.Direct Protocol: max name size is 9 including /0
 #define VE_MAX_VALUE_LEN 33 // VE.Direct Protocol: max value size is 33 including /0
 #define VE_MAX_HEX_LEN 100 // Maximum size of hex frame - max payload 34 byte (=68 char) + safe buffer
 
-
 typedef struct {
     uint16_t PID;                   // product id
     char SER[VE_MAX_VALUE_LEN];     // serial number
@@ -93,7 +83,7 @@ public:
     void loop();                                 // main loop to read ve.direct data
     unsigned long getLastUpdate();               // timestamp of last successful frame read
     bool isDataValid();                          // return true if data valid and not outdated
-    String getPidAsString(uint16_t pid);      // product id as string  
+    String getPidAsString(uint16_t pid);      // product id as string
     String getCsAsString(uint8_t cs);        // current state as string
     String getErrAsString(uint8_t err);      // errer state as string
     String getOrAsString(uint32_t offReason); // off reason as string
@@ -109,12 +99,11 @@ private:
     void frameEndEvent(bool);                 // copy temp struct to public struct
     int hexRxEvent(uint8_t);
 
-    //bool mStop;                               // not sure what Victron uses this for, not using
     Print* _msgOut;
     bool _verboseLogging;
     int _state;                                // current state
     int _prevState;                            // previous state
-    uint8_t	_checksum;                         // checksum value
+    uint8_t _checksum;                         // checksum value
     char * _textPointer;                       // pointer to the private buffer we're writing to, name or value
     int _hexSize;                               // length of hex buffer
     char _name[VE_MAX_VALUE_LEN];              // buffer for the field name

src/BatteryStats.cpp

@@ -1,5 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
+#include <vector>
+#include <algorithm>
 #include "BatteryStats.h"
+#include "Configuration.h"
 #include "MqttSettings.h"
 #include "JkBmsDataPoints.h"
 
@@ -145,6 +148,34 @@ void PylontechBatteryStats::mqttPublish() const
 void JkBmsBatteryStats::mqttPublish() const
 {
     BatteryStats::mqttPublish();
+
+    using Label = JkBms::DataPointLabel;
+
+    static std::vector<Label> mqttSkip = {
+        Label::CellsMilliVolt, // complex data format
+        Label::ModificationPassword, // sensitive data
+        Label::BatterySoCPercent // already published by base class
+    };
+
+    CONFIG_T& config = Configuration.get();
+
+    // publish all topics every minute, unless the retain flag is enabled
+    bool fullPublish = _lastFullMqttPublish + 60 * 1000 < millis();
+    fullPublish &= !config.Mqtt_Retain;
+
+    for (auto iter = _dataPoints.cbegin(); iter != _dataPoints.cend(); ++iter) {
+        // skip data points that did not change since last published
+        if (!fullPublish && iter->second.getTimestamp() < _lastMqttPublish) { continue; }
+
+        auto skipMatch = std::find(mqttSkip.begin(), mqttSkip.end(), iter->first);
+        if (skipMatch != mqttSkip.end()) { continue; }
+
+        String topic((std::string("battery/") + iter->second.getLabelText()).c_str());
+        MqttSettings.publish(topic, iter->second.getValueText().c_str());
+    }
+
+    _lastMqttPublish = millis();
+    if (fullPublish) { _lastFullMqttPublish = _lastMqttPublish; }
 }
 
 void JkBmsBatteryStats::updateFrom(JkBms::DataPointContainer const& dp)

src/HttpPowerMeter.cpp

@@ -7,6 +7,7 @@
 #include <FirebaseJson.h>
 #include <Crypto.h>
 #include <SHA256.h>
+#include <memory>
 
 void HttpPowerMeterClass::init()
 {
@@ -23,39 +24,36 @@ bool HttpPowerMeterClass::updateValues()
 
     char response[2000],
         errorMessage[256];
-    bool success = true;
 
     for (uint8_t i = 0; i < POWERMETER_MAX_PHASES; i++) {
         POWERMETER_HTTP_PHASE_CONFIG_T phaseConfig = config.Powermeter_Http_Phase[i];
 
-        if (!phaseConfig.Enabled || !success) {
+        if (!phaseConfig.Enabled) {
             power[i] = 0.0;
-            continue;
+            continue;	
-        }
+        } 
 
         if (i == 0 || config.PowerMeter_HttpIndividualRequests) {
-            if (!httpRequest(phaseConfig.Url, phaseConfig.AuthType, phaseConfig.Username, phaseConfig.Password, phaseConfig.HeaderKey, phaseConfig.HeaderValue, phaseConfig.Timeout,
+            if (httpRequest(phaseConfig.Url, phaseConfig.AuthType, phaseConfig.Username, phaseConfig.Password, phaseConfig.HeaderKey, phaseConfig.HeaderValue, phaseConfig.Timeout,
                 response, sizeof(response), errorMessage, sizeof(errorMessage))) {
+                  if (!getFloatValueByJsonPath(response, phaseConfig.JsonPath, power[i])) {
+                      MessageOutput.printf("[HttpPowerMeter] Couldn't find a value with Json query \"%s\"\r\n", phaseConfig.JsonPath);
+                      return false;
+                  }
+            } else {
                 MessageOutput.printf("[HttpPowerMeter] Getting the power of phase %d failed. Error: %s\r\n",
                     i + 1, errorMessage);
-                success = false;
+                return false;
             }
         }
-
-        if (!getFloatValueByJsonPath(response, phaseConfig.JsonPath, power[i])) {
-            MessageOutput.printf("[HttpPowerMeter] Couldn't find a value with Json query \"%s\"\r\n", phaseConfig.JsonPath);
-            success = false;
-        }
     }
 
-    return success;
+    return true;
 }
 
 bool HttpPowerMeterClass::httpRequest(const char* url, Auth authType, const char* username, const char* password, const char* httpHeader, const char* httpValue, uint32_t timeout,
         char* response, size_t responseSize, char* error, size_t errorSize)
 {
-    WiFiClient* wifiClient = NULL;
-    HTTPClient httpClient;
 
     String newUrl = url;
     String urlProtocol;
@@ -77,17 +75,22 @@ bool HttpPowerMeterClass::httpRequest(const char* url, Auth authType, const char
         newUrl += urlUri;
     }
     
+    // secureWifiClient MUST be created before HTTPClient
+    // see discussion: https://github.com/helgeerbe/OpenDTU-OnBattery/issues/381
+    std::unique_ptr<WiFiClient> wifiClient;
+
     if (urlProtocol == "https") {
-        wifiClient = new WiFiClientSecure;
+      auto secureWifiClient = std::make_unique<WiFiClientSecure>();
-        reinterpret_cast<WiFiClientSecure*>(wifiClient)->setInsecure();
+      secureWifiClient->setInsecure();
+      wifiClient = std::move(secureWifiClient);
     } else {
-        wifiClient = new WiFiClient;
+      wifiClient = std::make_unique<WiFiClient>();
     }
 
+    HTTPClient httpClient;
     if (!httpClient.begin(*wifiClient, newUrl)) {
-        snprintf_P(error, errorSize, "httpClient.begin(%s) failed", newUrl.c_str());
+      snprintf_P(error, errorSize, "httpClient.begin(%s) failed", newUrl.c_str());
-        delete wifiClient;
+      return false;
-        return false;
     }
 
     httpClient.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
@@ -179,7 +182,6 @@ bool HttpPowerMeterClass::httpRequest(const char* url, Auth authType, const char
     }
 
     httpClient.end();
-    delete wifiClient;
 
     if (error[0] != '\0') {
         return false;

src/JkBmsController.cpp

@@ -312,9 +312,9 @@ void Controller::frameComplete()
             ts, _buffer.size());
         for (size_t ctr = 0; ctr < _buffer.size(); ++ctr) {
             if (ctr % 16 == 0) {
-                MessageOutput.printf("\r\n[%11.3f] JK BMS: ", ts);
+                MessageOutput.printf("\r\n[%11.3f] JK BMS:", ts);
             }
-            MessageOutput.printf("%02x ", _buffer[ctr]);
+            MessageOutput.printf(" %02x", _buffer[ctr]);
         }
         MessageOutput.println();
     }

src/JkBmsDataPoints.cpp

@@ -48,7 +48,14 @@ std::string dataPointValueToStr(tCells const& v) {
 void DataPointContainer::updateFrom(DataPointContainer const& source)
 {
     for (auto iter = source.cbegin(); iter != source.cend(); ++iter) {
-        _dataPoints.erase(iter->first);
+        auto pos = _dataPoints.find(iter->first);
+
+        if (pos != _dataPoints.end()) {
+            // do not update existing data points with the same value
+            if (pos->second == iter->second) { continue; }
+
+            _dataPoints.erase(pos);
+        }
         _dataPoints.insert(*iter);
     }
 }