INRO: Replace network device discovery with UDP based discovery mechanism

Signed-off-by: Simon Stürz <simon.stuerz@nymea.io>

inro/integrationplugininro.cpp

@@ -31,7 +31,6 @@
 #include "integrationplugininro.h"
 #include "plugininfo.h"
 
-#include <network/networkdevicediscovery.h>
 #include <hardwaremanager.h>
 #include <hardware/electricity.h>
 
@@ -39,37 +38,29 @@
 
 IntegrationPluginInro::IntegrationPluginInro()
 {
-    m_updDiscovery = new PantaboxUdpDiscovery(this);
 
 }
 
 void IntegrationPluginInro::discoverThings(ThingDiscoveryInfo *info)
 {
-
+    PantaboxDiscovery *discovery = new PantaboxDiscovery(info);
-    if (!hardwareManager()->networkDeviceDiscovery()->available()) {
-        qCWarning(dcInro()) << "The network discovery is not available on this platform.";
-        info->finish(Thing::ThingErrorUnsupportedFeature, QT_TR_NOOP("The network device discovery is not available."));
-        return;
-    }
-
-    PantaboxDiscovery *discovery = new PantaboxDiscovery(hardwareManager()->networkDeviceDiscovery(), info);
     connect(discovery, &PantaboxDiscovery::discoveryFinished, info, [this, info, discovery](){
 
         foreach (const PantaboxDiscovery::Result &result, discovery->results()) {
-            QString title = QString("PANTABOX - %1").arg(result.serialNumber);
+            QString title = QString("PANTABOX - %1").arg(result.deviceInfo.serialNumber);
-            QString description = QString("%1 (%2)").arg(result.networkDeviceInfo.macAddress(), result.networkDeviceInfo.address().toString());
+            QString description = QString("%1 (%2)").arg(result.deviceInfo.macAddress.toString(), result.deviceInfo.ipAddress.toString());
             ThingDescriptor descriptor(pantaboxThingClassId, title, description);
 
             // Check if we already have set up this device
-            Things existingThings = myThings().filterByParam(pantaboxThingMacAddressParamTypeId, result.networkDeviceInfo.macAddress());
+            Things existingThings = myThings().filterByParam(pantaboxThingSerialNumberParamTypeId, result.deviceInfo.serialNumber);
             if (existingThings.count() == 1) {
-                qCDebug(dcInro()) << "This PANTABOX already exists in the system:" << result.networkDeviceInfo;
+                qCDebug(dcInro()) << "This PANTABOX already exists in the system:" << result.deviceInfo.serialNumber << result.deviceInfo.ipAddress.toString();
                 descriptor.setThingId(existingThings.first()->id());
             }
 
             ParamList params;
-            params << Param(pantaboxThingMacAddressParamTypeId, result.networkDeviceInfo.macAddress());
+            params << Param(pantaboxThingMacAddressParamTypeId, result.deviceInfo.macAddress.toString());
-            params << Param(pantaboxThingSerialNumberParamTypeId, result.serialNumber);
+            params << Param(pantaboxThingSerialNumberParamTypeId, result.deviceInfo.serialNumber);
             descriptor.setParams(params);
             info->addThingDescriptor(descriptor);
         }
@@ -87,13 +78,11 @@ void IntegrationPluginInro::setupThing(ThingSetupInfo *info)
 
     if (m_connections.contains(thing)) {
         qCDebug(dcInro()) << "Reconfiguring existing thing" << thing->name();
-        m_connections.take(thing)->deleteLater();
+        Pantabox *connection = m_connections.take(thing);
-
+        connection->modbusTcpMaster()->disconnectDevice();
-        if (m_monitors.contains(thing)) {
+        connection->deleteLater();
-            hardwareManager()->networkDeviceDiscovery()->unregisterMonitor(m_monitors.take(thing));
+        thing->setStateValue(pantaboxConnectedStateTypeId, false);
     }
-    }
-
 
     QString serialNumber = thing->paramValue(pantaboxThingSerialNumberParamTypeId).toString();
 
@@ -255,34 +244,38 @@ void IntegrationPluginInro::thingRemoved(Thing *thing)
 
     m_initReadRequired.remove(thing);
 
-    // Unregister related hardware resources
-    if (m_monitors.contains(thing))
-        hardwareManager()->networkDeviceDiscovery()->unregisterMonitor(m_monitors.take(thing));
-
     if (myThings().isEmpty() && m_refreshTimer) {
         qCDebug(dcInro()) << "Stopping reconnect timer";
         hardwareManager()->pluginTimerManager()->unregisterTimer(m_refreshTimer);
         m_refreshTimer = nullptr;
     }
+
+    if (myThings().isEmpty() && m_udpDiscovery) {
+        qCDebug(dcInro()) << "Destroy UDP discovery since not needed any more";
+        m_udpDiscovery->deleteLater();
+        m_udpDiscovery = nullptr;
+    }
 }
 
 void IntegrationPluginInro::setupConnection(ThingSetupInfo *info)
 {
+    if (!m_udpDiscovery)
+        m_udpDiscovery = new PantaboxUdpDiscovery(this);
+
     Thing *thing = info->thing();
-    // NetworkDeviceMonitor *monitor = m_monitors.value(thing);
 
     Pantabox *connection = new Pantabox(QHostAddress(), 502, 1, this);
     connect(info, &ThingSetupInfo::aborted, connection, &Pantabox::deleteLater);
 
-    connect(m_updDiscovery, &PantaboxUdpDiscovery::pantaboxDiscovered, connection, [connection, thing](const PantaboxUdpDiscovery::PantaboxUdp &pantabox){
+    connect(m_udpDiscovery, &PantaboxUdpDiscovery::pantaboxDiscovered, connection, [connection, thing](const PantaboxUdpDiscovery::DeviceInfo &deviceInfo){
         QString serialNumber = thing->paramValue(pantaboxThingSerialNumberParamTypeId).toString();
-        if (pantabox.serialNumber != serialNumber)
+        if (deviceInfo.serialNumber != serialNumber)
             return;
 
-        connection->modbusTcpMaster()->setHostAddress(pantabox.ipAddress);
+        connection->modbusTcpMaster()->setHostAddress(deviceInfo.ipAddress);
 
         if (!thing->stateValue("connected").toBool()) {
-            qCDebug(dcInro()) << "Received discovery paket for" << thing << "Start connecting to the PANTABOX on" << pantabox.ipAddress.toString();
+            qCDebug(dcInro()) << "Received discovery paket for" << thing << "Start connecting to the PANTABOX on" << deviceInfo.ipAddress.toString();
             connection->connectDevice();
         }
     });

inro/integrationplugininro.h

@@ -58,10 +58,9 @@ public:
 private:
     PluginTimer *m_refreshTimer = nullptr;
     QHash<Thing *, Pantabox *> m_connections;
-    QHash<Thing *, NetworkDeviceMonitor *> m_monitors;
     QHash<Thing *, bool> m_initReadRequired;
 
-    PantaboxUdpDiscovery *m_updDiscovery = nullptr;
+    PantaboxUdpDiscovery *m_udpDiscovery = nullptr;
 
     void setupConnection(ThingSetupInfo *info);
 };

inro/pantaboxdiscovery.cpp

@@ -31,9 +31,8 @@
 #include "pantaboxdiscovery.h"
 #include "extern-plugininfo.h"
 
-PantaboxDiscovery::PantaboxDiscovery(NetworkDeviceDiscovery *networkDeviceDiscovery, QObject *parent)
+PantaboxDiscovery::PantaboxDiscovery(QObject *parent)
-    : QObject{parent},
+    : QObject{parent}
-      m_networkDeviceDiscovery{networkDeviceDiscovery}
 {
 
 }
@@ -48,21 +47,22 @@ void PantaboxDiscovery::startDiscovery()
     qCInfo(dcInro()) << "Discovery: Start searching for PANTABOX wallboxes in the network...";
     m_startDateTime = QDateTime::currentDateTime();
 
-    NetworkDeviceDiscoveryReply *discoveryReply = m_networkDeviceDiscovery->discover();
+    m_discovery = new PantaboxUdpDiscovery(this);
-    connect(discoveryReply, &NetworkDeviceDiscoveryReply::networkDeviceInfoAdded, this, &PantaboxDiscovery::checkNetworkDevice);
+    connect(m_discovery, &PantaboxUdpDiscovery::pantaboxDiscovered, this, &PantaboxDiscovery::checkNetworkDevice);
-    connect(discoveryReply, &NetworkDeviceDiscoveryReply::finished, discoveryReply, &NetworkDeviceDiscoveryReply::deleteLater);
+
-    connect(discoveryReply, &NetworkDeviceDiscoveryReply::finished, this, [=](){
+    connect(&m_discoveryTimer, &QTimer::timeout, this, &PantaboxDiscovery::finishDiscovery);
-        // Finish with some delay so the last added network device information objects still can be checked.
+    m_discoveryTimer.setSingleShot(true);
-        QTimer::singleShot(3000, this, [this](){
+    m_discoveryTimer.start(10000);
-            qCDebug(dcInro()) << "Discovery: Grace period timer triggered.";
-            finishDiscovery();
-        });
-    });
 }
 
-void PantaboxDiscovery::checkNetworkDevice(const NetworkDeviceInfo &networkDeviceInfo)
+void PantaboxDiscovery::checkNetworkDevice(const PantaboxUdpDiscovery::DeviceInfo &deviceInfo)
 {
-    Pantabox *connection = new Pantabox(networkDeviceInfo.address(), m_port, m_modbusAddress, this);
+    if (m_alreadyCheckedHosts.contains(deviceInfo.ipAddress))
+        return;
+
+    m_alreadyCheckedHosts.append(deviceInfo.ipAddress);
+
+    Pantabox *connection = new Pantabox(deviceInfo.ipAddress, m_port, m_modbusAddress, this);
     m_connections.append(connection);
 
     connect(connection, &Pantabox::reachableChanged, this, [=](bool reachable){
@@ -75,7 +75,7 @@ void PantaboxDiscovery::checkNetworkDevice(const NetworkDeviceInfo &networkDevic
         // Modbus TCP connected...ok, let's try to initialize it!
         connect(connection, &Pantabox::initializationFinished, this, [=](bool success){
             if (!success) {
-                qCDebug(dcInro()) << "Discovery: Initialization failed on" << networkDeviceInfo.address().toString() << "Continue...";
+                qCDebug(dcInro()) << "Discovery: Initialization failed on" << deviceInfo.ipAddress.toString() << "Continue...";
                 cleanupConnection(connection);
                 return;
             }
@@ -96,9 +96,9 @@ void PantaboxDiscovery::checkNetworkDevice(const NetworkDeviceInfo &networkDevic
                 }
 
                 connect(reply, &QModbusReply::finished, reply, &QModbusReply::deleteLater);
-                connect(reply, &QModbusReply::finished, this, [this, reply, connection, networkDeviceInfo](){
+                connect(reply, &QModbusReply::finished, this, [this, reply, connection, deviceInfo](){
                     if (reply->error() != QModbusDevice::NoError) {
-                        qCDebug(dcInro()) << "Discovery: Error reading product name error on" << networkDeviceInfo.address().toString() << "Continue...";
+                        qCDebug(dcInro()) << "Discovery: Error reading product name error on" << deviceInfo.ipAddress.toString() << "Continue...";
                         cleanupConnection(connection);
                         return;
                     }
@@ -107,10 +107,10 @@ void PantaboxDiscovery::checkNetworkDevice(const NetworkDeviceInfo &networkDevic
                     if (unit.values().size() == 4) {
                         QString receivedProductName = ModbusDataUtils::convertToString(unit.values(), connection->stringEndianness());
                         if (receivedProductName.toUpper().contains("PANTABOX")) {
-                            addResult(connection, networkDeviceInfo);
+                            addResult(connection, deviceInfo);
                         } else {
                             qCDebug(dcInro()) << "Discovery: Invalid product name " << receivedProductName
-                                              << "on" << networkDeviceInfo.address().toString() << "Continue...";
+                                              << "on" << deviceInfo.ipAddress.toString() << "Continue...";
                             cleanupConnection(connection);
                         }
                     } else {
@@ -121,13 +121,13 @@ void PantaboxDiscovery::checkNetworkDevice(const NetworkDeviceInfo &networkDevic
             } else {
                 qCDebug(dcInro()) << "Discovery: Adding connection to results even tough the result is not precise due to modbus TCP protocol version:"
                                   << connection->modbusTcpVersion() << Pantabox::modbusVersionToString(connection->modbusTcpVersion());
-                addResult(connection, networkDeviceInfo);
+                addResult(connection, deviceInfo);
             }
         });
 
         // Initializing...
         if (!connection->initialize()) {
-            qCDebug(dcInro()) << "Discovery: Unable to initialize connection on" << networkDeviceInfo.address().toString() << "Continue...";
+            qCDebug(dcInro()) << "Discovery: Unable to initialize connection on" << deviceInfo.ipAddress.toString() << "Continue...";
             cleanupConnection(connection);
         }
     });
@@ -135,14 +135,14 @@ void PantaboxDiscovery::checkNetworkDevice(const NetworkDeviceInfo &networkDevic
     // If we get any error...skip this host...
     connect(connection->modbusTcpMaster(), &ModbusTcpMaster::connectionErrorOccurred, this, [=](QModbusDevice::Error error){
         if (error != QModbusDevice::NoError) {
-            qCDebug(dcInro()) << "Discovery: Connection error on" << networkDeviceInfo.address().toString() << "Continue...";
+            qCDebug(dcInro()) << "Discovery: Connection error on" << deviceInfo.ipAddress.toString() << "Continue...";
             cleanupConnection(connection);
         }
     });
 
     // If check reachability failed...skip this host...
     connect(connection, &Pantabox::checkReachabilityFailed, this, [=](){
-        qCDebug(dcInro()) << "Discovery: Check reachability failed on" << networkDeviceInfo.address().toString() << "Continue...";
+        qCDebug(dcInro()) << "Discovery: Check reachability failed on" << deviceInfo.ipAddress.toString() << "Continue...";
         cleanupConnection(connection);
     });
 
@@ -161,30 +161,42 @@ void PantaboxDiscovery::finishDiscovery()
 {
     qint64 durationMilliSeconds = QDateTime::currentMSecsSinceEpoch() - m_startDateTime.toMSecsSinceEpoch();
 
+    m_discovery->deleteLater();
+    m_discovery = nullptr;
+
+    m_alreadyCheckedHosts.clear();
+
     // Cleanup any leftovers...we don't care any more
     foreach (Pantabox *connection, m_connections)
         cleanupConnection(connection);
 
     qCInfo(dcInro()) << "Discovery: Finished the discovery process. Found" << m_results.count()
                      << "PANTABOXE wallboxes in" << QTime::fromMSecsSinceStartOfDay(durationMilliSeconds).toString("mm:ss.zzz");
+
     emit discoveryFinished();
 }
 
-void PantaboxDiscovery::addResult(Pantabox *connection, const NetworkDeviceInfo &networkDeviceInfo)
+void PantaboxDiscovery::addResult(Pantabox *connection, const PantaboxUdpDiscovery::DeviceInfo &deviceInfo)
 {
-    qCDebug(dcInro()) << "Discovery: Connection initialized successfully" << connection->serialNumber();
+    QString modbusSerialNumber = QString::number(connection->serialNumber(), 16).toUpper();
+    if (deviceInfo.serialNumber != modbusSerialNumber) {
+        qCWarning(dcInro()) << "Discovery: Successfully discovered PANTABOX, but the UPD serial number does not match the fetched modbus serial number. Ignoring result...";
+        cleanupConnection(connection);
+        return;
+    }
+
+    qCDebug(dcInro()) << "Discovery: Connection initialized successfully" << modbusSerialNumber;
 
     Result result;
-    result.serialNumber = QString::number(connection->serialNumber(), 16).toUpper();
     result.modbusTcpVersion = Pantabox::modbusVersionToString(connection->modbusTcpVersion());
-    result.networkDeviceInfo = networkDeviceInfo;
+    result.deviceInfo = deviceInfo;
     m_results.append(result);
 
     qCInfo(dcInro()) << "Discovery: --> Found"
-                     << "Serial number:" << result.serialNumber
+                     << "Serial number:" << result.deviceInfo.serialNumber
                      << "(" << connection->serialNumber() << ")"
                      << "ModbusTCP version:" << result.modbusTcpVersion
-                     << result.networkDeviceInfo;
+                     << "on" << result.deviceInfo.ipAddress.toString() << result.deviceInfo.macAddress.toString();
 
     // Done with this connection
     cleanupConnection(connection);

inro/pantaboxdiscovery.h

@@ -31,21 +31,21 @@
 #ifndef PANTABOXDISCOVERY_H
 #define PANTABOXDISCOVERY_H
 
+#include <QTimer>
 #include <QObject>
 
-#include <network/networkdevicediscovery.h>
 #include "pantabox.h"
+#include "pantaboxudpdiscovery.h"
 
 class PantaboxDiscovery : public QObject
 {
     Q_OBJECT
 public:
-    explicit PantaboxDiscovery(NetworkDeviceDiscovery *networkDeviceDiscovery, QObject *parent = nullptr);
+    explicit PantaboxDiscovery(QObject *parent = nullptr);
 
     typedef struct Result {
-        QString serialNumber;
+        PantaboxUdpDiscovery::DeviceInfo deviceInfo;
         QString modbusTcpVersion;
-        NetworkDeviceInfo networkDeviceInfo;
     } Result;
 
     QList<PantaboxDiscovery::Result> results() const;
@@ -57,21 +57,22 @@ signals:
     void discoveryFinished();
 
 private:
-    NetworkDeviceDiscovery *m_networkDeviceDiscovery = nullptr;
+    PantaboxUdpDiscovery *m_discovery = nullptr;
     quint16 m_port = 502;
     quint16 m_modbusAddress = 1;
-
     QDateTime m_startDateTime;
+    QTimer m_discoveryTimer;
 
     QList<Pantabox *> m_connections;
+    QList<QHostAddress> m_alreadyCheckedHosts;
 
     QList<Result> m_results;
 
-    void checkNetworkDevice(const NetworkDeviceInfo &networkDeviceInfo);
+    void checkNetworkDevice(const PantaboxUdpDiscovery::DeviceInfo &deviceInfo);
     void cleanupConnection(Pantabox *connection);
 
     void finishDiscovery();
-    void addResult(Pantabox *connection, const NetworkDeviceInfo &networkDeviceInfo);
+    void addResult(Pantabox *connection, const PantaboxUdpDiscovery::DeviceInfo &deviceInfo);
 };
 
 #endif // PANTABOXDISCOVERY_H

inro/pantaboxudpdiscovery.cpp

@@ -48,12 +48,15 @@ PantaboxUdpDiscovery::PantaboxUdpDiscovery(QObject *parent)
     }
 
     connect(m_socket, &QUdpSocket::readyRead, this, &PantaboxUdpDiscovery::readPendingDatagrams);
-
     m_available = true;
-
 }
 
-QHash<QString, PantaboxUdpDiscovery::PantaboxUdp> PantaboxUdpDiscovery::results() const
+bool PantaboxUdpDiscovery::available() const
+{
+    return m_available;
+}
+
+QHash<QString, PantaboxUdpDiscovery::DeviceInfo> PantaboxUdpDiscovery::results() const
 {
     return m_results;
 }
@@ -129,6 +132,7 @@ void PantaboxUdpDiscovery::processDataBuffer(const QHostAddress &address)
     }
 
     //qCDebug(dcInro()) << "UdpDiscovery:" << qUtf8Printable(jsonDoc.toJson(QJsonDocument::Indented));
+
     /*
          {
             "deviceId": "e45749d4-8c05-44b2-9dbc-xxxxxxxxxxxx",
@@ -146,7 +150,7 @@ void PantaboxUdpDiscovery::processDataBuffer(const QHostAddress &address)
 
     QVariantMap dataMap = jsonDoc.toVariant().toMap();
     if (dataMap.contains("serialNumber") && dataMap.contains("ipAddress") && dataMap.contains("macAddress")) {
-        PantaboxUdp pantabox;
+        DeviceInfo pantabox;
         pantabox.serialNumber = dataMap.value("serialNumber").toString().remove("#");
         pantabox.macAddress = MacAddress(dataMap.value("macAddress").toString());
         pantabox.ipAddress = QHostAddress(dataMap.value("ipAddress").toString());

inro/pantaboxudpdiscovery.h

@@ -42,18 +42,18 @@ class PantaboxUdpDiscovery : public QObject
 public:
     explicit PantaboxUdpDiscovery(QObject *parent = nullptr);
 
-    typedef struct PantaboxUdp {
+    typedef struct DeviceInfo {
         QString serialNumber;
         MacAddress macAddress;
         QHostAddress ipAddress;
-    } PantaboxUdp;
+    } DeviceInfo;
 
     bool available() const;
 
-    QHash<QString, PantaboxUdp> results() const;
+    QHash<QString, DeviceInfo> results() const;
 
 signals:
-    void pantaboxDiscovered(const PantaboxUdp &pantabox);
+    void pantaboxDiscovered(const PantaboxUdpDiscovery::DeviceInfo &deviceInfo);
 
 private slots:
     void readPendingDatagrams();
@@ -68,7 +68,7 @@ private:
     quint8 calculateCrc8(const QByteArray &data);
     void processDataBuffer(const QHostAddress &address);
 
-    QHash<QString, PantaboxUdp> m_results;
+    QHash<QString, DeviceInfo> m_results;
 };
 
 #endif // PANTABOXUDPDISCOVERY_H