Improve power stability when using MQTT (#1070)

* Changes to improve MQTT and power stability

* Re-added the memory leak fix

* Re-added the memory leak fix

* Stop client before deleting

* Fixed potential nullpointer

lib/AmsMqttHandler/include/AmsMqttHandler.h

@@ -62,6 +62,11 @@ public:
     virtual void onMessage(String &topic, String &payload) {};
 
     virtual ~AmsMqttHandler() {
+        if(mqttSecureClient != NULL) {
+            mqttSecureClient->stop();
+            delete mqttSecureClient;
+        }
+
         if(mqttClient != NULL) {
             mqttClient->stop();
             delete mqttClient;
@@ -81,6 +86,7 @@ protected:
     bool caVerification = true;
     WiFiClient *mqttClient = NULL;
     WiFiClientSecure *mqttSecureClient = NULL;
+    boolean _connected = false;
     char* json;
     uint16_t BufferSize = 2048;
     uint64_t lastStateUpdate = 0;

lib/AmsMqttHandler/src/AmsMqttHandler.cpp

@@ -102,6 +102,10 @@ bool AmsMqttHandler::connect() {
 		}
 		actualClient = mqttClient;
 	}
+	int clientTimeout = mqttConfig.timeout / 1000;
+	if(clientTimeout > 3) clientTimeout = 3; // 3000ms is default, see WiFiClient.cpp WIFI_CLIENT_DEF_CONN_TIMEOUT_MS
+	actualClient->setTimeout(clientTimeout);
+	// Why can't we set number of retries for write here? WiFiClient defaults to 10 (10*3s == 30s)
 
 	mqttConfigChanged = false;
 	mqtt.setTimeout(mqttConfig.timeout);
@@ -125,7 +129,7 @@ bool AmsMqttHandler::connect() {
 		#endif
 		debugger->printf_P(PSTR("Successfully connected to MQTT\n"));
 		mqtt.onMessage(std::bind(&AmsMqttHandler::onMessage, this, std::placeholders::_1, std::placeholders::_2));
-		mqtt.publish(statusTopic, "online", true, 0);
+		_connected = mqtt.publish(statusTopic, "online", true, 0);
         mqtt.loop();
 		defaultSubscribe();
 		postConnect();
@@ -169,10 +173,7 @@ bool AmsMqttHandler::defaultSubscribe() {
 void AmsMqttHandler::disconnect() {
     mqtt.disconnect();
     mqtt.loop();
-	if(mqttSecureClient != NULL) {
-		delete mqttSecureClient;
-		mqttSecureClient = NULL;
-	}
+	_connected = false;
     delay(10);
     yield();
 }
@@ -182,12 +183,12 @@ lwmqtt_err_t AmsMqttHandler::lastError() {
 }
 
 bool AmsMqttHandler::connected() {
-	return mqtt.connected();
+	return _connected && mqtt.connected();
 }
 
 bool AmsMqttHandler::loop() {
 	uint64_t now = millis64();
-    bool ret = mqtt.connected() && mqtt.loop();
+    bool ret = connected() && mqtt.loop();
 	if(ret) {
 		lastSuccessfulLoop = now;
 	} else if(mqttConfig.rebootMinutes > 0) {

lib/HomeAssistantMqttHandler/src/HomeAssistantMqttHandler.cpp

@@ -89,7 +89,7 @@ bool HomeAssistantMqttHandler::postConnect() {
 }
 
 bool HomeAssistantMqttHandler::publish(AmsData* update, AmsData* previousState, EnergyAccounting* ea, PriceService* ps) {
-	if(pubTopic.isEmpty() || !mqtt.connected())
+	if(pubTopic.isEmpty() || !connected())
 		return false;
 
     if(time(nullptr) < FirmwareVersion::BuildEpoch)
@@ -358,7 +358,7 @@ bool HomeAssistantMqttHandler::publishTemperatures(AmsConfiguration* config, HwT
 }
 
 bool HomeAssistantMqttHandler::publishPrices(PriceService* ps) {
-	if(pubTopic.isEmpty() || !mqtt.connected())
+	if(pubTopic.isEmpty() || !connected())
 		return false;
 	if(!ps->hasPrice())
 		return false;
@@ -494,7 +494,7 @@ bool HomeAssistantMqttHandler::publishPrices(PriceService* ps) {
 }
 
 bool HomeAssistantMqttHandler::publishSystem(HwTools* hw, PriceService* ps, EnergyAccounting* ea) {
-	if(pubTopic.isEmpty() || !mqtt.connected())
+	if(pubTopic.isEmpty() || !connected())
 		return false;
 
     publishSystemSensors();

lib/JsonMqttHandler/src/JsonMqttHandler.cpp

@@ -14,7 +14,7 @@ bool JsonMqttHandler::publish(AmsData* update, AmsData* previousState, EnergyAcc
     if(strlen(mqttConfig.publishTopic) == 0) {
         return false;
     }
-	if(!mqtt.connected()) {
+	if(!connected()) {
 		return false;
     }
 
@@ -295,7 +295,7 @@ bool JsonMqttHandler::publishTemperatures(AmsConfiguration* config, HwTools* hw)
 }
 
 bool JsonMqttHandler::publishPrices(PriceService* ps) {
-	if(strlen(mqttConfig.publishTopic) == 0 || !mqtt.connected())
+	if(strlen(mqttConfig.publishTopic) == 0 || !connected())
 		return false;
 	if(!ps->hasPrice())
 		return false;
@@ -444,7 +444,7 @@ bool JsonMqttHandler::publishPrices(PriceService* ps) {
 }
 
 bool JsonMqttHandler::publishSystem(HwTools* hw, PriceService* ps, EnergyAccounting* ea) {
-	if(strlen(mqttConfig.publishTopic) == 0 || !mqtt.connected())
+	if(strlen(mqttConfig.publishTopic) == 0 || !connected())
 		return false;
 
     snprintf_P(json, BufferSize, PSTR("{\"id\":\"%s\",\"name\":\"%s\",\"up\":%d,\"vcc\":%.3f,\"rssi\":%d,\"temp\":%.2f,\"version\":\"%s\"}"),
@@ -491,7 +491,7 @@ bool JsonMqttHandler::publishFirmware() {
 }
 
 void JsonMqttHandler::onMessage(String &topic, String &payload) {
-    if(strlen(mqttConfig.publishTopic) == 0 || !mqtt.connected())
+    if(strlen(mqttConfig.publishTopic) == 0 || !connected())
 		return;
 
     #if defined(AMS_REMOTE_DEBUG)

lib/RawMqttHandler/src/RawMqttHandler.cpp

@@ -10,7 +10,7 @@
 #include "FirmwareVersion.h"
 
 bool RawMqttHandler::publish(AmsData* update, AmsData* previousState, EnergyAccounting* ea, PriceService* ps) {
-	if(topic.isEmpty() || !mqtt.connected())
+	if(topic.isEmpty() || !connected())
 		return false;
 
     AmsData data;
@@ -237,7 +237,7 @@ bool RawMqttHandler::publishTemperatures(AmsConfiguration* config, HwTools* hw)
 }
 
 bool RawMqttHandler::publishPrices(PriceService* ps) {
-	if(topic.isEmpty() || !mqtt.connected())
+	if(topic.isEmpty() || !connected())
 		return false;
 	if(!ps->hasPrice())
 		return false;
@@ -369,7 +369,7 @@ bool RawMqttHandler::publishPrices(PriceService* ps) {
 }
 
 bool RawMqttHandler::publishSystem(HwTools* hw, PriceService* ps, EnergyAccounting* ea) {
-	if(topic.isEmpty() || !mqtt.connected())
+	if(topic.isEmpty() || !connected())
 		return false;
 
 	mqtt.publish(topic + "/id", WiFi.macAddress(), true, 0);