From 93f733311f26913a10264a25cdb2d1eb4120b067 Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Wed, 6 Apr 2022 19:25:40 +0200 Subject: [PATCH 1/2] Fix modbus RTU warning prints --- libnymea-core/hardware/modbus/modbusrtumasterimpl.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libnymea-core/hardware/modbus/modbusrtumasterimpl.cpp b/libnymea-core/hardware/modbus/modbusrtumasterimpl.cpp index a11bf6e7..a1ae99f4 100644 --- a/libnymea-core/hardware/modbus/modbusrtumasterimpl.cpp +++ b/libnymea-core/hardware/modbus/modbusrtumasterimpl.cpp @@ -462,7 +462,7 @@ ModbusRtuReply *ModbusRtuMasterImpl::writeCoils(int slaveAddress, int registerAd // Check if the reply finished with an error if (modbusReply->error() != QModbusDevice::NoError) { - qCWarning(dcModbusRtu()) << "Read coil request finished with error" << modbusReply->error() << modbusReply->errorString(); + qCWarning(dcModbusRtu()) << "Write coil request finished with error" << modbusReply->error() << modbusReply->errorString(); emit reply->errorOccurred(reply->error()); emit reply->finished(); return; @@ -475,7 +475,7 @@ ModbusRtuReply *ModbusRtuMasterImpl::writeCoils(int slaveAddress, int registerAd }); connect(modbusReply, &QModbusReply::errorOccurred, modbusReply, [=](QModbusDevice::Error error){ - qCWarning(dcModbusRtu()) << "Read coil request finished with error" << error << modbusReply->errorString(); + qCWarning(dcModbusRtu()) << "Write coil request finished with error" << error << modbusReply->errorString(); reply->setFinished(true); reply->setError(static_cast(modbusReply->error())); reply->setErrorString(modbusReply->errorString()); @@ -518,7 +518,7 @@ ModbusRtuReply *ModbusRtuMasterImpl::writeHoldingRegisters(int slaveAddress, int // Check if the reply finished with an error if (modbusReply->error() != QModbusDevice::NoError) { - qCWarning(dcModbusRtu()) << "Read coil request finished with error" << modbusReply->error() << modbusReply->errorString(); + qCWarning(dcModbusRtu()) << "Write holding register request finished with error" << modbusReply->error() << modbusReply->errorString(); emit reply->errorOccurred(reply->error()); emit reply->finished(); return; @@ -531,7 +531,7 @@ ModbusRtuReply *ModbusRtuMasterImpl::writeHoldingRegisters(int slaveAddress, int }); connect(modbusReply, &QModbusReply::errorOccurred, modbusReply, [=](QModbusDevice::Error error){ - qCWarning(dcModbusRtu()) << "Read coil request finished with error" << error << modbusReply->errorString(); + qCWarning(dcModbusRtu()) << "Write holding register request finished with error" << error << modbusReply->errorString(); reply->setFinished(true); reply->setError(static_cast(modbusReply->error())); reply->setErrorString(modbusReply->errorString()); From b2cc7ac769414d6400797021f8f8a1f66354e33f Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Fri, 8 Apr 2022 14:30:41 +0200 Subject: [PATCH 2/2] Add a timeout to ModbusRtuReply QModbusReply seems to never finish in some rare cases: For instance if a request is sent and before the reply arrives, the modbus adapter is unplugged. This change should keep upper logic working which relies on requests finishing eventually. Also fixes the issue that we emit finished() multiple times if QModbusClient retries multiple times and emits error() for each attempt. We're only interested in a single result anyways. --- .../hardware/modbus/modbusrtumasterimpl.cpp | 86 ++++--------------- .../hardware/modbus/modbusrtureplyimpl.cpp | 8 +- .../hardware/modbus/modbusrtureplyimpl.h | 3 +- 3 files changed, 27 insertions(+), 70 deletions(-) diff --git a/libnymea-core/hardware/modbus/modbusrtumasterimpl.cpp b/libnymea-core/hardware/modbus/modbusrtumasterimpl.cpp index a1ae99f4..302ba9af 100644 --- a/libnymea-core/hardware/modbus/modbusrtumasterimpl.cpp +++ b/libnymea-core/hardware/modbus/modbusrtumasterimpl.cpp @@ -238,9 +238,10 @@ ModbusRtuReply *ModbusRtuMasterImpl::readCoil(int slaveAddress, int registerAddr QModbusDataUnit request = QModbusDataUnit(QModbusDataUnit::RegisterType::Coils, registerAddress, size); QModbusReply *modbusReply = m_modbus->sendReadRequest(request, slaveAddress); - connect(modbusReply, &QModbusReply::finished, modbusReply, [=](){ - modbusReply->deleteLater(); + // Cleaning up modbusReply when our reply finishes, regardless if that happened because modbusReply finished or reply timeouted + connect(reply, &ModbusRtuReply::finished, modbusReply, &QModbusReply::deleteLater); + connect(modbusReply, &QModbusReply::finished, reply, [=](){ // Fill common reply data reply->setFinished(true); reply->setError(static_cast(modbusReply->error())); @@ -260,15 +261,6 @@ ModbusRtuReply *ModbusRtuMasterImpl::readCoil(int slaveAddress, int registerAddr emit reply->finished(); }); - connect(modbusReply, &QModbusReply::errorOccurred, modbusReply, [=](QModbusDevice::Error error){ - qCWarning(dcModbusRtu()) << "Read coil request finished with error" << error << modbusReply->errorString(); - reply->setFinished(true); - reply->setError(static_cast(modbusReply->error())); - reply->setErrorString(modbusReply->errorString()); - emit reply->errorOccurred(reply->error()); - emit reply->finished(); - }); - return qobject_cast(reply); #else Q_UNUSED(slaveAddress) @@ -291,9 +283,10 @@ ModbusRtuReply *ModbusRtuMasterImpl::readDiscreteInput(int slaveAddress, int reg QModbusDataUnit request = QModbusDataUnit(QModbusDataUnit::RegisterType::DiscreteInputs, registerAddress, size); QModbusReply *modbusReply = m_modbus->sendReadRequest(request, slaveAddress); - connect(modbusReply, &QModbusReply::finished, modbusReply, [=](){ - modbusReply->deleteLater(); + // Cleaning up modbusReply when our reply finishes, regardless if that happened because modbusReply finished or reply timeouted + connect(reply, &ModbusRtuReply::finished, modbusReply, &QModbusReply::deleteLater); + connect(modbusReply, &QModbusReply::finished, reply, [=](){ // Fill common reply data reply->setFinished(true); reply->setError(static_cast(modbusReply->error())); @@ -313,15 +306,6 @@ ModbusRtuReply *ModbusRtuMasterImpl::readDiscreteInput(int slaveAddress, int reg emit reply->finished(); }); - connect(modbusReply, &QModbusReply::errorOccurred, modbusReply, [=](QModbusDevice::Error error){ - qCWarning(dcModbusRtu()) << "Read descrete inputs request finished with error" << error << modbusReply->errorString(); - reply->setFinished(true); - reply->setError(static_cast(modbusReply->error())); - reply->setErrorString(modbusReply->errorString()); - emit reply->errorOccurred(reply->error()); - emit reply->finished(); - }); - return qobject_cast(reply); #else Q_UNUSED(slaveAddress) @@ -344,9 +328,10 @@ ModbusRtuReply *ModbusRtuMasterImpl::readInputRegister(int slaveAddress, int reg QModbusDataUnit request = QModbusDataUnit(QModbusDataUnit::RegisterType::InputRegisters, registerAddress, size); QModbusReply *modbusReply = m_modbus->sendReadRequest(request, slaveAddress); - connect(modbusReply, &QModbusReply::finished, modbusReply, [=](){ - modbusReply->deleteLater(); + // Cleaning up modbusReply when our reply finishes, regardless if that happened because modbusReply finished or reply timeouted + connect(reply, &ModbusRtuReply::finished, modbusReply, &QModbusReply::deleteLater); + connect(modbusReply, &QModbusReply::finished, reply, [=](){ // Fill common reply data reply->setFinished(true); reply->setError(static_cast(modbusReply->error())); @@ -366,15 +351,6 @@ ModbusRtuReply *ModbusRtuMasterImpl::readInputRegister(int slaveAddress, int reg emit reply->finished(); }); - connect(modbusReply, &QModbusReply::errorOccurred, modbusReply, [=](QModbusDevice::Error error){ - qCWarning(dcModbusRtu()) << "Read input registers request finished with error" << error << modbusReply->errorString(); - reply->setFinished(true); - reply->setError(static_cast(modbusReply->error())); - reply->setErrorString(modbusReply->errorString()); - emit reply->errorOccurred(reply->error()); - emit reply->finished(); - }); - return qobject_cast(reply); #else Q_UNUSED(slaveAddress) @@ -397,9 +373,10 @@ ModbusRtuReply *ModbusRtuMasterImpl::readHoldingRegister(int slaveAddress, int r QModbusDataUnit request = QModbusDataUnit(QModbusDataUnit::RegisterType::HoldingRegisters, registerAddress, size); QModbusReply *modbusReply = m_modbus->sendReadRequest(request, slaveAddress); - connect(modbusReply, &QModbusReply::finished, modbusReply, [=](){ - modbusReply->deleteLater(); + // Cleaning up modbusReply when our reply finishes, regardless if that happened because modbusReply finished or reply timeouted + connect(reply, &ModbusRtuReply::finished, modbusReply, &QModbusReply::deleteLater); + connect(modbusReply, &QModbusReply::finished, reply, [=](){ // Fill common reply data reply->setFinished(true); reply->setError(static_cast(modbusReply->error())); @@ -419,15 +396,6 @@ ModbusRtuReply *ModbusRtuMasterImpl::readHoldingRegister(int slaveAddress, int r emit reply->finished(); }); - connect(modbusReply, &QModbusReply::errorOccurred, modbusReply, [=](QModbusDevice::Error error){ - qCWarning(dcModbusRtu()) << "Read holding registers request finished with error" << error << modbusReply->errorString(); - reply->setFinished(true); - reply->setError(static_cast(modbusReply->error())); - reply->setErrorString(modbusReply->errorString()); - emit reply->errorOccurred(reply->error()); - emit reply->finished(); - }); - return qobject_cast(reply); #else Q_UNUSED(slaveAddress) @@ -449,12 +417,12 @@ ModbusRtuReply *ModbusRtuMasterImpl::writeCoils(int slaveAddress, int registerAd // Create the actual modbus lib reply QModbusDataUnit request = QModbusDataUnit(QModbusDataUnit::RegisterType::Coils, registerAddress, values.length()); request.setValues(values); - QModbusReply *modbusReply = m_modbus->sendWriteRequest(request, slaveAddress); - connect(modbusReply, &QModbusReply::finished, modbusReply, [=](){ - modbusReply->deleteLater(); + // Cleaning up modbusReply when our reply finishes, regardless if that happened because modbusReply finished or reply timeouted + connect(reply, &ModbusRtuReply::finished, modbusReply, &QModbusReply::deleteLater); + connect(modbusReply, &QModbusReply::finished, reply, [=](){ // Fill common reply data reply->setFinished(true); reply->setError(static_cast(modbusReply->error())); @@ -474,15 +442,6 @@ ModbusRtuReply *ModbusRtuMasterImpl::writeCoils(int slaveAddress, int registerAd emit reply->finished(); }); - connect(modbusReply, &QModbusReply::errorOccurred, modbusReply, [=](QModbusDevice::Error error){ - qCWarning(dcModbusRtu()) << "Write coil request finished with error" << error << modbusReply->errorString(); - reply->setFinished(true); - reply->setError(static_cast(modbusReply->error())); - reply->setErrorString(modbusReply->errorString()); - emit reply->errorOccurred(reply->error()); - emit reply->finished(); - }); - return qobject_cast(reply); #else Q_UNUSED(slaveAddress) @@ -505,12 +464,12 @@ ModbusRtuReply *ModbusRtuMasterImpl::writeHoldingRegisters(int slaveAddress, int // Create the actual modbus lib reply QModbusDataUnit request = QModbusDataUnit(QModbusDataUnit::RegisterType::HoldingRegisters, registerAddress, values.length()); request.setValues(values); - QModbusReply *modbusReply = m_modbus->sendWriteRequest(request, slaveAddress); - connect(modbusReply, &QModbusReply::finished, modbusReply, [=](){ - modbusReply->deleteLater(); + // Cleaning up modbusReply when our reply finishes, regardless if that happened because modbusReply finished or reply timeouted + connect(reply, &ModbusRtuReply::finished, modbusReply, &QModbusReply::deleteLater); + connect(modbusReply, &QModbusReply::finished, reply, [=](){ // Fill common reply data reply->setFinished(true); reply->setError(static_cast(modbusReply->error())); @@ -530,15 +489,6 @@ ModbusRtuReply *ModbusRtuMasterImpl::writeHoldingRegisters(int slaveAddress, int emit reply->finished(); }); - connect(modbusReply, &QModbusReply::errorOccurred, modbusReply, [=](QModbusDevice::Error error){ - qCWarning(dcModbusRtu()) << "Write holding register request finished with error" << error << modbusReply->errorString(); - reply->setFinished(true); - reply->setError(static_cast(modbusReply->error())); - reply->setErrorString(modbusReply->errorString()); - emit reply->errorOccurred(reply->error()); - emit reply->finished(); - }); - return qobject_cast(reply); #else Q_UNUSED(slaveAddress) diff --git a/libnymea-core/hardware/modbus/modbusrtureplyimpl.cpp b/libnymea-core/hardware/modbus/modbusrtureplyimpl.cpp index dc00ac94..0388110a 100644 --- a/libnymea-core/hardware/modbus/modbusrtureplyimpl.cpp +++ b/libnymea-core/hardware/modbus/modbusrtureplyimpl.cpp @@ -37,7 +37,13 @@ ModbusRtuReplyImpl::ModbusRtuReplyImpl(int slaveAddress, int registerAddress, QO m_slaveAddress(slaveAddress), m_registerAddress(registerAddress) { - + m_timeoutTimer.start(10000); + connect(&m_timeoutTimer, &QTimer::timeout, this, [=](){ + if (!m_finished) { + m_error = TimeoutError; + emit errorOccurred(TimeoutError); + } + }); } bool ModbusRtuReplyImpl::isFinished() const diff --git a/libnymea-core/hardware/modbus/modbusrtureplyimpl.h b/libnymea-core/hardware/modbus/modbusrtureplyimpl.h index 5851b423..f52575e4 100644 --- a/libnymea-core/hardware/modbus/modbusrtureplyimpl.h +++ b/libnymea-core/hardware/modbus/modbusrtureplyimpl.h @@ -32,6 +32,7 @@ #define MODBUSRTUREPLYIMPL_H #include +#include #include "hardware/modbus/modbusrtureply.h" @@ -65,7 +66,7 @@ private: Error m_error = UnknownError; QString m_errorString; QVector m_result; - + QTimer m_timeoutTimer; }; }