diff --git a/libnymea-core/hardware/network/networkdevicediscoveryimpl.cpp b/libnymea-core/hardware/network/networkdevicediscoveryimpl.cpp index 70c1f670..863c425f 100644 --- a/libnymea-core/hardware/network/networkdevicediscoveryimpl.cpp +++ b/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(); diff --git a/libnymea/network/ping.cpp b/libnymea/network/ping.cpp index ec8aa1cd..be53b8e4 100644 --- a/libnymea/network/ping.cpp +++ b/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(); - qCDebug(dcPing()) << "Send next reply," << m_replyQueue.count() << "left in queue"; + m_currentReply = m_replyQueue.dequeue(); + 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')) diff --git a/libnymea/network/ping.h b/libnymea/network/ping.h index 42f26cea..7da73d6c 100644 --- a/libnymea/network/ping.h +++ b/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 m_replyQueue; QTimer *m_queueTimer = nullptr; + PingReply *m_currentReply = nullptr; void sendNextReply(); QHash m_pendingHostLookups;