From d4522fe45737e82a35baae7630583b9d5fb5650a Mon Sep 17 00:00:00 2001 From: gene Date: Tue, 10 Dec 2024 00:37:49 +0100 Subject: [PATCH] v1.2.42 - Fixed bug in *Modules.ParameterSet* when setting *ConfigureOption.* fields (not saving MQTT client settings) - Fixed memory leak in *Modules.ParameterGet* API method - Fixed memory leak in MQTT client lifecycle --- src/data/JsonStore.h | 4 +-- src/net/MQTTClient.h | 44 +++++++++++++++++----------- src/service/api/HomeGenieHandler.cpp | 19 +++++++----- src/version.h | 2 +- 4 files changed, 42 insertions(+), 27 deletions(-) diff --git a/src/data/JsonStore.h b/src/data/JsonStore.h index 5cc02a9..1a7e307 100644 --- a/src/data/JsonStore.h +++ b/src/data/JsonStore.h @@ -63,7 +63,7 @@ namespace Data { } else { - // TODO: report error / disable scheduler + // TODO: report error } } @@ -81,7 +81,7 @@ namespace Data { } else { - // TODO: report error / disable scheduler + // TODO: report error } } diff --git a/src/net/MQTTClient.h b/src/net/MQTTClient.h index 3cd40a1..1e52453 100644 --- a/src/net/MQTTClient.h +++ b/src/net/MQTTClient.h @@ -82,12 +82,15 @@ namespace Net { static MQTTRequestHandler* requestHandler; MQTTClient() { - setLoopInterval(5000); + setLoopInterval(2000); }; void loop() override { - if (isEnabled && !clientStarted) { + if (stopRequested) { + stop(); + stopRequested = false; + } else if (isEnabled && !clientStarted) { start(); } @@ -98,8 +101,8 @@ namespace Net { auto port = String(); auto tls = String(); auto webSockets = String(); - auto username = String(); - auto password = String(); + username = ""; + password = ""; for (ModuleParameter* p: parameters) { if(p->name.equals("ConfigureOptions.ServerAddress")) { address = String(p->value); @@ -116,20 +119,18 @@ namespace Net { } } - auto brokerUrl = new String(); if (tls.equals("on")) { - *brokerUrl = webSockets.equals("on") ? "wss://" : "mqtts://"; + brokerUrl = webSockets.equals("on") ? "wss://" : "mqtts://"; } else { - *brokerUrl = webSockets.equals("on") ? "ws://" : "mqtt://"; + brokerUrl = webSockets.equals("on") ? "ws://" : "mqtt://"; } - *brokerUrl += address + String(":") + port; + brokerUrl += address + String(":") + port; - stop(); - - mqtt_cfg.uri = brokerUrl->c_str(); + mqtt_cfg.uri = brokerUrl.c_str(); mqtt_cfg.username = username.c_str(); mqtt_cfg.password = password.c_str(); + stopRequested = true; } void enable() { @@ -137,7 +138,7 @@ namespace Net { } void disable() { isEnabled = false; - stop(); + stopRequested = true; } void start() { @@ -150,12 +151,16 @@ namespace Net { arduino_esp_crt_bundle_attach(&conf); */ + esp_mqtt_client_destroy(client); client = esp_mqtt_client_init(&mqtt_cfg); - /* The last argument may be used to pass data to the event handler, in this example mqtt_event_handler */ - esp_mqtt_client_register_event(client, static_cast(ESP_EVENT_ANY_ID), mqtt_event_handler, nullptr); + if (client != nullptr) { + /* The last argument may be used to pass data to the event handler, in this example mqtt_event_handler */ + esp_mqtt_client_register_event(client, static_cast(ESP_EVENT_ANY_ID), + mqtt_event_handler, nullptr); - if (esp_mqtt_client_start(client) == ESP_OK) { - clientStarted = true; + if (esp_mqtt_client_start(client) == ESP_OK) { + clientStarted = true; + } } } @@ -164,6 +169,7 @@ namespace Net { void stop() { if (clientStarted) { + esp_mqtt_client_disconnect(client); esp_mqtt_client_stop(client); clientStarted = false; } @@ -175,8 +181,12 @@ namespace Net { } private: - bool clientStarted = false; + String brokerUrl; + String username; + String password; bool isEnabled = false; + bool stopRequested = false; + bool clientStarted = false; esp_mqtt_client_handle_t client = nullptr; esp_mqtt_client_config_t mqtt_cfg { .uri = "" }; diff --git a/src/service/api/HomeGenieHandler.cpp b/src/service/api/HomeGenieHandler.cpp index a8c5262..b7a1141 100644 --- a/src/service/api/HomeGenieHandler.cpp +++ b/src/service/api/HomeGenieHandler.cpp @@ -318,6 +318,7 @@ namespace Service { namespace API { */ auto jsonParameter = HomeGenie::createModuleParameter(parameter->name.c_str(), parameter->value.c_str(), parameter->updateTime.c_str()); responseCallback->writeAll(jsonParameter); + free((void*)jsonParameter); return true; } } @@ -343,8 +344,13 @@ namespace Service { namespace API { } else { auto propName = request->getOption(2); auto propValue = WebServer::urlDecode(request->getOption(3)); - auto p = ModuleParameter(propName, propValue); - parameters.add(p); + if (!propName.isEmpty()) { + auto p = ModuleParameter(propName, propValue); + parameters.add(p); + } + } + if (parameters.size() == 0) { + return false; } // Update module parameters if (module != nullptr) { @@ -358,8 +364,10 @@ namespace Service { namespace API { m.value = p.value; homeGenie->getEventRouter().signalEvent(m); } - responseCallback->writeAll(ApiHandlerResponseText::OK); - return true; + if (!isProgramConfiguration) { + responseCallback->writeAll(ApiHandlerResponseText::OK); + return true; + } } // Update program configuration if (isProgramConfiguration) { @@ -367,9 +375,6 @@ namespace Service { namespace API { #ifndef DISABLE_MQTT_CLIENT if (address.equals(MQTT_NETWORK_CONFIGURATION)) { auto mqttNetwork = homeGenie->programs.getItem(MQTT_NETWORK_CONFIGURATION); - for (const ModuleParameter& p: parameters) { - mqttNetwork->setProperty(p.name, p.value); - } homeGenie->programs.save(); homeGenie->getNetManager().getMQTTClient().configure(mqttNetwork->properties); } diff --git a/src/version.h b/src/version.h index a243052..7e5468b 100644 --- a/src/version.h +++ b/src/version.h @@ -29,7 +29,7 @@ #define VERSION_MAJOR 1 #define VERSION_MINOR 2 -#define VERSION_PATCH 41 +#define VERSION_PATCH 42 #define STRING_VALUE(...) STRING_VALUE__(__VA_ARGS__) #define STRING_VALUE__(...) #__VA_ARGS__