Fronius: Improve reply handling on connection timout

fronius/froniusdiscovery.cpp

@@ -74,12 +74,12 @@ void FroniusDiscovery::checkHostAddress(const QHostAddress &address)
 
     FroniusNetworkReply *reply = connection->getVersion();
     connect(reply, &FroniusNetworkReply::finished, this, [=] {
-        QByteArray data = reply->networkReply()->readAll();
+        QByteArray data = reply->readAll();
-        if (reply->networkReply()->error() != QNetworkReply::NoError) {
+        if (reply->error() != QNetworkReply::NoError) {
-            if (reply->networkReply()->error() == QNetworkReply::ContentNotFoundError) {
+            if (reply->error() == QNetworkReply::ContentNotFoundError) {
                 qCInfo(dcFronius()) << "Discovery: The device on" << address.toString() << "does not reply to our requests. Please verify that the Fronius Solar API is enabled on the device.";
             } else {
-                qCDebug(dcFronius()) << "Discovery: Reply finished with error on" << address.toString() << reply->networkReply()->errorString();
+                qCDebug(dcFronius()) << "Discovery: Reply finished with error on" << address.toString() << reply->errorString();
             }
             cleanupConnection(connection);
             return;

fronius/froniusnetworkreply.cpp

@@ -28,7 +28,7 @@ FroniusNetworkReply::~FroniusNetworkReply()
 {
     if (m_networkReply) {
         // We don't need the finished signal any more, object gets deleted
-        disconnect(m_networkReply, &QNetworkReply::finished, this, &FroniusNetworkReply::finished);
+        disconnect(m_networkReply, &QNetworkReply::finished, this, &FroniusNetworkReply::handleFinished);
 
         if (m_networkReply->isRunning()) {
             // Abort the reply, we are not interested in it any more
@@ -54,6 +54,26 @@ QNetworkReply *FroniusNetworkReply::networkReply() const
     return m_networkReply;
 }
 
+QByteArray FroniusNetworkReply::readAll()
+{
+    return m_networkReply ? m_networkReply->readAll() : QByteArray();
+}
+
+QNetworkReply::NetworkError FroniusNetworkReply::error() const
+{
+    return m_networkReply ? m_networkReply->error() : m_error;
+}
+
+QString FroniusNetworkReply::errorString() const
+{
+    return m_networkReply ? m_networkReply->errorString() : m_errorString;
+}
+
+QUrl FroniusNetworkReply::url() const
+{
+    return m_networkReply ? m_networkReply->url() : m_request.url();
+}
+
 FroniusNetworkReply::FroniusNetworkReply(const QNetworkRequest &request, QObject *parent) :
     QObject(parent),
     m_request(request)
@@ -66,6 +86,39 @@ void FroniusNetworkReply::setNetworkReply(QNetworkReply *networkReply)
     m_networkReply = networkReply;
 
     // The QNetworkReply will be deleted in the destructor if set
-    connect(m_networkReply, &QNetworkReply::finished, this, &FroniusNetworkReply::finished);
+    connect(m_networkReply, &QNetworkReply::finished, this, &FroniusNetworkReply::handleFinished);
 }
 
+void FroniusNetworkReply::cancel()
+{
+    if (m_finished)
+        return;
+
+    m_canceled = true;
+
+    if (m_networkReply) {
+        m_networkReply->abort();
+        if (!m_finished)
+            handleFinished();
+        return;
+    }
+
+    m_error = QNetworkReply::OperationCanceledError;
+    m_errorString = tr("Request canceled because the device address changed.");
+    m_finished = true;
+    emit finished();
+}
+
+bool FroniusNetworkReply::wasCanceled() const
+{
+    return m_canceled;
+}
+
+void FroniusNetworkReply::handleFinished()
+{
+    if (m_finished)
+        return;
+
+    m_finished = true;
+    emit finished();
+}

fronius/froniusnetworkreply.h

@@ -41,6 +41,10 @@ public:
 
     QNetworkRequest request() const;
     QNetworkReply *networkReply() const;
+    QByteArray readAll();
+    QNetworkReply::NetworkError error() const;
+    QString errorString() const;
+    QUrl url() const;
 
 signals:
     void finished();
@@ -50,8 +54,15 @@ private:
 
     QNetworkRequest m_request;
     QNetworkReply *m_networkReply = nullptr;
+    QNetworkReply::NetworkError m_error = QNetworkReply::NoError;
+    QString m_errorString;
+    bool m_finished = false;
+    bool m_canceled = false;
 
     void setNetworkReply(QNetworkReply *networkReply);
+    void cancel();
+    bool wasCanceled() const;
+    void handleFinished();
 };
 
 #endif // FRONIUSNETWORKREPLY_H

fronius/froniussolarconnection.cpp

@@ -33,7 +33,8 @@ FroniusSolarConnection::FroniusSolarConnection(NetworkAccessManager *networkMana
     m_networkManager(networkManager),
     m_address(address)
 {
-
+    m_requestDispatchTimer.setSingleShot(true);
+    connect(&m_requestDispatchTimer, &QTimer::timeout, this, &FroniusSolarConnection::sendNextRequest);
 }
 
 QHostAddress FroniusSolarConnection::address() const
@@ -49,17 +50,14 @@ void FroniusSolarConnection::setAddress(const QHostAddress &address)
     m_address = address;
 
     // The address has changed, let's clean up any queue and refresh
+    m_requestDispatchTimer.stop();
+    cancelPendingRequests();
+    resetCustomNetworkManager();
+    m_errorCount = 0;
+    m_errorOperationCanceledCount = 0;
+    m_availabilityErrorCount = 0;
 
-    // Note: the destructor will take care about the cleanup of any pending replies
+    if (m_address.isNull() && m_available) {
-    qDeleteAll(m_requestQueue);
-    m_requestQueue.clear();
-
-    if (m_currentReply) {
-        m_currentReply->deleteLater();
-        m_currentReply = nullptr;
-    }
-
-    if (m_address.isNull()) {
         m_available = false;
         emit availableChanged(m_available);
     }
@@ -72,7 +70,7 @@ bool FroniusSolarConnection::available() const
 
 bool FroniusSolarConnection::busy() const
 {
-    return m_requestQueue.count() > 1;
+    return m_currentReply || !m_requestQueue.isEmpty();
 }
 
 FroniusNetworkReply *FroniusSolarConnection::getVersion()
@@ -106,25 +104,23 @@ FroniusNetworkReply *FroniusSolarConnection::getActiveDevices()
 
     // Note: we use this request for detecting if the logger is available or not.
     // Some other requests are only available if the device actually is loaded
-    connect(reply, &FroniusNetworkReply::finished, this, [=](){
+    connect(reply, &FroniusNetworkReply::finished, this, [this, reply](){
-        if (reply->networkReply()->error() == QNetworkReply::NoError) {
+        if (reply->wasCanceled())
+            return;
+
+        if (reply->error() == QNetworkReply::NoError) {
+            m_availabilityErrorCount = 0;
+
             // Reply was successfully, we can communicate
             if (!m_available) {
                 qCDebug(dcFronius()) << "Connection: the connection is now available";
                 m_available = true;
                 emit availableChanged(m_available);
-
-                // Destroy any pending requests
-                qDeleteAll(m_requestQueue);
-                m_requestQueue.clear();
-            }
-        } else {
-            // There have been multiple errors in a row, seems like we not available any more
-            if (m_available && m_errorCount >= m_errorCountLimit) {
-                qCDebug(dcFronius()) << "Connection: the connection is not available any more:" << reply->networkReply()->errorString();
-                m_available = false;
-                emit availableChanged(m_available);
             }
+        } else if (++m_availabilityErrorCount >= m_errorCountLimit && m_available) {
+            qCWarning(dcFronius()) << "Connection: active-device probe failed" << m_availabilityErrorCount << "times. Marking the connection unavailable:" << reply->errorString();
+            m_available = false;
+            emit availableChanged(m_available);
         }
     });
 
@@ -210,6 +206,8 @@ QNetworkRequest FroniusSolarConnection::buildRequest(const QUrl &url)
     request.setUrl(url);
     // Note: some inverter stop accepting requests, this might help
     request.setAttribute(QNetworkRequest::HttpPipeliningAllowedAttribute, false);
+    request.setTransferTimeout(requestTimeout);
+    request.setRawHeader("Connection", "close");
     return request;
 }
 
@@ -218,10 +216,14 @@ void FroniusSolarConnection::sendNextRequest()
     if (m_currentReply)
         return;
 
+    if (m_requestDispatchTimer.isActive())
+        return;
+
     if (m_requestQueue.isEmpty())
         return;
 
     m_currentReply = m_requestQueue.dequeue();
+    FroniusNetworkReply *reply = m_currentReply;
 
     if (m_useCustomNetworkManager) {
         qCDebug(dcFronius()) << "Connection: --> Sending request using custom network manager (queue: " << m_requestQueue.size() << "): " << m_currentReply->request().url().toString();
@@ -236,15 +238,21 @@ void FroniusSolarConnection::sendNextRequest()
     }
 
 
-    connect(m_currentReply, &FroniusNetworkReply::finished, this, [=](){
+    connect(reply, &FroniusNetworkReply::finished, this, [this, reply](){
+
+        // The address may have changed while this request was running.
+        if (reply != m_currentReply) {
+            reply->deleteLater();
+            return;
+        }
 
         // Note: the network reply will be deleted in the destructor
-        m_currentReply->deleteLater();
+        reply->deleteLater();
 
-        if (m_currentReply->networkReply()->error() != QNetworkReply::NoError) {
+        if (reply->error() != QNetworkReply::NoError) {
             m_errorCount++;
-            qCWarning(dcFronius()).nospace() << "Connection: <--  Request finished with error (count: " << m_errorCount << ") " << m_currentReply->networkReply()->error() << " for url " << m_currentReply->request().url().toString();
+            qCWarning(dcFronius()).nospace() << "Connection: <-- Request finished with error (count: " << m_errorCount << ") " << reply->error() << " (" << reply->errorString() << ") for url " << reply->request().url().toString();
-            if (m_currentReply->networkReply()->error() == QNetworkReply::OperationCanceledError) {
+            if (reply->error() == QNetworkReply::OperationCanceledError) {
                 m_errorOperationCanceledCount++;
                 if (!m_useCustomNetworkManager && m_errorOperationCanceledCount >= m_errorOperationCanceledCountLimit) {
                     qCWarning(dcFronius()) << "Received" << m_errorOperationCanceledCountLimit << "in a row, skipping to internal network access manager. This is a workaround in order to free all requests after each reply.";
@@ -252,7 +260,7 @@ void FroniusSolarConnection::sendNextRequest()
                 }
             }
         } else {
-            qCDebug(dcFronius()) << "Connection: <-- Request finished successfully for" << m_currentReply->request().url().toString();
+            qCDebug(dcFronius()) << "Connection: <-- Request finished successfully for" << reply->request().url().toString();
             m_errorCount = 0;
             m_errorOperationCanceledCount = 0;
         }
@@ -260,12 +268,36 @@ void FroniusSolarConnection::sendNextRequest()
         m_currentReply = nullptr;
 
         // Note: this is a workaround for some fronius devices, we recreate the networkaccessmanager after each request
-        if (m_useCustomNetworkManager && m_customNetworkManager) {
+        resetCustomNetworkManager();
-            m_customNetworkManager->deleteLater();
-            m_customNetworkManager = nullptr;
-        }
 
-        // Wait some time until we send the next request
+        // Back off after repeated failures to give the Fronius webserver time to recover.
-        QTimer::singleShot(500, this, &FroniusSolarConnection::sendNextRequest);
+        const int nextRequestDelay = m_errorCount >= m_errorCountLimit ? unavailableRetryInterval : requestInterval;
+        qCDebug(dcFronius()) << "Connection: next request in" << nextRequestDelay << "ms (queue:" << m_requestQueue.size() << ")";
+        m_requestDispatchTimer.start(nextRequestDelay);
     });
 }
+
+void FroniusSolarConnection::cancelPendingRequests()
+{
+    if (m_currentReply) {
+        FroniusNetworkReply *reply = m_currentReply;
+        m_currentReply = nullptr;
+        reply->cancel();
+        reply->deleteLater();
+    }
+
+    while (!m_requestQueue.isEmpty()) {
+        FroniusNetworkReply *reply = m_requestQueue.dequeue();
+        reply->cancel();
+        reply->deleteLater();
+    }
+}
+
+void FroniusSolarConnection::resetCustomNetworkManager()
+{
+    if (!m_customNetworkManager)
+        return;
+
+    m_customNetworkManager->deleteLater();
+    m_customNetworkManager = nullptr;
+}

fronius/froniussolarconnection.h

@@ -29,6 +29,7 @@
 #include <QQueue>
 #include <QHostAddress>
 #include <QNetworkAccessManager>
+#include <QTimer>
 
 #include <network/networkaccessmanager.h>
 
@@ -64,6 +65,10 @@ private:
 
     bool m_available = false;
 
+    static constexpr int requestInterval = 4000;
+    static constexpr int requestTimeout = 15000;
+    static constexpr int unavailableRetryInterval = 60000;
+
     // Fallback solution for dead nam requests, this happens on some platforms
     // Note: we enable for now the custom network access manager
     // Some fronius inverters keep the connection alive and get stuck somehow.
@@ -78,13 +83,17 @@ private:
     uint m_errorOperationCanceledCountLimit = 3;
     uint m_errorCount = 0;
     uint m_errorCountLimit = 5;
+    uint m_availabilityErrorCount = 0;
 
     // Request queue to prevent overloading the device with requests
     FroniusNetworkReply *m_currentReply = nullptr;
     QQueue<FroniusNetworkReply *> m_requestQueue;
+    QTimer m_requestDispatchTimer;
 
     QNetworkRequest buildRequest(const QUrl &url);
 
+    void cancelPendingRequests();
+    void resetCustomNetworkManager();
     void sendNextRequest();
 
 };

fronius/integrationpluginfronius.cpp

@@ -157,10 +157,10 @@ void IntegrationPluginFronius::setupThing(ThingSetupInfo *info)
             // Verify the version
             FroniusNetworkReply *reply = connection->getVersion();
             connect(reply, &FroniusNetworkReply::finished, info, [=] {
-                QByteArray data = reply->networkReply()->readAll();
+                QByteArray data = reply->readAll();
-                if (reply->networkReply()->error() != QNetworkReply::NoError) {
+                if (reply->error() != QNetworkReply::NoError) {
-                    qCWarning(dcFronius()) << "Network request error:" << reply->networkReply()->error() << reply->networkReply()->errorString() << reply->networkReply()->url();
+                    qCWarning(dcFronius()) << "Network request error:" << reply->error() << reply->errorString() << reply->url();
-                    if (reply->networkReply()->error() == QNetworkReply::ContentNotFoundError) {
+                    if (reply->error() == QNetworkReply::ContentNotFoundError) {
                         info->finish(Thing::ThingErrorHardwareNotAvailable, QT_TR_NOOP("The device does not reply to our requests. Please verify that the Fronius Solar API is enabled on the device."));
                     } else {
                         info->finish(Thing::ThingErrorHardwareNotAvailable, QT_TR_NOOP("The device is not reachable."));
@@ -292,7 +292,7 @@ void IntegrationPluginFronius::refreshConnection(FroniusSolarConnection *connect
     // Note: this call will be used to monitor the available state of the connection internally
     FroniusNetworkReply *reply = connection->getActiveDevices();
     connect(reply, &FroniusNetworkReply::finished, this, [=]() {
-        if (reply->networkReply()->error() != QNetworkReply::NoError) {
+        if (reply->error() != QNetworkReply::NoError) {
             // Note: the connection warns about any errors if available changed
             return;
         }
@@ -301,7 +301,7 @@ void IntegrationPluginFronius::refreshConnection(FroniusSolarConnection *connect
         if (!connectionThing)
             return;
 
-        QByteArray data = reply->networkReply()->readAll();
+        QByteArray data = reply->readAll();
 
         QJsonParseError error;
         QJsonDocument jsonDoc = QJsonDocument::fromJson(data, &error);
@@ -342,10 +342,10 @@ void IntegrationPluginFronius::refreshConnection(FroniusSolarConnection *connect
                 // Get the meter realtime data for details
                 FroniusNetworkReply *realtimeDataReply = connection->getMeterRealtimeData(meterId.toInt());
                 connect(realtimeDataReply, &FroniusNetworkReply::finished, this, [=]() {
-                    if (realtimeDataReply->networkReply()->error() != QNetworkReply::NoError)
+                    if (realtimeDataReply->error() != QNetworkReply::NoError)
                         return;
 
-                    QByteArray data = realtimeDataReply->networkReply()->readAll();
+                    QByteArray data = realtimeDataReply->readAll();
 
                     QJsonParseError error;
                     QJsonDocument jsonDoc = QJsonDocument::fromJson(data, &error);
@@ -397,10 +397,10 @@ void IntegrationPluginFronius::refreshConnection(FroniusSolarConnection *connect
                 // Get the meter realtime data for details
                 FroniusNetworkReply *realtimeDataReply = connection->getStorageRealtimeData(storageId.toInt());
                 connect(realtimeDataReply, &FroniusNetworkReply::finished, this, [=]() {
-                    if (realtimeDataReply->networkReply()->error() != QNetworkReply::NoError)
+                    if (realtimeDataReply->error() != QNetworkReply::NoError)
                         return;
 
-                    QByteArray data = realtimeDataReply->networkReply()->readAll();
+                    QByteArray data = realtimeDataReply->readAll();
 
                     QJsonParseError error;
                     QJsonDocument jsonDoc = QJsonDocument::fromJson(data, &error);
@@ -470,10 +470,10 @@ void IntegrationPluginFronius::updatePowerFlow(FroniusSolarConnection *connectio
     // to make sure the sum is correct. Battery seems to be feeded DC to DC before the AC power convertion
     FroniusNetworkReply *powerFlowReply = connection->getPowerFlowRealtimeData();
     connect(powerFlowReply, &FroniusNetworkReply::finished, this, [=]() {
-        if (powerFlowReply->networkReply()->error() != QNetworkReply::NoError)
+        if (powerFlowReply->error() != QNetworkReply::NoError)
             return;
 
-        QByteArray data = powerFlowReply->networkReply()->readAll();
+        QByteArray data = powerFlowReply->readAll();
 
         QJsonParseError error;
         QJsonDocument jsonDoc = QJsonDocument::fromJson(data, &error);
@@ -541,7 +541,7 @@ void IntegrationPluginFronius::updateInverters(FroniusSolarConnection *connectio
         // Get the inverter realtime data
         FroniusNetworkReply *realtimeDataReply = connection->getInverterRealtimeData(inverterId);
         connect(realtimeDataReply, &FroniusNetworkReply::finished, this, [=]() {
-            if (realtimeDataReply->networkReply()->error() != QNetworkReply::NoError) {
+            if (realtimeDataReply->error() != QNetworkReply::NoError) {
                 m_thingRequestErrorCounter[inverterThing] = m_thingRequestErrorCounter.value(inverterThing, 0) + 1;
                 if (m_thingRequestErrorCounter.value(inverterThing) >= m_thingRequestErrorCountLimit) {
                     if (inverterThing->stateValue("connected").toBool()) {
@@ -556,7 +556,7 @@ void IntegrationPluginFronius::updateInverters(FroniusSolarConnection *connectio
             // Reset the error counter on a successfull refresh
             m_thingRequestErrorCounter[inverterThing] = 0;
 
-            QByteArray data = realtimeDataReply->networkReply()->readAll();
+            QByteArray data = realtimeDataReply->readAll();
 
             QJsonParseError error;
             QJsonDocument jsonDoc = QJsonDocument::fromJson(data, &error);
@@ -614,7 +614,7 @@ void IntegrationPluginFronius::updateMeters(FroniusSolarConnection *connection)
         // Get the inverter realtime data
         FroniusNetworkReply *realtimeDataReply = connection->getMeterRealtimeData(meterId);
         connect(realtimeDataReply, &FroniusNetworkReply::finished, this, [=]() {
-            if (realtimeDataReply->networkReply()->error() != QNetworkReply::NoError) {
+            if (realtimeDataReply->error() != QNetworkReply::NoError) {
                 m_thingRequestErrorCounter[meterThing] = m_thingRequestErrorCounter.value(meterThing, 0) + 1;
                 if (m_thingRequestErrorCounter.value(meterThing) >= m_thingRequestErrorCountLimit) {
                     if (meterThing->stateValue("connected").toBool()) {
@@ -629,7 +629,7 @@ void IntegrationPluginFronius::updateMeters(FroniusSolarConnection *connection)
             // Reset the error counter on a successfull refresh
             m_thingRequestErrorCounter[meterThing] = 0;
 
-            QByteArray data = realtimeDataReply->networkReply()->readAll();
+            QByteArray data = realtimeDataReply->readAll();
 
             QJsonParseError error;
             QJsonDocument jsonDoc = QJsonDocument::fromJson(data, &error);
@@ -731,7 +731,7 @@ void IntegrationPluginFronius::updateStorages(FroniusSolarConnection *connection
         // Get the storage realtime data
         FroniusNetworkReply *realtimeDataReply = connection->getStorageRealtimeData(storageId);
         connect(realtimeDataReply, &FroniusNetworkReply::finished, this, [=]() {
-            if (realtimeDataReply->networkReply()->error() != QNetworkReply::NoError) {
+            if (realtimeDataReply->error() != QNetworkReply::NoError) {
                 m_thingRequestErrorCounter[storageThing] = m_thingRequestErrorCounter.value(storageThing, 0) + 1;
                 if (m_thingRequestErrorCounter.value(storageThing) >= m_thingRequestErrorCountLimit) {
                     if (storageThing->stateValue("connected").toBool()) {
@@ -746,13 +746,13 @@ void IntegrationPluginFronius::updateStorages(FroniusSolarConnection *connection
             // Reset the error counter on a successfull refresh
             m_thingRequestErrorCounter[storageThing] = 0;
 
-            if (realtimeDataReply->networkReply()->error() != QNetworkReply::NoError) {
+            if (realtimeDataReply->error() != QNetworkReply::NoError) {
                 // Thing does not seem to be reachable
                 markStorageAsDisconnected(storageThing);
                 return;
             }
 
-            QByteArray data = realtimeDataReply->networkReply()->readAll();
+            QByteArray data = realtimeDataReply->readAll();
 
             QJsonParseError error;
             QJsonDocument jsonDoc = QJsonDocument::fromJson(data, &error);
@@ -821,4 +821,3 @@ void IntegrationPluginFronius::markStorageAsDisconnected(Thing *thing)
     thing->setStateValue("chargingState", "idle");
     // Note: do not reset the energy counters since they are always counting up until reset on the device
 }
-