From 329b9ac8648130743bd78ba1c6f9f2a487125113 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simon=20St=C3=BCrz?= <simon.stuerz@nymea.io>
Date: Fri, 22 Mar 2024 15:31:35 +0100
Subject: [PATCH] SunSpec: Improve discovery and handle blocking connections

---
 debian/compat                |  2 +-
 sunspec/sunspecdiscovery.cpp | 42 +++++++++++++++++++++++++++++++++---
 sunspec/sunspecdiscovery.h   |  1 +
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/debian/compat b/debian/compat
index ec63514..f599e28 100644
--- a/debian/compat
+++ b/debian/compat
@@ -1 +1 @@
-9
+10
diff --git a/sunspec/sunspecdiscovery.cpp b/sunspec/sunspecdiscovery.cpp
index dcb1e3f..de761cf 100644
--- a/sunspec/sunspecdiscovery.cpp
+++ b/sunspec/sunspecdiscovery.cpp
@@ -36,9 +36,9 @@
 
 SunSpecDiscovery::SunSpecDiscovery(NetworkDeviceDiscovery *networkDeviceDiscovery, const QList<quint16> &slaveIds, SunSpecDataPoint::ByteOrder byteOrder, QObject *parent)
     : QObject{parent},
-      m_networkDeviceDiscovery{networkDeviceDiscovery},
-      m_slaveIds{slaveIds},
-      m_byteOrder{byteOrder}
+    m_networkDeviceDiscovery{networkDeviceDiscovery},
+    m_slaveIds{slaveIds},
+    m_byteOrder{byteOrder}
 {
     m_scanPorts.append(502);
     m_scanPorts.append(1502);
@@ -94,7 +94,31 @@ void SunSpecDiscovery::testNextConnection(const QHostAddress &address)
     if (!connection->connectDevice()) {
         qCDebug(dcSunSpec()) << "Discovery: Failed to connect to" << QString("%1:%2").arg(address.toString()).arg(connection->port()) << "slave ID:" << connection->slaveId() << "Continue...";;
         cleanupConnection(connection);
+        return;
     }
+
+    /* Some connection will block the connection process and the connection will pending in the
+     * connecting state for over a minute before failing.
+     * This behavior blocks further connection tests for during the disdovery process and the discovery failes to detect the device.
+     * This timer will make sure the connection can be established within 5 seconds, otherwise we continue with the next connection.
+     */
+
+    QTimer *connectionTimer = new QTimer(connection);
+    connectionTimer->setInterval(5000);
+    connectionTimer->setSingleShot(true);
+
+    m_connectionTimers.insert(connection, connectionTimer);
+
+    connect(connectionTimer, &QTimer::timeout, connection, [this, connection, connectionTimer](){
+        qCDebug(dcSunSpec()) << "Discovery: Could not establish a connection within" << connectionTimer->interval() << "ms. Continue...";
+        m_connectionTimers.remove(connection);
+        connection->disconnectDevice();
+        // Note: since the connection is the parent of the timer, it will be deleted with the connection
+        cleanupConnection(connection);
+    });
+
+    // Once connected, the timer will be stopped.
+    connectionTimer->start();
 }
 
 void SunSpecDiscovery::checkNetworkDevice(const NetworkDeviceInfo &networkDeviceInfo)
@@ -121,6 +145,13 @@ void SunSpecDiscovery::checkNetworkDevice(const NetworkDeviceInfo &networkDevice
                     return;
                 }
 
+                // Successfully connected, we can stop the connection timer which takes care about blocking connection attempts
+                if (m_connectionTimers.contains(connection)) {
+                    QTimer *connectionTimer = m_connectionTimers.take(connection);
+                    connectionTimer->stop();
+                    connectionTimer->deleteLater();
+                }
+
                 // Modbus TCP connected, try to discovery sunspec models...
                 connect(connection, &SunSpecConnection::discoveryFinished, this, [=](bool success){
                     if (!success) {
@@ -180,6 +211,11 @@ void SunSpecDiscovery::cleanupConnection(SunSpecConnection *connection)
     connection->disconnectDevice();
     connection->deleteLater();
 
+    if (m_connectionTimers.contains(connection)) {
+        QTimer *connectionTimer = m_connectionTimers.take(connection);
+        connectionTimer->stop();
+    }
+
     testNextConnection(connection->hostAddress());
 }
 
diff --git a/sunspec/sunspecdiscovery.h b/sunspec/sunspecdiscovery.h
index f54cee6..2ea7a22 100644
--- a/sunspec/sunspecdiscovery.h
+++ b/sunspec/sunspecdiscovery.h
@@ -66,6 +66,7 @@ private:
 
     QDateTime m_startDateTime;
     QHash<QHostAddress, QQueue<SunSpecConnection *>> m_pendingConnectionAttempts;
+    QHash<SunSpecConnection *, QTimer *> m_connectionTimers;
 
     QList<SunSpecConnection *> m_connections;
     QList<Result> m_results;