diff --git a/libnymea-modbus/modbustcpmaster.cpp b/libnymea-modbus/modbustcpmaster.cpp index 4a332fa..b689921 100644 --- a/libnymea-modbus/modbustcpmaster.cpp +++ b/libnymea-modbus/modbustcpmaster.cpp @@ -453,7 +453,7 @@ QUuid ModbusTCPMaster::writeHoldingRegister(uint slaveAddress, uint registerAddr void ModbusTCPMaster::onModbusErrorOccurred(QModbusDevice::Error error) { - qCWarning(dcModbusTcpMaster()) << "An error occured" << error; + qCWarning(dcModbusTcpMaster()) << "An error occurred" << error; } void ModbusTCPMaster::onModbusStateChanged(QModbusDevice::State state) diff --git a/libnymea-sunspec/sunspecconnection.cpp b/libnymea-sunspec/sunspecconnection.cpp index f07de35..4f852d8 100644 --- a/libnymea-sunspec/sunspecconnection.cpp +++ b/libnymea-sunspec/sunspecconnection.cpp @@ -41,6 +41,7 @@ SunSpecConnection::SunSpecConnection(const QHostAddress &hostAddress, uint port, m_port(port), m_slaveId(slaveId) { + qCDebug(dcSunSpec()) << "Creating connection for" << QString("%1:%2").arg(m_hostAddress.toString()).arg(m_port); createConnection(); } @@ -51,6 +52,7 @@ SunSpecConnection::SunSpecConnection(const QHostAddress &hostAddress, uint port, m_slaveId(slaveId), m_byteOrder(byteOrder) { + qCDebug(dcSunSpec()) << "Creating connection for" << QString("%1:%2").arg(m_hostAddress.toString()).arg(m_port); createConnection(); } @@ -68,8 +70,6 @@ void SunSpecConnection::setHostAddress(const QHostAddress &hostAddress) { m_hostAddress = hostAddress; m_modbusTcpClient->setConnectionParameter(QModbusDevice::NetworkAddressParameter, m_hostAddress.toString()); - - // TODO: reconnect if different } uint SunSpecConnection::port() const @@ -81,9 +81,6 @@ void SunSpecConnection::setPort(uint port) { m_port = port; m_modbusTcpClient->setConnectionParameter(QModbusDevice::NetworkPortParameter, m_port); - - // TODO: reconnect if different - } uint SunSpecConnection::slaveId() const @@ -123,7 +120,7 @@ void SunSpecConnection::setNumberOfRetries(uint retries) bool SunSpecConnection::connected() const { - return m_modbusTcpClient->state() == QModbusDevice::ConnectedState; + return m_connected; } bool SunSpecConnection::discoveryRunning() const @@ -143,6 +140,20 @@ void SunSpecConnection::disconnectDevice() m_modbusTcpClient->disconnectDevice(); } +bool SunSpecConnection::reconnectDevice() +{ + // Recreate the entire connection so we clean up also any pending replies + qCDebug(dcSunSpec()) << "Reconnecting" << this << "..."; + if (m_modbusTcpClient) { + m_modbusTcpClient->disconnectDevice(); + delete m_modbusTcpClient; + m_modbusTcpClient = nullptr; + } + + createConnection(); + return connectDevice(); +} + quint16 SunSpecConnection::baseRegister() const { return m_baseRegister; @@ -153,6 +164,36 @@ QList SunSpecConnection::models() const return m_models; } +QModbusReply *SunSpecConnection::sendReadRequest(const QModbusDataUnit &read, int serverAddress) +{ + if (!m_modbusTcpClient) + return nullptr; + + QModbusReply *reply = m_modbusTcpClient->sendReadRequest(read, serverAddress); + monitorTimoutErrors(reply); + return reply; +} + +QModbusReply *SunSpecConnection::sendWriteRequest(const QModbusDataUnit &write, int serverAddress) +{ + if (!m_modbusTcpClient) + return nullptr; + + QModbusReply *reply = m_modbusTcpClient->sendWriteRequest(write, serverAddress); + monitorTimoutErrors(reply); + return reply; +} + +QModbusReply *SunSpecConnection::sendRawRequest(const QModbusRequest &request, int serverAddress) +{ + if (!m_modbusTcpClient) + return nullptr; + + QModbusReply *reply = m_modbusTcpClient->sendRawRequest(request, serverAddress); + monitorTimoutErrors(reply); + return reply; +} + bool SunSpecConnection::startDiscovery() { // Verify connection state @@ -186,7 +227,6 @@ bool SunSpecConnection::startDiscovery() void SunSpecConnection::createConnection() { - qCDebug(dcSunSpec()) << "Creating connection for" << QString("%1:%2").arg(m_hostAddress.toString()).arg(m_port); m_modbusTcpClient = new QModbusTcpClient(this); m_modbusTcpClient->setConnectionParameter(QModbusDevice::NetworkPortParameter, m_port); m_modbusTcpClient->setConnectionParameter(QModbusDevice::NetworkAddressParameter, m_hostAddress.toString()); @@ -207,19 +247,20 @@ void SunSpecConnection::createConnection() }); connect(m_modbusTcpClient, &QModbusTcpClient::stateChanged, this, [this](QModbusDevice::State state){ - bool connected = (state == QModbusDevice::ConnectedState); - bool disconnected = (state == QModbusDevice::UnconnectedState); - - if (connected) { + qCDebug(dcSunSpec()) << this << "client state changed" << state; + if (!m_connected && state == QModbusDevice::ConnectedState) { m_reconnectTimer.stop(); - emit connectedChanged(true); + m_timoutReplyCounter = 0; + m_connected = true; + emit connectedChanged(m_connected); return; } - if (disconnected) { + if (m_connected && state == QModbusDevice::UnconnectedState) { // Try to reconnect in 10 seconds m_reconnectTimer.start(); - emit connectedChanged(false); + m_connected = false; + emit connectedChanged(m_connected); return; } }); @@ -436,9 +477,9 @@ void SunSpecConnection::scanModelsOnBaseRegister(quint16 offset) // Scan next model block, current offset + 2 header bytes + model length scanModelsOnBaseRegister(offset + 2 + modelLength); } else { - qCWarning(dcSunSpec()) << "Error occured while reading model header from" << this << "using offset" << offset << m_modbusTcpClient->errorString(); + qCWarning(dcSunSpec()) << "Error occurred while reading model header from" << this << "using offset" << offset << m_modbusTcpClient->errorString(); if (!m_modelDiscoveryResult.isEmpty()) { - qCWarning(dcSunSpec()) << "Error occured but already discovered" << m_modelDiscoveryResult.count() << "models. Continue with the discovered models, but the discovery may be incomplete due to header reading errors."; + qCWarning(dcSunSpec()) << "Error occurred but already discovered" << m_modelDiscoveryResult.count() << "models. Continue with the discovered models, but the discovery may be incomplete due to header reading errors."; qCDebug(dcSunSpec()) << "Scan for SunSpec models on" << this << m_baseRegister << "finished successfully"; processDiscoveryResult(); } else { @@ -449,6 +490,36 @@ void SunSpecConnection::scanModelsOnBaseRegister(quint16 offset) }); } +void SunSpecConnection::monitorTimoutErrors(QModbusReply *reply) +{ + // Some modbus device over time seem to stop responding randomly but keep the connection up. + // All replies finish repeatedly with the timeout error. Normally a reconnect will fix this behaviour. + // In order to minimize the downtime and having proper logs when this happens, we monitor every + // reply sent to the sunspec connection from the models and trigger a reconnect if to many timouts occurred in a row. + + if (!reply) return; + + connect(reply, &QModbusReply::errorOccurred, this, [this](QModbusDevice::Error error){ + // Note: we handle + switch (error) { + case QModbusDevice::NoError: + // If any reply finished succssfully, we can reset the counter since the device + // is responding somehow and the workaround is not required + m_timoutReplyCounter = 0; + break; + case QModbusDevice::TimeoutError: + m_timoutReplyCounter++; + if (m_timoutReplyCounter > m_timoutReplyCounterLimit) { + qCWarning(dcSunSpec()) << "More than" << m_timoutReplyCounterLimit << "modbus replies finished with timeout error on" << this << "Triggering a reconnect..."; + reconnectDevice(); + } + break; + default: + break; + } + }); +} + QDebug operator<<(QDebug debug, SunSpecConnection *connection) { debug.nospace().noquote() << "SunSpecConnection(" << connection->hostAddress().toString() << ":" << connection->port() << ", Slave ID: " << connection->slaveId() << ")"; diff --git a/libnymea-sunspec/sunspecconnection.h b/libnymea-sunspec/sunspecconnection.h index 2f7b7c9..c063fe9 100644 --- a/libnymea-sunspec/sunspecconnection.h +++ b/libnymea-sunspec/sunspecconnection.h @@ -73,15 +73,20 @@ public: bool connected() const; bool discoveryRunning() const; - bool connectDevice(); - void disconnectDevice(); - quint16 baseRegister() const; QList models() const; + // Helper methods for internal queue handling if enabled + QModbusReply *sendReadRequest(const QModbusDataUnit &read, int serverAddress); + QModbusReply *sendWriteRequest(const QModbusDataUnit &write, int serverAddress); + QModbusReply *sendRawRequest(const QModbusRequest &request, int serverAddress); + public slots: bool startDiscovery(); + bool connectDevice(); + void disconnectDevice(); + bool reconnectDevice(); signals: void connectedChanged(bool connected); @@ -95,6 +100,7 @@ private: uint m_port; int m_slaveId = 1; QTimer m_reconnectTimer; + bool m_connected = false; quint16 m_baseRegister = 40000; QQueue m_baseRegisterQueue; @@ -112,6 +118,9 @@ private: QList m_uninitializedModels; SunSpecDataPoint::ByteOrder m_byteOrder = SunSpecDataPoint::ByteOrderLittleEndian; + int m_timoutReplyCounter = 0; + int m_timoutReplyCounterLimit = 16; + void createConnection(); void processDiscoveryResult(); @@ -124,6 +133,7 @@ private: void scanModelsOnBaseRegister(quint16 offset = 2); + void monitorTimoutErrors(QModbusReply *reply); }; QDebug operator<<(QDebug debug, SunSpecConnection *connection); diff --git a/libnymea-sunspec/sunspecmodel.cpp b/libnymea-sunspec/sunspecmodel.cpp index 27ef1a2..c266555 100644 --- a/libnymea-sunspec/sunspecmodel.cpp +++ b/libnymea-sunspec/sunspecmodel.cpp @@ -106,55 +106,55 @@ void SunSpecModel::readBlockData() { // Read the block data, start register + 2 header reisters (id, length) QModbusDataUnit request = QModbusDataUnit(QModbusDataUnit::RegisterType::HoldingRegisters, m_modbusStartRegister, m_modelLength + 2); - if (QModbusReply *reply = m_connection->modbusTcpClient()->sendReadRequest(request, m_connection->slaveId())) { - if (!reply->isFinished()) { - connect(reply, &QModbusReply::finished, reply, &QModbusReply::deleteLater); - connect(reply, &QModbusReply::finished, this, [=]() { - if (reply->error() != QModbusDevice::NoError) { - qCWarning(dcSunSpec()) << name() << description() << "Read block data response error:" << reply->error(); - return; - } - - const QModbusDataUnit unit = reply->result(); - qCDebug(dcSunSpecModelData()) << "-->" << "Received block data" << this << unit.values().count() << SunSpecDataPoint::registersToString(unit.values()); - m_blockData = unit.values(); - emit blockDataChanged(m_blockData); - - if (m_blockData.count() != m_modelLength + 2) { - qCWarning(dcSunSpecModelData()) << "Received invalid block data count from read block data request. Model lenght:" << m_modelLength << "Response block count:" << m_blockData.count(); - return; - } - - // Fill the data points - foreach (const QString &dataPointName, m_dataPoints.keys()) { - QVector rawData = m_blockData.mid(m_dataPoints[dataPointName].addressOffset(), m_dataPoints[dataPointName].size()); - m_dataPoints[dataPointName].setRawData(rawData); - qCDebug(dcSunSpecModelData()) << "Set raw data:" << m_dataPoints[dataPointName] << SunSpecDataPoint::registersToString(rawData) << (m_dataPoints[dataPointName].isValid() ? "Valid" : "Invalid"); - } - - // Fill the private member data using the data points - processBlockData(); - - // Make sure initialized gets called - setInitializedFinished(); - - // Inform about the new block data - emit blockUpdated(); - }); - - connect(reply, &QModbusReply::errorOccurred, this, [this, reply] (QModbusDevice::Error error) { - qCWarning(dcSunSpecModelData()) << name() << description() << "Modbus reply while reading block data. Error:" << error; - }); - - } else { - qCWarning(dcSunSpecModelData()) << "Read block data error: " << m_connection->modbusTcpClient()->errorString(); - reply->deleteLater(); // broadcast replies return immediately - return; - } - } else { - qCWarning(dcSunSpecModelData()) << "Read block data error: " << m_connection->modbusTcpClient()->errorString(); + QModbusReply *reply = m_connection->sendReadRequest(request, m_connection->slaveId()); + if (!reply) { + qCDebug(dcSunSpecModelData()) << "Read block data error: " << m_connection->modbusTcpClient()->errorString(); return; } + + if (reply->isFinished()) { + qCWarning(dcSunSpecModelData()) << "Read block data error: " << m_connection->modbusTcpClient()->errorString(); + reply->deleteLater(); // broadcast replies return immediately + return; + } + + connect(reply, &QModbusReply::finished, reply, &QModbusReply::deleteLater); + connect(reply, &QModbusReply::finished, this, [=]() { + if (reply->error() != QModbusDevice::NoError) { + qCWarning(dcSunSpec()) << name() << description() << "Read block data response error:" << reply->error(); + return; + } + + const QModbusDataUnit unit = reply->result(); + qCDebug(dcSunSpecModelData()) << "-->" << "Received block data" << this << unit.values().count() << SunSpecDataPoint::registersToString(unit.values()); + m_blockData = unit.values(); + emit blockDataChanged(m_blockData); + + if (m_blockData.count() != m_modelLength + 2) { + qCWarning(dcSunSpecModelData()) << "Received invalid block data count from read block data request. Model lenght:" << m_modelLength << "Response block count:" << m_blockData.count(); + return; + } + + // Fill the data points + foreach (const QString &dataPointName, m_dataPoints.keys()) { + QVector rawData = m_blockData.mid(m_dataPoints[dataPointName].addressOffset(), m_dataPoints[dataPointName].size()); + m_dataPoints[dataPointName].setRawData(rawData); + qCDebug(dcSunSpecModelData()) << "Set raw data:" << m_dataPoints[dataPointName] << SunSpecDataPoint::registersToString(rawData) << (m_dataPoints[dataPointName].isValid() ? "Valid" : "Invalid"); + } + + // Fill the private member data using the data points + processBlockData(); + + // Make sure initialized gets called + setInitializedFinished(); + + // Inform about the new block data + emit blockUpdated(); + }); + + connect(reply, &QModbusReply::errorOccurred, this, [this, reply] (QModbusDevice::Error error) { + qCWarning(dcSunSpecModelData()) << name() << description() << "Modbus reply while reading block data. Error:" << error << reply->errorString(); + }); } bool SunSpecModel::operator ==(const SunSpecModel &other) const diff --git a/sunspec/integrationpluginsunspec.cpp b/sunspec/integrationpluginsunspec.cpp index c4ad08d..f918fa4 100644 --- a/sunspec/integrationpluginsunspec.cpp +++ b/sunspec/integrationpluginsunspec.cpp @@ -81,20 +81,6 @@ void IntegrationPluginSunSpec::init() m_connectionSlaveIdParamTypeIds.insert(sunspecConnectionThingClassId, sunspecConnectionThingSlaveIdParamTypeId); m_connectionSlaveIdParamTypeIds.insert(solarEdgeConnectionThingClassId, solarEdgeConnectionThingSlaveIdParamTypeId); - // Connected state for all things - m_connectedStateTypeIds.insert(sunspecConnectionThingClassId, sunspecConnectionConnectedStateTypeId); - m_connectedStateTypeIds.insert(solarEdgeConnectionThingClassId, solarEdgeConnectionConnectedStateTypeId); - m_connectedStateTypeIds.insert(solarEdgeBatteryThingClassId, solarEdgeBatteryConnectedStateTypeId); - - // Child things - m_connectedStateTypeIds.insert(sunspecStorageThingClassId, sunspecStorageConnectedStateTypeId); - m_connectedStateTypeIds.insert(sunspecSinglePhaseInverterThingClassId, sunspecSinglePhaseInverterConnectedStateTypeId); - m_connectedStateTypeIds.insert(sunspecSplitPhaseInverterThingClassId, sunspecSplitPhaseInverterConnectedStateTypeId); - m_connectedStateTypeIds.insert(sunspecThreePhaseInverterThingClassId, sunspecThreePhaseInverterConnectedStateTypeId); - m_connectedStateTypeIds.insert(sunspecSinglePhaseMeterThingClassId, sunspecSinglePhaseMeterConnectedStateTypeId); - m_connectedStateTypeIds.insert(sunspecSplitPhaseMeterThingClassId, sunspecSplitPhaseMeterConnectedStateTypeId); - m_connectedStateTypeIds.insert(sunspecThreePhaseMeterThingClassId, sunspecThreePhaseMeterConnectedStateTypeId); - // Params for sunspec things m_modelIdParamTypeIds.insert(sunspecSinglePhaseInverterThingClassId, sunspecSinglePhaseInverterThingModelIdParamTypeId); m_modelIdParamTypeIds.insert(sunspecSplitPhaseInverterThingClassId, sunspecSplitPhaseInverterThingModelIdParamTypeId); @@ -528,12 +514,17 @@ void IntegrationPluginSunSpec::setupConnection(ThingSetupInfo *info) // Update all child things connected states for this connection connect(connection, &SunSpecConnection::connectedChanged, thing, [this, connection, thing] (bool connected) { - qCDebug(dcSunSpec()) << connection << (connected ? "connected" : "disconnected"); - thing->setStateValue(m_connectedStateTypeIds.value(thing->thingClassId()), connected); + if (connected) { + qCDebug(dcSunSpec()) << connection << "connected"; + } else { + qCWarning(dcSunSpec()) << connection << "disconnected"; + } + + thing->setStateValue("connected", connected); // Update connected state of child things foreach (Thing *child, myThings().filterByParentId(thing->id())) { - child->setStateValue(m_connectedStateTypeIds.value(child->thingClassId()), connected); + child->setStateValue("connected", connected); // Refresh childs if connected successfully if (connected && m_sunSpecThings.contains(child)) { @@ -702,7 +693,7 @@ void IntegrationPluginSunSpec::searchSolarEdgeBattery(SunSpecConnection *connect // If init failed, no battery connected if (!success) { - qCWarning(dcSunSpec()) << "No SolarEdge battery connected on register" << startRegister << ". Not creating battery device."; + qCDebug(dcSunSpec()) << "No SolarEdge battery connected on register" << startRegister << "- not creating thing."; return; } diff --git a/sunspec/integrationpluginsunspec.h b/sunspec/integrationpluginsunspec.h index d4a5a40..0e198c8 100644 --- a/sunspec/integrationpluginsunspec.h +++ b/sunspec/integrationpluginsunspec.h @@ -59,9 +59,6 @@ public: void executeAction(ThingActionInfo *info) override; private: - // Connected state for all things - QHash m_connectedStateTypeIds; - // SunSpec Connection params map QHash m_connectionIpParamTypeIds; QHash m_connectionPortParamTypeIds; diff --git a/sunspec/integrationpluginsunspec.json b/sunspec/integrationpluginsunspec.json index abdf3ad..15282f1 100644 --- a/sunspec/integrationpluginsunspec.json +++ b/sunspec/integrationpluginsunspec.json @@ -17,7 +17,7 @@ "displayName": "Timout", "type": "uint", "unit": "MilliSeconds", - "defaultValue": 500 + "defaultValue": 1000 }, { "id": "9a4bfe01-315f-4ee7-98a9-f16b08ba12ad",