From 8824bbe4c24d4d4b3f6a80fe8603e04703ad4dd1 Mon Sep 17 00:00:00 2001
From: proddy <paul@derbyshire.nl>
Date: Sun, 7 Mar 2021 17:50:14 +0100
Subject: [PATCH] fix for HA topics with invalid command formats - #728

---
 src/devices/thermostat.cpp |  1 +
 src/mqtt.cpp               | 11 +++++++----
 src/test/test.cpp          |  3 +++
 src/test/test.h            |  4 ++--
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/devices/thermostat.cpp b/src/devices/thermostat.cpp
index c3f6d98be..b6b57df4c 100644
--- a/src/devices/thermostat.cpp
+++ b/src/devices/thermostat.cpp
@@ -414,6 +414,7 @@ void Thermostat::register_mqtt_ha_config_hc(uint8_t hc_num) {
 }
 
 // for HA specifically when receiving over MQTT in the thermostat topic
+// e.g. thermostat_hc1
 // it could be either a 'mode' or a float value for a temperature. we try brute force both and see which one works.
 // return true if it parses the message correctly
 bool Thermostat::thermostat_ha_cmd(const char * message, uint8_t hc_num) {
diff --git a/src/mqtt.cpp b/src/mqtt.cpp
index 956a653a6..aad21c92b 100644
--- a/src/mqtt.cpp
+++ b/src/mqtt.cpp
@@ -249,14 +249,17 @@ void Mqtt::on_message(const char * fulltopic, const char * payload, size_t len)
     // see if we have this topic in our subscription list, then call its callback handler
     for (const auto & mf : mqtt_subfunctions_) {
         if (strcmp(topic, mf.topic_.c_str()) == 0) {
+            // if we have call back function then call it
+            // otherwise proceed as process as a command
             if (mf.mqtt_subfunction_) {
-                // matching function, call it. If it returns true keep quit
-                if ((mf.mqtt_subfunction_)(message)) {
-                    return; // function executed successfully
+                if (!(mf.mqtt_subfunction_)(message)) {
+                    LOG_ERROR(F("MQTT error: invalid payload %s for this topic %s"), message, topic);
                 }
+                return;
             }
 
-            // empty function. It's a command then. Find the command from the json and call it directly.
+            // It's a command then with the payload being JSON like {"cmd":"<cmd>", "data":<data>, "id":<n>}
+            // Find the command from the json and call it directly
             StaticJsonDocument<EMSESP_JSON_SIZE_SMALL> doc;
             DeserializationError                       error = deserializeJson(doc, message);
             if (error) {
diff --git a/src/test/test.cpp b/src/test/test.cpp
index 4475c2cc6..a60a6947c 100644
--- a/src/test/test.cpp
+++ b/src/test/test.cpp
@@ -421,6 +421,9 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) {
         EMSESP::mqtt_.incoming("ems-esp/thermostat_hc1", "heat");
         EMSESP::mqtt_.incoming("ems-esp/thermostat_hc2", "28.8");
         EMSESP::mqtt_.incoming("ems-esp/thermostat", "{\"cmd\":\"temp\",\"id\":2,\"data\":22}");
+
+        EMSESP::mqtt_.incoming("ems-esp/thermostat_hc3", "{\"cmd\":\"offsettemp\",\"data\":-3}");
+        EMSESP::mqtt_.incoming("ems-esp/thermostat_hc3", "{\"cmd\":\"temp\",\"data\":-3}");
     }
 
     if (command == "tc100") {
diff --git a/src/test/test.h b/src/test/test.h
index a7c2d4f4f..fdf70ff6c 100644
--- a/src/test/test.h
+++ b/src/test/test.h
@@ -25,13 +25,13 @@
 
 namespace emsesp {
 
-// #define EMSESP_TEST_DEFAULT "thermostat"
+#define EMSESP_TEST_DEFAULT "thermostat"
 // #define EMSESP_TEST_DEFAULT "solar"
 // #define EMSESP_TEST_DEFAULT "mixer"
 // #define EMSESP_TEST_DEFAULT "web"
 // #define EMSESP_TEST_DEFAULT "general"
 // #define EMSESP_TEST_DEFAULT "boiler"
-#define EMSESP_TEST_DEFAULT "mqtt2"
+// #define EMSESP_TEST_DEFAULT "mqtt2"
 
 class Test {
   public: