From e6c1626deeb1f81f0c3abc2b15802499579b06ec Mon Sep 17 00:00:00 2001 From: Lukasz Michalski Date: Mon, 30 Dec 2024 19:51:11 +0100 Subject: [PATCH] fixed bug when non recent change was published if topic was updated as part of poll group read --- libmodmqttsrv/mqttclient.cpp | 7 +++- libmodmqttsrv/mqttobject.hpp | 2 ++ unittests/mqtt_publish_retain_tests.cpp | 44 +++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/libmodmqttsrv/mqttclient.cpp b/libmodmqttsrv/mqttclient.cpp index ed3f150..d01f4ad 100644 --- a/libmodmqttsrv/mqttclient.cpp +++ b/libmodmqttsrv/mqttclient.cpp @@ -151,6 +151,7 @@ MqttClient::processRegisterValues(const std::string& pModbusNetworkName, const M for (std::shared_ptr& obj: *affectedObjects) { AvailableFlag oldAvail = obj->getAvailableFlag(); + bool hadValue = obj->hasValue(); obj->updateRegisterValues(pModbusNetworkName, pSlaveData); AvailableFlag newAvail = obj->getAvailableFlag(); @@ -163,8 +164,12 @@ MqttClient::processRegisterValues(const std::string& pModbusNetworkName, const M publishState(*obj, true); } else { // delete retained message - if (oldAvail == AvailableFlag::NotSet) + if (oldAvail == AvailableFlag::NotSet) { mMqttImpl->publish(obj->getStateTopic().c_str(), 0, NULL, true); + // remember initial payload for comparsion with subsequent modbus data updates + if (!obj->getRetain()) + obj->setLastPublishedPayload(MqttPayload::generate(*obj)); + } if (obj->getPublishMode() == PublishMode::EVERY_POLL) publishState(*obj, true); } diff --git a/libmodmqttsrv/mqttobject.hpp b/libmodmqttsrv/mqttobject.hpp index 0197a60..e72844e 100644 --- a/libmodmqttsrv/mqttobject.hpp +++ b/libmodmqttsrv/mqttobject.hpp @@ -195,6 +195,8 @@ class MqttObject { void setAvailableValue(const MqttValue& pValue) { mAvailability.setAvailableValue(pValue); } AvailableFlag getAvailableFlag() const { return mIsAvailable; } + bool hasValue() const { return mState.hasAllValues(); } + void setLastPublishedPayload(const std::string& pVal) { mLastPublishedPayload = pVal; } const std::string& getLastPublishedPayload() const { return mLastPublishedPayload; } diff --git a/unittests/mqtt_publish_retain_tests.cpp b/unittests/mqtt_publish_retain_tests.cpp index 774d6ce..7077af9 100644 --- a/unittests/mqtt_publish_retain_tests.cpp +++ b/unittests/mqtt_publish_retain_tests.cpp @@ -100,3 +100,47 @@ TestConfig config(R"( } } +TEST_CASE ("When retain topic is a part of poll group") { + +TestConfig config(R"( +modbus: + networks: + - name: tcptest + address: localhost + port: 501 + slaves: + - address: 1 + poll_groups: + - register: 2 + register_type: holding + count: 2 +mqtt: + client_id: mqtt_test + refresh: 50ms + broker: + host: localhost + objects: + - topic: test_sensor + retain: false + state: + register: tcptest.1.2 + - topic: other_sensor + state: + register: tcptest.1.3 +)"); + + SECTION("its value should be ignored if related register value is not changed") { + MockedModMqttServerThread server(config.toString()); + server.setModbusRegisterValue("tcptest", 1, 2, modmqttd::RegisterType::HOLDING, 2); + server.setModbusRegisterValue("tcptest", 1, 3, modmqttd::RegisterType::HOLDING, 3); + + server.start(); + server.waitForPublish("other_sensor/state"); + + //this caused test_sensor value change due to null -> 3 + server.setModbusRegisterValue("tcptest", 1, 3, modmqttd::RegisterType::HOLDING, 33); + server.waitForPublish("other_sensor/state"); + + server.requirePublishCount("test_sensor/state", 1); + } +}