Speed up ping, inform about unfinished ping replies and abort on timeout

libnymea-core/hardware/network/networkdevicediscoveryimpl.cpp

@@ -64,7 +64,20 @@ NetworkDeviceDiscoveryImpl::NetworkDeviceDiscoveryImpl(QObject *parent) :
     m_discoveryTimer->setInterval(20000);
     m_discoveryTimer->setSingleShot(true);
     connect(m_discoveryTimer, &QTimer::timeout, this, [=](){
-        if (m_runningPingReplies.isEmpty() && m_runningMacDatabaseReplies.isEmpty() && m_currentDiscoveryReply) {
+        if (!m_runningPingReplies.isEmpty()) {
+            qCDebug(dcNetworkDeviceDiscovery()) << "Discovery timeout occurred. There are still" << m_runningPingReplies.count()  << "ping replies pending and" << m_ping->queueCount() << "addresses int the ping queue. Aborting them...";
+            foreach (PingReply *reply, m_runningPingReplies) {
+                reply->abort();
+            }
+        }
+
+        // We still wait for any mac manufacturer lookups, since we got already a mac...
+        if (!m_runningMacDatabaseReplies.isEmpty()) {
+            qCDebug(dcNetworkDeviceDiscovery()) << "Discovery timeout occurred but there are still" << m_runningMacDatabaseReplies.count() << "mac database replies pending. Waiting for them to finish...";
+            return;
+        }
+
+        if (m_currentDiscoveryReply) {
             qCDebug(dcNetworkDeviceDiscovery()) << "Discovery timeout occurred and all pending replies have finished.";
             finishDiscovery();
         }
@@ -378,6 +391,11 @@ void NetworkDeviceDiscoveryImpl::pingAllNetworkDevices()
                         }
                     }
 
+                    if (m_runningPingReplies.isEmpty() && !m_runningMacDatabaseReplies.isEmpty()) {
+                        qCDebug(dcNetworkDeviceDiscovery()) << "All ping replies have finished but there are still" << m_runningMacDatabaseReplies.count() << "mac db lookups pending. Waiting for them to finish...";
+                        return;
+                    }
+
                     if (m_runningPingReplies.isEmpty() && m_runningMacDatabaseReplies.isEmpty() && m_currentDiscoveryReply && !m_discoveryTimer->isActive()) {
                         qCDebug(dcNetworkDeviceDiscovery()) << "All pending replies have finished.";
                         finishDiscovery();

libnymea/network/ping.cpp

@@ -50,7 +50,7 @@ NYMEA_LOGGING_CATEGORY(dcPingTraffic, "PingTraffic")
 Ping::Ping(QObject *parent) : QObject(parent)
 {
     m_queueTimer = new QTimer(this);
-    m_queueTimer->setInterval(20);
+    m_queueTimer->setInterval(10);
     m_queueTimer->setSingleShot(true);
     connect(m_queueTimer, &QTimer::timeout, this, [=](){
         sendNextReply();
@@ -111,6 +111,11 @@ PingReply::Error Ping::error() const
     return m_error;
 }
 
+int Ping::queueCount() const
+{
+    return m_replyQueue.count();
+}
+
 PingReply *Ping::ping(const QHostAddress &hostAddress, uint retries)
 {
     PingReply *reply = createReply(hostAddress);
@@ -144,10 +149,15 @@ void Ping::sendNextReply()
     if (m_replyQueue.isEmpty())
         return;
 
-    PingReply *reply = m_replyQueue.dequeue();
+    m_currentReply = m_replyQueue.dequeue();
-    qCDebug(dcPing()) << "Send next reply," << m_replyQueue.count() << "left in queue";
+    qCDebug(dcPing()) << "Send next reply " << m_currentReply->targetHostAddress().toString() << QString("0x%1").arg(m_currentReply->requestId(), 4, 16, QChar('0')) << ", " << m_replyQueue.count() << "left in queue";
     m_queueTimer->start();
-    QTimer::singleShot(0, reply, [=]() { performPing(reply); });
+    QTimer::singleShot(0, this, [=]() {
+        if (!m_currentReply)
+            return;
+
+        performPing(m_currentReply);
+    });
 }
 
 void Ping::performPing(PingReply *reply)
@@ -298,7 +308,7 @@ PingReply *Ping::createReply(const QHostAddress &hostAddress)
     reply->m_networkInterface = NetworkUtils::getInterfaceForHostaddress(hostAddress);
 
     connect(reply, &PingReply::timeout, this, [=](){
-        // Note: this is not the ICMP timeout, here we actually got nothing from nobody...
+        // Note: this is not the ICMP timeout, here we actually got nothing from anybody...
         finishReply(reply, PingReply::ErrorTimeout);
     });
 
@@ -310,8 +320,18 @@ PingReply *Ping::createReply(const QHostAddress &hostAddress)
         reply->deleteLater();
 
         // Cleanup any retry left over queue stuff
+        if (m_currentReply == reply)
+            m_currentReply = nullptr;
+
         m_pendingReplies.remove(reply->requestId());
         m_replyQueue.removeAll(reply);
+
+        if (m_pendingHostLookups.values().contains(reply)) {
+            // Abort any pending host lookups, the reply has been finished
+            int lookupId = m_pendingHostLookups.key(reply);
+            QHostInfo::abortHostLookup(lookupId);
+            m_pendingHostLookups.remove(lookupId);
+        }
     });
 
     return reply;
@@ -391,7 +411,7 @@ void Ping::onSocketReadyRead(int socketDescriptor)
 
         if (responsePacket->icmp_type == ICMP_ECHOREPLY) {
 
-            PingReply *reply = m_pendingReplies.take(icmpId);
+            PingReply *reply = m_pendingReplies.value(icmpId);
             if (!reply) {
                 qCDebug(dcPing()) << "No pending reply for ping echo response with id" << QString("0x%1").arg(icmpId, 4, 16, QChar('0')) << "Sequence:" << icmpSequnceNumber << "from" << senderAddress.toString();
                 return;
@@ -426,12 +446,12 @@ void Ping::onSocketReadyRead(int socketDescriptor)
                 // Note: due to a Qt bug < 5.9 we need to use old SLOT style and cannot make use of lambda here
                 int lookupId = QHostInfo::lookupHost(senderAddress.toString(), this, SLOT(onHostLookupFinished(QHostInfo)));
                 m_pendingHostLookups.insert(lookupId, reply);
+                // Finish the reply after the host lookup has finished
             } else {
                 finishReply(reply, PingReply::ErrorNoError);
             }
 
 
-
         } else if (responsePacket->icmp_type == ICMP_DEST_UNREACH) {
 
             // Get the sending package
@@ -463,7 +483,7 @@ void Ping::onSocketReadyRead(int socketDescriptor)
                               << "ID:" << QString("0x%1").arg(icmpId, 4, 16, QChar('0'))
                               << "Sequence:" << icmpSequnceNumber;
 
-            PingReply *reply = m_pendingReplies.take(icmpId);
+            PingReply *reply = m_pendingReplies.value(icmpId);
             if (!reply) {
                 qCDebug(dcPingTraffic()) << "No pending reply for ping echo response unreachable with ID"
                                               << QString("0x%1").arg(icmpId, 4, 16, QChar('0'))

libnymea/network/ping.h

@@ -62,6 +62,8 @@ public:
 
     PingReply::Error error() const;
 
+    int queueCount() const;
+
     PingReply *ping(const QHostAddress &hostAddress, uint retries = 3);
     PingReply *ping(const QHostAddress &hostAddress, bool lookupHost, uint retries = 3);
 
@@ -87,6 +89,7 @@ private:
 
     QQueue<PingReply *> m_replyQueue;
     QTimer *m_queueTimer = nullptr;
+    PingReply *m_currentReply = nullptr;
     void sendNextReply();
     QHash<int, PingReply *> m_pendingHostLookups;