Merge branch 'issue_70'

BlackZork · Sep 29, 2024 · 8bcf1f4 · 8bcf1f4
2 parents 6b95703 + 6464a03
commit 8bcf1f4
Show file tree

Hide file tree

Showing 8 changed files with 114 additions and 44 deletions.
diff --git a/libmodmqttsrv/config.cpp b/libmodmqttsrv/config.cpp
@@ -29,26 +29,34 @@ ConfigurationException::ConfigurationException(const YAML::Mark& mark, const cha
 ModbusNetworkConfig::ModbusNetworkConfig(const YAML::Node& source) {
     mName = ConfigTools::readRequiredString(source, "name");
 
-    YAML::Node rtNode(ConfigTools::setOptionalValueFromNode<std::chrono::milliseconds>(this->mResponseTimeout, source, "response_timeout"));
+    YAML::Node rtNode(ConfigTools::setOptionalValueFromNode<std::chrono::milliseconds>(mResponseTimeout, source, "response_timeout"));
     if (rtNode.IsDefined()) {
-        if ((this->mResponseTimeout < std::chrono::milliseconds::zero()) || (this->mResponseTimeout > MAX_RESPONSE_TIMEOUT))
+        if ((mResponseTimeout < std::chrono::milliseconds::zero()) || (mResponseTimeout > MAX_RESPONSE_TIMEOUT))
             throw ConfigurationException(rtNode.Mark(), "response_timeout value must be in range 0-999ms");
     }
 
-    YAML::Node rtdNode(ConfigTools::setOptionalValueFromNode<std::chrono::milliseconds>(this->mResponseDataTimeout, source, "response_data_timeout"));
+    YAML::Node rtdNode(ConfigTools::setOptionalValueFromNode<std::chrono::milliseconds>(mResponseDataTimeout, source, "response_data_timeout"));
     if (rtdNode.IsDefined()) {
-        if ((this->mResponseDataTimeout < std::chrono::milliseconds::zero()) || (this->mResponseDataTimeout > MAX_RESPONSE_TIMEOUT))
+        if ((mResponseDataTimeout < std::chrono::milliseconds::zero()) || (mResponseDataTimeout > MAX_RESPONSE_TIMEOUT))
             throw ConfigurationException(rtdNode.Mark(), "response_data_timeout value must be in range 0-999ms");
     }
 
-    if (ConfigTools::readOptionalValue<std::chrono::milliseconds>(this->mDelayBeforeCommand, source, "min_delay_before_poll")) {
-            BOOST_LOG_SEV(log, Log::warn) << "'min_delay_before_poll' is deprecated and will be removed in future releases. Rename it to 'delay_before_command'";
+    std::chrono::milliseconds tmpval;
+    if (ConfigTools::readOptionalValue<std::chrono::milliseconds>(tmpval, source, "min_delay_before_poll")) {
+        BOOST_LOG_SEV(log, Log::warn) << "'min_delay_before_poll' is deprecated and will be removed in future releases. Rename it to 'delay_before_command'";
+        setDelayBeforeCommand(tmpval);
     }
-    ConfigTools::readOptionalValue<std::chrono::milliseconds>(this->mDelayBeforeCommand, source, "delay_before_command");
-    ConfigTools::readOptionalValue<std::chrono::milliseconds>(this->mDelayBeforeFirstCommand, source, "delay_before_first_command");
 
-    ConfigTools::readOptionalValue<unsigned short>(this->mMaxWriteRetryCount, source, "write_retries");
-    ConfigTools::readOptionalValue<unsigned short>(this->mMaxReadRetryCount, source, "read_retries");
+    if (ConfigTools::readOptionalValue<std::chrono::milliseconds>(tmpval, source, "delay_before_command")) {
+        setDelayBeforeCommand(tmpval);
+    }
+
+    if (ConfigTools::readOptionalValue<std::chrono::milliseconds>(tmpval, source, "delay_before_first_command")) {
+        setDelayBeforeFirstCommand(tmpval);
+    }
+
+    ConfigTools::readOptionalValue<unsigned short>(mMaxWriteRetryCount, source, "write_retries");
+    ConfigTools::readOptionalValue<unsigned short>(mMaxReadRetryCount, source, "read_retries");
 
 
     if (source["device"]) {
@@ -58,9 +66,9 @@ ModbusNetworkConfig::ModbusNetworkConfig(const YAML::Node& source) {
         mParity = ConfigTools::readRequiredValue<char>(source, "parity");
         mDataBit = ConfigTools::readRequiredValue<int>(source, "data_bit");
         mStopBit = ConfigTools::readRequiredValue<int>(source, "stop_bit");
-        ConfigTools::readOptionalValue<RtuSerialMode>(this->mRtuSerialMode, source, "rtu_serial_mode");
-        ConfigTools::readOptionalValue<RtuRtsMode>(this->mRtsMode, source, "rtu_rts_mode");
-        ConfigTools::readOptionalValue<int>(this->mRtsDelayUs, source, "rtu_rts_delay_us");
+        ConfigTools::readOptionalValue<RtuSerialMode>(mRtuSerialMode, source, "rtu_serial_mode");
+        ConfigTools::readOptionalValue<RtuRtsMode>(mRtsMode, source, "rtu_rts_mode");
+        ConfigTools::readOptionalValue<int>(mRtsDelayUs, source, "rtu_rts_delay_us");
 
         mWatchdogConfig.mDevicePath = mDevice;
     } else if (source["address"]) {
@@ -72,7 +80,7 @@ ModbusNetworkConfig::ModbusNetworkConfig(const YAML::Node& source) {
     }
 
     if (source["watchdog"]) {
-        ConfigTools::readOptionalValue<std::chrono::milliseconds>(this->mWatchdogConfig.mWatchPeriod, source["watchdog"], "watch_period");
+        ConfigTools::readOptionalValue<std::chrono::milliseconds>(mWatchdogConfig.mWatchPeriod, source["watchdog"], "watch_period");
     }
 }
 

diff --git a/libmodmqttsrv/config.hpp b/libmodmqttsrv/config.hpp
@@ -113,8 +113,14 @@ class ModbusNetworkConfig {
         std::chrono::milliseconds mResponseTimeout = std::chrono::milliseconds(500);
         std::chrono::milliseconds mResponseDataTimeout = std::chrono::seconds(0);
 
-        std::chrono::milliseconds mDelayBeforeCommand = std::chrono::seconds(0);
-        std::chrono::milliseconds mDelayBeforeFirstCommand = std::chrono::seconds(0);
+        bool hasDelayBeforeCommand() const { return mDelayBeforeCommand != nullptr; }
+        bool hasDelayBeforeFirstCommand() const { return mDelayBeforeFirstCommand != nullptr; }
+
+        const std::shared_ptr<std::chrono::milliseconds>& getDelayBeforeCommand() const { return mDelayBeforeCommand; }
+        const std::shared_ptr<std::chrono::milliseconds>& getDelayBeforeFirstCommand() const { return mDelayBeforeFirstCommand; }
+
+        void setDelayBeforeCommand(const std::chrono::milliseconds& pDelay) { mDelayBeforeCommand.reset(new std::chrono::milliseconds(pDelay)); }
+        void setDelayBeforeFirstCommand(const std::chrono::milliseconds& pDelay) { mDelayBeforeFirstCommand.reset(new std::chrono::milliseconds(pDelay)); }
 
         unsigned short mMaxWriteRetryCount = 2;
         unsigned short mMaxReadRetryCount = 1;
@@ -135,6 +141,9 @@ class ModbusNetworkConfig {
         int mPort = 0;
 
         ModbusWatchdogConfig mWatchdogConfig;
+    private:
+        std::shared_ptr<std::chrono::milliseconds> mDelayBeforeCommand;
+        std::shared_ptr<std::chrono::milliseconds> mDelayBeforeFirstCommand;
 };
 
 class MqttBrokerConfig {

diff --git a/libmodmqttsrv/modbus_slave.cpp b/libmodmqttsrv/modbus_slave.cpp
@@ -9,21 +9,30 @@ boost::log::sources::severity_logger<Log::severity> ModbusSlaveConfig::log;
 ModbusSlaveConfig::ModbusSlaveConfig(int pAddress, const YAML::Node& data)
     : mAddress(pAddress)
 {
-    if (ConfigTools::readOptionalValue<std::chrono::milliseconds>(mDelayBeforeCommand, data, "delay_before_poll")) {
+    ConfigTools::readOptionalValue<std::string>(mSlaveName, data, "name");
+
+    std::chrono::milliseconds tmpval;
+    if (ConfigTools::readOptionalValue<std::chrono::milliseconds>(tmpval, data, "delay_before_poll")) {
         BOOST_LOG_SEV(log, Log::warn) << "'delay_before_poll' is deprecated and will be removed in future releases. Rename it to 'delay_before_command'";
+        setDelayBeforeCommand(tmpval);
     }
 
-    if (ConfigTools::readOptionalValue<std::chrono::milliseconds>(mDelayBeforeFirstCommand, data, "delay_before_first_poll")) {
-        BOOST_LOG_SEV(log, Log::warn) << "'delay_before_first_poll' is deprecated and will be removed in future releases. Rename it to 'delay_before_first_command'";
+    if (ConfigTools::readOptionalValue<std::chrono::milliseconds>(tmpval, data, "delay_before_command")) {
+        setDelayBeforeCommand(tmpval);
     }
 
-    ConfigTools::readOptionalValue<std::string>(this->mSlaveName, data, "name");
 
-    ConfigTools::readOptionalValue<std::chrono::milliseconds>(this->mDelayBeforeCommand, data, "delay_before_command");
-    ConfigTools::readOptionalValue<std::chrono::milliseconds>(this->mDelayBeforeFirstCommand, data, "delay_before_first_command");
+    if (ConfigTools::readOptionalValue<std::chrono::milliseconds>(tmpval, data, "delay_before_first_poll")) {
+        BOOST_LOG_SEV(log, Log::warn) << "'delay_before_first_poll' is deprecated and will be removed in future releases. Rename it to 'delay_before_first_command'";
+        setDelayBeforeFirstCommand(tmpval);
+    }
+
+    if (ConfigTools::readOptionalValue<std::chrono::milliseconds>(tmpval, data, "delay_before_first_command")) {
+        setDelayBeforeFirstCommand(tmpval);
+    }
 
-    ConfigTools::readOptionalValue<unsigned short>(this->mMaxWriteRetryCount, data, "write_retries");
-    ConfigTools::readOptionalValue<unsigned short>(this->mMaxReadRetryCount, data, "read_retries");
+    ConfigTools::readOptionalValue<unsigned short>(mMaxWriteRetryCount, data, "write_retries");
+    ConfigTools::readOptionalValue<unsigned short>(mMaxReadRetryCount, data, "read_retries");
 }
 
 }
diff --git a/libmodmqttsrv/modbus_slave.hpp b/libmodmqttsrv/modbus_slave.hpp
@@ -15,11 +15,22 @@ class ModbusSlaveConfig {
         ModbusSlaveConfig(int pAddress, const YAML::Node& data);
         int mAddress;
         std::string mSlaveName;
-        std::chrono::milliseconds mDelayBeforeCommand = std::chrono::milliseconds::zero();
-        std::chrono::milliseconds mDelayBeforeFirstCommand = std::chrono::milliseconds::zero();
+
+        bool hasDelayBeforeCommand() const { return mDelayBeforeCommand != nullptr; }
+        bool hasDelayBeforeFirstCommand() const { return mDelayBeforeFirstCommand != nullptr; }
+
+        const std::shared_ptr<std::chrono::milliseconds>& getDelayBeforeCommand() const { return mDelayBeforeCommand; }
+        const std::shared_ptr<std::chrono::milliseconds>& getDelayBeforeFirstCommand() const { return mDelayBeforeFirstCommand; }
+
+        void setDelayBeforeCommand(const std::chrono::milliseconds& pDelay) { mDelayBeforeCommand.reset(new std::chrono::milliseconds(pDelay)); }
+        void setDelayBeforeFirstCommand(const std::chrono::milliseconds& pDelay) { mDelayBeforeFirstCommand.reset(new std::chrono::milliseconds(pDelay)); }
+
 
         unsigned short mMaxWriteRetryCount = 0;
         unsigned short mMaxReadRetryCount = 0;
+    private:
+        std::shared_ptr<std::chrono::milliseconds> mDelayBeforeCommand;
+        std::shared_ptr<std::chrono::milliseconds> mDelayBeforeFirstCommand;
 };
 
 }
diff --git a/libmodmqttsrv/modbus_thread.cpp b/libmodmqttsrv/modbus_thread.cpp
@@ -9,9 +9,11 @@
 namespace modmqttd {
 
 void
-setCommandDelays(RegisterCommand& cmd, const std::chrono::milliseconds& everyTime, const std::chrono::milliseconds& onChange) {
-    cmd.setDelayBeforeCommand(everyTime);
-    cmd.setDelayBeforeFirstCommand(onChange);
+setCommandDelays(RegisterCommand& cmd, const std::shared_ptr<const std::chrono::milliseconds>& everyTime, const std::shared_ptr<const std::chrono::milliseconds>& onChange) {
+    if (everyTime != nullptr)
+        cmd.setDelayBeforeCommand(*everyTime);
+    if (onChange != nullptr)
+        cmd.setDelayBeforeFirstCommand(*onChange);
 }
 
 void
@@ -37,13 +39,19 @@ ModbusThread::configure(const ModbusNetworkConfig& config) {
     mExecutor.init(mModbus);
     mWatchdog.init(config.mWatchdogConfig);
 
-    mDelayBeforeCommand = config.mDelayBeforeCommand;
-    mDelayBeforeFirstCommand = config.mDelayBeforeFirstCommand;
-    if (mDelayBeforeCommand.count() != 0 || mDelayBeforeFirstCommand.count() != 0) {
-        BOOST_LOG_SEV(log, Log::info) << "Global minimum delays set. Delay before every command "
-            << std::chrono::duration_cast<std::chrono::milliseconds>(mDelayBeforeCommand).count() << "ms"
-            << ", delay when slave changes "
-            << std::chrono::duration_cast<std::chrono::milliseconds>(mDelayBeforeFirstCommand).count() << "ms";
+    if (config.hasDelayBeforeCommand())
+        mDelayBeforeCommand = config.getDelayBeforeCommand();
+    if (config.hasDelayBeforeFirstCommand())
+        mDelayBeforeFirstCommand = config.getDelayBeforeFirstCommand();
+
+    if (mDelayBeforeCommand != nullptr) {
+        BOOST_LOG_SEV(log, Log::info) << "Network default delay before every command set to "
+            << std::chrono::duration_cast<std::chrono::milliseconds>(*mDelayBeforeCommand).count() << "ms";
+    }
+
+    if (mDelayBeforeFirstCommand != nullptr) {
+        BOOST_LOG_SEV(log, Log::info) << "Network default delay when slave changes set to"
+            << std::chrono::duration_cast<std::chrono::milliseconds>(*mDelayBeforeFirstCommand).count() << "ms";
     }
 
     mMaxReadRetryCount = config.mMaxReadRetryCount;
@@ -66,7 +74,7 @@ ModbusThread::setPollSpecification(const MsgRegisterPollSpecification& spec) {
             reg->setMaxRetryCounts(mMaxReadRetryCount, mMaxWriteRetryCount, true);
 
             if (slave_cfg != mSlaves.end()) {
-                setCommandDelays(*reg, slave_cfg->second.mDelayBeforeCommand, slave_cfg->second.mDelayBeforeFirstCommand);
+                setCommandDelays(*reg, slave_cfg->second.getDelayBeforeCommand(), slave_cfg->second.getDelayBeforeFirstCommand());
                 reg->setMaxRetryCounts(slave_cfg->second.mMaxReadRetryCount, slave_cfg->second.mMaxWriteRetryCount);
             }
 
@@ -106,7 +114,7 @@ ModbusThread::processWrite(const std::shared_ptr<MsgRegisterValues>& msg) {
     cmd->setMaxRetryCounts(mMaxReadRetryCount, mMaxWriteRetryCount, true);
     std::map<int, ModbusSlaveConfig>::const_iterator it = mSlaves.find(msg->mSlaveId);
     if (it != mSlaves.end()) {
-        setCommandDelays(*cmd, it->second.mDelayBeforeCommand, it->second.mDelayBeforeFirstCommand);
+        setCommandDelays(*cmd, it->second.getDelayBeforeCommand(), it->second.getDelayBeforeFirstCommand());
         cmd->setMaxRetryCounts(it->second.mMaxReadRetryCount, it->second.mMaxWriteRetryCount);
     }
 
@@ -157,7 +165,7 @@ ModbusThread::updateFromSlaveConfig(const ModbusSlaveConfig& pConfig) {
     std::map<int, std::vector<std::shared_ptr<RegisterPoll>>>::const_iterator slave_registers = registers.find(pConfig.mAddress);
     if (slave_registers != registers.end()) {
         for (auto it = slave_registers->second.begin(); it != slave_registers->second.end(); it++) {
-            setCommandDelays(**it, pConfig.mDelayBeforeCommand, pConfig.mDelayBeforeFirstCommand);
+            setCommandDelays(**it, pConfig.getDelayBeforeCommand(), pConfig.getDelayBeforeFirstCommand());
             (*it)->setMaxRetryCounts(pConfig.mMaxReadRetryCount, pConfig.mMaxWriteRetryCount);
         }
     }

diff --git a/libmodmqttsrv/modbus_thread.hpp b/libmodmqttsrv/modbus_thread.hpp
@@ -28,8 +28,8 @@ class ModbusThread {
 
         // global config
         std::string mNetworkName;
-        std::chrono::milliseconds mDelayBeforeCommand = std::chrono::milliseconds::zero();
-        std::chrono::milliseconds mDelayBeforeFirstCommand = std::chrono::milliseconds::zero();
+        std::shared_ptr<std::chrono::milliseconds> mDelayBeforeCommand;
+        std::shared_ptr<std::chrono::milliseconds> mDelayBeforeFirstCommand;
         short mMaxReadRetryCount;
         short mMaxWriteRetryCount;
 

diff --git a/libmodmqttsrv/modmqtt.cpp b/libmodmqttsrv/modmqtt.cpp
@@ -395,9 +395,6 @@ ModMqtt::initModbusClients(const YAML::Node& config) {
 
                     for(int addr = addr_range.first; addr <= addr_range.second; addr++) {
                         ModbusSlaveConfig slave_config(addr, ySlave);
-                        // if (slave_config.mDelayBeforeCommand != std::chrono::milliseconds::zero() && slave_config.mDelayBeforeFirstCommand != std::chrono::milliseconds::zero()) {
-                        //     BOOST_LOG_SEV(log, Log::warn) << "Ignoring delay_before_first_poll for slave " << slave_config.mAddress << " because delay_before_poll is set";
-                        // }
                         modbus->mToModbusQueue.enqueue(QueueItem::create(slave_config));
                         spec.merge(readModbusPollGroups(modbus_config.mName, slave_config.mAddress, ySlave["poll_groups"]));
 

diff --git a/unittests/modbus_silence_before_poll_tests.cpp b/unittests/modbus_silence_before_poll_tests.cpp
@@ -12,6 +12,8 @@ TestConfig config(R"(
     - name: tcptest
       address: localhost
       port: 501
+      # overwritten by slave config
+      delay_before_command: 1ms
       slaves:
         - address: 1
           delay_before_command: 15ms
@@ -46,6 +48,32 @@ TestConfig config(R"(
         server.stop();
     }
 
+    SECTION("should respect delay_before_command on network level") {
+        config.mYAML["modbus"]["networks"][0]["slaves"][0].remove("delay_before_command");
+        config.mYAML["modbus"]["networks"][0]["delay_before_command"] = "30ms";
+
+        std::cerr << config.toString() << std::endl;
+        MockedModMqttServerThread server(config.toString());
+
+        server.setModbusRegisterValue("tcptest", 1, 2, modmqttd::RegisterType::HOLDING, 1);
+        server.start();
+        // default mocked modbus read time is 5ms per register
+        // refresh is 5ms so without silence poll should be executed every 10ms
+        server.waitForPublish("test_sensor/state");
+        REQUIRE(server.mqttValue("test_sensor/state") == "1");
+        std::chrono::time_point<std::chrono::steady_clock> first_poll_ts = server.getLastPollTime();
+
+        // we should respect 30ms silence
+        server.setModbusRegisterValue("tcptest", 1, 2, modmqttd::RegisterType::HOLDING, 2);
+        server.waitForPublish("test_sensor/state");
+        auto ptime = server.getLastPollTime() - first_poll_ts;
+        REQUIRE(ptime > std::chrono::milliseconds(30));
+        REQUIRE(server.mqttValue("test_sensor/state") == "2");
+
+        server.stop();
+    }
+
+
     SECTION("should be ignored when next poll is later") {
         config.mYAML["mqtt"]["objects"][0]["state"]["refresh"] = "25ms";
         MockedModMqttServerThread server(config.toString());