Network device discovery: create reply for each discovery request

libnymea-core/hardware/network/networkdevicediscoveryimpl.cpp

@@ -98,35 +98,68 @@ NetworkDeviceDiscoveryImpl::~NetworkDeviceDiscoveryImpl()
 
 NetworkDeviceDiscoveryReply *NetworkDeviceDiscoveryImpl::discover()
 {
-    if (m_currentReply) {
-        qCDebug(dcNetworkDeviceDiscovery()) << "Discovery already running. Returning current pending discovery reply...";
-        return m_currentReply;
+    // Each user calling this method receives it's own reply object.
+    // For internal tracking we use the current reply, but each caller gets it's own
+    // reply and owns the object, even if the discovery has been finished.
+
+    // Create the internal object if required
+    bool alreadyRunning = (m_currentReply != nullptr);
+    if (alreadyRunning) {
+        qCDebug(dcNetworkDeviceDiscovery()) << "Discovery internally already running. Re-using the current running discovery reply.";
+    } else {
+        qCDebug(dcNetworkDeviceDiscovery()) << "Starting internally a new discovery.";
+        m_currentReply = new NetworkDeviceDiscoveryReplyImpl(this);
+        connect(m_currentReply, &NetworkDeviceDiscoveryReplyImpl::networkDeviceInfoAdded, this, &NetworkDeviceDiscoveryImpl::updateCache);
     }
 
-    m_currentReply = new NetworkDeviceDiscoveryReplyImpl(this);
+    // Create the reply for the user
+    NetworkDeviceDiscoveryReplyImpl *reply = new NetworkDeviceDiscoveryReplyImpl(this);
+    connect(m_currentReply, &NetworkDeviceDiscoveryReplyImpl::networkDeviceInfoAdded, reply, &NetworkDeviceDiscoveryReplyImpl::addCompleteNetworkDeviceInfo);
+    connect(m_currentReply, &NetworkDeviceDiscoveryReplyImpl::hostAddressDiscovered, reply, &NetworkDeviceDiscoveryReplyImpl::hostAddressDiscovered);
+    connect(m_currentReply, &NetworkDeviceDiscoveryReplyImpl::finished, reply, [this, reply](){
+        // Sync all network device infos with all pending replies
+        foreach (const NetworkDeviceInfo &info, m_currentReply->networkDeviceInfos()) {
+            reply->addCompleteNetworkDeviceInfo(info);
+        }
+
+        foreach (const NetworkDeviceInfo &info, m_currentReply->virtualNetworkDeviceInfos()) {
+            reply->addVirtualNetworkDeviceInfo(info);
+        }
+
+        reply->setFinished(true);
+        emit reply->finished();
+    });
 
     if (!available()) {
         qCWarning(dcNetworkDeviceDiscovery()) << "The network discovery is not available. Please make sure the binary has the required capability (CAP_NET_RAW) or start the application as root.";
-        // Finish the discovery in the next event loop so anny connections after the creation will work as expected
+        // Finish the discovery in the next event loop so any connections after the creation will work as expected
         QTimer::singleShot(0, this, &NetworkDeviceDiscoveryImpl::finishDiscovery);
-        return m_currentReply;
+        return reply;
     }
 
-    connect(m_currentReply, &NetworkDeviceDiscoveryReplyImpl::networkDeviceInfoAdded, this, &NetworkDeviceDiscoveryImpl::updateCache);
-    qCInfo(dcNetworkDeviceDiscovery()) << "Starting network device discovery ...";
+    if (alreadyRunning) {
+        // Add already discovered network device infos in the next event loop
+        // so any connections after this method call will work as expected
+        QTimer::singleShot(0, reply, [this, reply](){
+            foreach (const NetworkDeviceInfo &networkDeviceInfo, m_currentReply->networkDeviceInfos()) {
+                reply->addCompleteNetworkDeviceInfo(networkDeviceInfo);
+            }
+        });
+    } else {
+        qCInfo(dcNetworkDeviceDiscovery()) << "Starting network device discovery ...";
 
-    if (m_ping->available())
-        pingAllNetworkDevices();
+        if (m_ping->available())
+            pingAllNetworkDevices();
 
-    if (m_arpSocket->isOpen())
-        m_arpSocket->sendRequest();
+        if (m_arpSocket->isOpen())
+            m_arpSocket->sendRequest();
 
-    m_discoveryTimer->start();
+        m_discoveryTimer->start();
 
-    m_running = true;
-    emit runningChanged(m_running);
-
-    return m_currentReply;
+        m_running = true;
+        emit runningChanged(m_running);
+    }
+    return reply;
 }
 
 bool NetworkDeviceDiscoveryImpl::available() const
@@ -185,7 +218,8 @@ NetworkDeviceMonitor *NetworkDeviceDiscoveryImpl::registerMonitor(const MacAddre
 
     if (!monitor->networkDeviceInfo().isValid()) {
         qCDebug(dcNetworkDeviceDiscovery()) << "Adding network device monitor for unresolved mac address. Starting a discovery...";
-        discover();
+        NetworkDeviceDiscoveryReply *reply = discover();
+        connect(reply, &NetworkDeviceDiscoveryReply::finished, reply, &NetworkDeviceDiscoveryReply::deleteLater);
     }
 
     evaluateMonitor(monitor);
@@ -628,7 +662,8 @@ void NetworkDeviceDiscoveryImpl::evaluateMonitors()
 
     if (monitorRequiresRediscovery && longerAgoThan(m_lastDiscovery, m_rediscoveryInterval)) {
         qCDebug(dcNetworkDeviceDiscovery()) << "There are unreachable monitors and the last discovery is more than" << m_rediscoveryInterval << "s ago. Starting network discovery to search the monitored network devices...";
-        discover();
+        NetworkDeviceDiscoveryReply *reply = discover();
+        connect(reply, &NetworkDeviceDiscoveryReply::finished, reply, &NetworkDeviceDiscoveryReply::deleteLater);
     }
 
     // Do some cache housekeeping if required
@@ -659,6 +694,7 @@ void NetworkDeviceDiscoveryImpl::finishDiscovery()
 
     m_lastDiscovery = QDateTime::currentDateTime();
 
+    // Clean up internal reply
     m_currentReply->processDiscoveryFinished();
     m_currentReply->deleteLater();
     m_currentReply = nullptr;

libnymea-core/hardware/network/networkdevicediscoveryreplyimpl.cpp

@@ -53,39 +53,14 @@ NetworkDeviceInfos NetworkDeviceDiscoveryReplyImpl::virtualNetworkDeviceInfos()
     return m_virtualNetworkDeviceInfos;
 }
 
-QString NetworkDeviceDiscoveryReplyImpl::macAddressFromHostAddress(const QHostAddress &address)
+bool NetworkDeviceDiscoveryReplyImpl::isFinished() const
 {
-    foreach (const NetworkDeviceInfo &info, m_networkDeviceCache) {
-        if (info.address() == address) {
-            return info.macAddress();
-        }
-    }
-
-    return QString();
+    return m_isFinished;
 }
 
-bool NetworkDeviceDiscoveryReplyImpl::hasHostAddress(const QHostAddress &address)
+void NetworkDeviceDiscoveryReplyImpl::setFinished(bool finished)
 {
-    return ! macAddressFromHostAddress(address).isEmpty();
-}
-
-void NetworkDeviceDiscoveryReplyImpl::verifyComplete(const MacAddress &macAddress)
-{
-    if (!m_networkDeviceCache.contains(macAddress))
-        return;
-
-    if (m_networkDeviceCache[macAddress].isComplete() && m_networkDeviceCache[macAddress].isValid()) {
-        if (m_networkDeviceInfos.hasMacAddress(macAddress)) {
-            if (m_networkDeviceInfos.get(macAddress) != m_networkDeviceCache.value(macAddress)) {
-                qCWarning(dcNetworkDeviceDiscovery()) << "Already complete network device info changed during discovery process! Please report a bug if you see this message.";
-                qCWarning(dcNetworkDeviceDiscovery()) << m_networkDeviceInfos.get(macAddress);
-                qCWarning(dcNetworkDeviceDiscovery()) << m_networkDeviceCache.value(macAddress);
-            }
-        } else {
-            m_networkDeviceInfos.append(m_networkDeviceCache.value(macAddress));
-            emit networkDeviceInfoAdded(m_networkDeviceCache[macAddress]);
-        }
-    }
+    m_isFinished = finished;
 }
 
 void NetworkDeviceDiscoveryReplyImpl::processPingResponse(const QHostAddress &address, const QString &hostName)
@@ -217,7 +192,63 @@ void NetworkDeviceDiscoveryReplyImpl::processDiscoveryFinished()
         qCDebug(dcNetworkDeviceDiscovery()) << "--> " << info << "Valid:" << info.isValid() << "Complete:" << info.isComplete() << info.incompleteProperties();
     }
 
+    m_isFinished = true;
     emit finished();
 }
 
+void NetworkDeviceDiscoveryReplyImpl::addCompleteNetworkDeviceInfo(const NetworkDeviceInfo &networkDeviceInfo)
+{
+    // Note: this method will be called only from the discovery
+    if (!m_networkDeviceInfos.hasMacAddress(networkDeviceInfo.macAddress())) {
+        m_networkDeviceInfos.append(networkDeviceInfo);
+
+        emit hostAddressDiscovered(networkDeviceInfo.address());
+
+        emit networkDeviceInfoAdded(networkDeviceInfo);
+    }
+}
+
+void NetworkDeviceDiscoveryReplyImpl::addVirtualNetworkDeviceInfo(const NetworkDeviceInfo &networkDeviceInfo)
+{
+    // Note: this method will be called only from the discovery
+    if (!m_networkDeviceInfos.hasHostAddress(networkDeviceInfo.address())) {
+        m_virtualNetworkDeviceInfos.append(networkDeviceInfo);
+    }
+}
+
+QString NetworkDeviceDiscoveryReplyImpl::macAddressFromHostAddress(const QHostAddress &address)
+{
+    foreach (const NetworkDeviceInfo &info, m_networkDeviceCache) {
+        if (info.address() == address) {
+            return info.macAddress();
+        }
+    }
+
+    return QString();
+}
+
+bool NetworkDeviceDiscoveryReplyImpl::hasHostAddress(const QHostAddress &address)
+{
+    return ! macAddressFromHostAddress(address).isEmpty();
+}
+
+void NetworkDeviceDiscoveryReplyImpl::verifyComplete(const MacAddress &macAddress)
+{
+    if (!m_networkDeviceCache.contains(macAddress))
+        return;
+
+    if (m_networkDeviceCache[macAddress].isComplete() && m_networkDeviceCache[macAddress].isValid()) {
+        if (m_networkDeviceInfos.hasMacAddress(macAddress)) {
+            if (m_networkDeviceInfos.get(macAddress) != m_networkDeviceCache.value(macAddress)) {
+                qCWarning(dcNetworkDeviceDiscovery()) << "Already complete network device info changed during discovery process! Please report a bug if you see this message.";
+                qCWarning(dcNetworkDeviceDiscovery()) << m_networkDeviceInfos.get(macAddress);
+                qCWarning(dcNetworkDeviceDiscovery()) << m_networkDeviceCache.value(macAddress);
+            }
+        } else {
+            m_networkDeviceInfos.append(m_networkDeviceCache.value(macAddress));
+            emit networkDeviceInfoAdded(m_networkDeviceCache[macAddress]);
+        }
+    }
+}
+
 }

libnymea-core/hardware/network/networkdevicediscoveryreplyimpl.h

@@ -43,8 +43,6 @@ class NetworkDeviceDiscoveryReplyImpl : public NetworkDeviceDiscoveryReply
 {
     Q_OBJECT
 
-    friend class NetworkDeviceDiscoveryImpl;
-
 public:
     explicit NetworkDeviceDiscoveryReplyImpl(QObject *parent = nullptr);
     ~NetworkDeviceDiscoveryReplyImpl() override = default;
@@ -52,6 +50,20 @@ public:
     NetworkDeviceInfos networkDeviceInfos() const override;
     NetworkDeviceInfos virtualNetworkDeviceInfos() const override;
 
+    bool isFinished() const override;
+    void setFinished(bool finished);
+
+    // Add or update the network device info and verify if completed
+    void processPingResponse(const QHostAddress &address, const QString &hostName);
+    void processArpResponse(const QNetworkInterface &interface, const QHostAddress &address, const MacAddress &macAddress);
+    void processMacManufacturer(const MacAddress &macAddress, const QString &manufacturer);
+
+    void processDiscoveryFinished();
+
+public slots:
+    void addCompleteNetworkDeviceInfo(const NetworkDeviceInfo &networkDeviceInfo);
+    void addVirtualNetworkDeviceInfo(const NetworkDeviceInfo &networkDeviceInfo);
+
 private:
     NetworkDeviceInfos m_networkDeviceInfos; // Contains only complete and valid infos
     NetworkDeviceInfos m_virtualNetworkDeviceInfos; // Contains ping responses without ARP, like VPN devices
@@ -59,6 +71,8 @@ private:
     QHash<MacAddress, NetworkDeviceInfo> m_networkDeviceCache;
     qint64 m_startTimestamp;
 
+    bool m_isFinished = false;
+
     // Temporary cache for ping responses where the mac is not known yet (like VPN devices)
     QHash<QHostAddress, NetworkDeviceInfo> m_pingCache;
 
@@ -67,12 +81,7 @@ private:
 
     void verifyComplete(const MacAddress &macAddress);
 
-    // Add or update the network device info and verify if completed
-    void processPingResponse(const QHostAddress &address, const QString &hostName);
-    void processArpResponse(const QNetworkInterface &interface, const QHostAddress &address, const MacAddress &macAddress);
-    void processMacManufacturer(const MacAddress &macAddress, const QString &manufacturer);
 
-    void processDiscoveryFinished();
 };
 
 }

libnymea/network/networkdevicediscoveryreply.h

@@ -46,6 +46,8 @@ public:
     virtual NetworkDeviceInfos networkDeviceInfos() const = 0;
     virtual NetworkDeviceInfos virtualNetworkDeviceInfos() const = 0;
 
+    virtual bool isFinished() const = 0;
+
 signals:
     // Emitted whenever a certain host address has been pinged successfully
     void hostAddressDiscovered(const QHostAddress &address);