From 986b0249fe5ba39080fe07c9c49fc4fb3eb0d821 Mon Sep 17 00:00:00 2001 From: Henry Thasler Date: Wed, 29 May 2024 21:30:18 +0200 Subject: [PATCH 1/4] derive correct node_id for homeassistant service discovery in nested topics (fixes #1792) --- code/components/jomjol_mqtt/server_mqtt.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/code/components/jomjol_mqtt/server_mqtt.cpp b/code/components/jomjol_mqtt/server_mqtt.cpp index c95affd24..e317e9471 100644 --- a/code/components/jomjol_mqtt/server_mqtt.cpp +++ b/code/components/jomjol_mqtt/server_mqtt.cpp @@ -69,11 +69,20 @@ bool sendHomeAssistantDiscoveryTopic(std::string group, std::string field, name = group + " " + name; } + /** + * homeassistant needs the MQTT discovery topic according to the following structure: + * //[/]/config + * if the main topic is embedded in a nested structure, we just use the last part as node_id + * This means a maintopic "home/test/watermeter" is transformed to the discovery topic "homeassistant/sensor/watermeter/..." + */ + auto splitPos = maintopic.find_last_of('/'); + string node_id = ((splitPos == string::npos) ? maintopic : maintopic.substr(splitPos + 1)); + if (field == "problem") { // Special binary sensor which is based on error topic - topicFull = "homeassistant/binary_sensor/" + maintopic + "/" + configTopic + "/config"; + topicFull = "homeassistant/binary_sensor/" + node_id + "/" + configTopic + "/config"; } else { - topicFull = "homeassistant/sensor/" + maintopic + "/" + configTopic + "/config"; + topicFull = "homeassistant/sensor/" + node_id + "/" + configTopic + "/config"; } /* See https://www.home-assistant.io/docs/mqtt/discovery/ */ From 7bd8eabf0b32d215b669e6f25efc79328d0dbb13 Mon Sep 17 00:00:00 2001 From: Henry Thasler Date: Thu, 30 May 2024 09:56:50 +0200 Subject: [PATCH 2/4] explicit use of std::string --- code/components/jomjol_mqtt/server_mqtt.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/components/jomjol_mqtt/server_mqtt.cpp b/code/components/jomjol_mqtt/server_mqtt.cpp index e317e9471..badba42ec 100644 --- a/code/components/jomjol_mqtt/server_mqtt.cpp +++ b/code/components/jomjol_mqtt/server_mqtt.cpp @@ -76,7 +76,7 @@ bool sendHomeAssistantDiscoveryTopic(std::string group, std::string field, * This means a maintopic "home/test/watermeter" is transformed to the discovery topic "homeassistant/sensor/watermeter/..." */ auto splitPos = maintopic.find_last_of('/'); - string node_id = ((splitPos == string::npos) ? maintopic : maintopic.substr(splitPos + 1)); + std::string node_id = ((splitPos == std::string::npos) ? maintopic : maintopic.substr(splitPos + 1)); if (field == "problem") { // Special binary sensor which is based on error topic topicFull = "homeassistant/binary_sensor/" + node_id + "/" + configTopic + "/config"; From eebcb06f05a0111045389766e708730c52523eb1 Mon Sep 17 00:00:00 2001 From: Henry Date: Sun, 2 Jun 2024 18:32:07 +0200 Subject: [PATCH 3/4] move nodeId creation to separate function add unit-tests --- code/components/jomjol_mqtt/server_mqtt.cpp | 13 ++++++++--- code/components/jomjol_mqtt/server_mqtt.h | 1 + .../jomjol_mqtt/test_server_mqtt.cpp | 22 +++++++++++++++++++ code/test/test_suite_flowcontroll.cpp | 3 ++- 4 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 code/test/components/jomjol_mqtt/test_server_mqtt.cpp diff --git a/code/components/jomjol_mqtt/server_mqtt.cpp b/code/components/jomjol_mqtt/server_mqtt.cpp index badba42ec..7c7591d0b 100644 --- a/code/components/jomjol_mqtt/server_mqtt.cpp +++ b/code/components/jomjol_mqtt/server_mqtt.cpp @@ -49,6 +49,15 @@ void mqttServer_setMeterType(std::string _meterType, std::string _valueUnit, std rateUnit = _rateUnit; } +/** + * Takes any multi-level MQTT-topic and returns the last topic level as nodeId + * see https://www.hivemq.com/blog/mqtt-essentials-part-5-mqtt-topics-best-practices/ for details about MQTT topics +*/ +std::string createNodeId(std::string &topic) { + auto splitPos = topic.find_last_of('/'); + return (splitPos == std::string::npos) ? topic : topic.substr(splitPos + 1); +} + bool sendHomeAssistantDiscoveryTopic(std::string group, std::string field, std::string name, std::string icon, std::string unit, std::string deviceClass, std::string stateClass, std::string entityCategory, int qos) { @@ -75,9 +84,7 @@ bool sendHomeAssistantDiscoveryTopic(std::string group, std::string field, * if the main topic is embedded in a nested structure, we just use the last part as node_id * This means a maintopic "home/test/watermeter" is transformed to the discovery topic "homeassistant/sensor/watermeter/..." */ - auto splitPos = maintopic.find_last_of('/'); - std::string node_id = ((splitPos == std::string::npos) ? maintopic : maintopic.substr(splitPos + 1)); - + std::string node_id = createNodeId(maintopic); if (field == "problem") { // Special binary sensor which is based on error topic topicFull = "homeassistant/binary_sensor/" + node_id + "/" + configTopic + "/config"; } diff --git a/code/components/jomjol_mqtt/server_mqtt.h b/code/components/jomjol_mqtt/server_mqtt.h index 925358e35..41c359c98 100644 --- a/code/components/jomjol_mqtt/server_mqtt.h +++ b/code/components/jomjol_mqtt/server_mqtt.h @@ -22,6 +22,7 @@ std::string getTimeUnit(void); void GotConnected(std::string maintopic, bool SetRetainFlag); esp_err_t sendDiscovery_and_static_Topics(void); +std::string createNodeId(std::string &topic); #endif //SERVERMQTT_H #endif //ENABLE_MQTT \ No newline at end of file diff --git a/code/test/components/jomjol_mqtt/test_server_mqtt.cpp b/code/test/components/jomjol_mqtt/test_server_mqtt.cpp new file mode 100644 index 000000000..e59b12254 --- /dev/null +++ b/code/test/components/jomjol_mqtt/test_server_mqtt.cpp @@ -0,0 +1,22 @@ +#include +#include + +void test_createNodeId() +{ + std::string topic = "watermeter"; + TEST_ASSERT_EQUAL_STRING("watermeter", createNodeId(topic).c_str()); + + topic = "/watermeter"; + TEST_ASSERT_EQUAL_STRING("watermeter", createNodeId(topic).c_str()); + + topic = "home/test/watermeter"; + TEST_ASSERT_EQUAL_STRING("watermeter", createNodeId(topic).c_str()); + + topic = "home/test/subtopic/something/test/watermeter"; + TEST_ASSERT_EQUAL_STRING("watermeter", createNodeId(topic).c_str()); +} + +void test_mqtt() +{ + test_createNodeId(); +} \ No newline at end of file diff --git a/code/test/test_suite_flowcontroll.cpp b/code/test/test_suite_flowcontroll.cpp index b1643c26f..c59c7e96b 100644 --- a/code/test/test_suite_flowcontroll.cpp +++ b/code/test/test_suite_flowcontroll.cpp @@ -20,7 +20,7 @@ #include "components/jomjol-flowcontroll/test_PointerEvalAnalogToDigitNew.cpp" #include "components/jomjol-flowcontroll/test_getReadoutRawString.cpp" #include "components/jomjol-flowcontroll/test_cnnflowcontroll.cpp" - +#include "components/jomjol_mqtt/test_server_mqtt.cpp" bool Init_NVS_SDCard() { @@ -167,6 +167,7 @@ extern "C" void app_main() // getReadoutRawString test RUN_TEST(test_getReadoutRawString); + RUN_TEST(test_mqtt); UNITY_END(); } From f985a4d744bd26359f84edab323b4496617ea087 Mon Sep 17 00:00:00 2001 From: Henry Date: Sun, 2 Jun 2024 19:07:13 +0200 Subject: [PATCH 4/4] add documentation about node_id generation for Home Assistant MQTT Service Discovery --- param-docs/parameter-pages/MQTT/MainTopic.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/param-docs/parameter-pages/MQTT/MainTopic.md b/param-docs/parameter-pages/MQTT/MainTopic.md index 3cf12ec25..837b42eea 100644 --- a/param-docs/parameter-pages/MQTT/MainTopic.md +++ b/param-docs/parameter-pages/MQTT/MainTopic.md @@ -16,3 +16,5 @@ See [MQTT Result Topics](../MQTT-API#result) for a full list of topics. !!! Note The main topic is allowed to contain `/` which can be used to split it into multiple levels, eg. `/basement/meters/watermeter/1/` if you have multiple water meters in your basement. + +The nodeId for the Home Assistant MQTT Service Discovery must follow the schema `//[/]/config`. The node_id is not configurable but derived from the `MainTopic` by stripping any but the last topic level. A `MainTopic` with the value `home/basement/watermeter` is transformed into the node_id `watermeter`, resulting in the discovery topic `homeassistant/sensor/watermeter/value/config` for the current value.