From 2dbd8c47bac3f8ddf8ba8c3696c03d23c2ece51a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=BCrz?= Date: Fri, 3 Jun 2022 16:38:07 +0200 Subject: [PATCH] Add ping retry feature and provide it to the monitor --- .../network/networkdevicediscoveryimpl.cpp | 22 ++++++-- .../network/networkdevicediscoveryimpl.h | 2 +- .../network/networkdevicemonitorimpl.cpp | 24 +++++++++ .../network/networkdevicemonitorimpl.h | 11 +++- libnymea/network/networkdevicediscovery.h | 2 +- libnymea/network/networkdevicemonitor.h | 3 ++ libnymea/network/ping.cpp | 51 +++++++++++++++---- libnymea/network/ping.h | 3 +- libnymea/network/pingreply.cpp | 17 +++++++ libnymea/network/pingreply.h | 11 ++++ 10 files changed, 127 insertions(+), 19 deletions(-) diff --git a/libnymea-core/hardware/network/networkdevicediscoveryimpl.cpp b/libnymea-core/hardware/network/networkdevicediscoveryimpl.cpp index ed86dc82..9ffdb426 100644 --- a/libnymea-core/hardware/network/networkdevicediscoveryimpl.cpp +++ b/libnymea-core/hardware/network/networkdevicediscoveryimpl.cpp @@ -201,10 +201,10 @@ void NetworkDeviceDiscoveryImpl::unregisterMonitor(NetworkDeviceMonitor *network unregisterMonitor(MacAddress(networkDeviceMonitor->networkDeviceInfo().macAddress())); } -PingReply *NetworkDeviceDiscoveryImpl::ping(const QHostAddress &address) +PingReply *NetworkDeviceDiscoveryImpl::ping(const QHostAddress &address, uint retries) { // Note: we use any ping used trough this method also for the monitor evaluation - PingReply *reply = m_ping->ping(address); + PingReply *reply = m_ping->ping(address, retries); connect(reply, &PingReply::finished, this, [=](){ // Search cache for mac address and update last seen @@ -304,7 +304,8 @@ void NetworkDeviceDiscoveryImpl::pingAllNetworkDevices() if (targetAddress == entry.ip()) continue; - PingReply *reply = ping(targetAddress); + // Retry only once to ping a device + PingReply *reply = ping(targetAddress, 1); m_runningPingRepies.append(reply); connect(reply, &PingReply::finished, this, [=](){ m_runningPingRepies.removeAll(reply); @@ -333,7 +334,7 @@ void NetworkDeviceDiscoveryImpl::processMonitorPingResult(PingReply *reply, Netw monitor->setLastSeen(QDateTime::currentDateTime()); monitor->setReachable(true); } else { - qCDebug(dcNetworkDeviceDiscovery()) << "Failed to ping device from" << monitor << reply->error(); + qCDebug(dcNetworkDeviceDiscovery()) << "Failed to ping device from" << monitor << "retrying" << reply->retries() << "times:" << reply->error(); monitor->setReachable(false); } } @@ -452,12 +453,23 @@ void NetworkDeviceDiscoveryImpl::evaluateMonitor(NetworkDeviceMonitorImpl *monit if (monitor->networkDeviceInfo().address().isNull()) return; + if (monitor->currentPingReply()) + return; + // Try to ping first qCDebug(dcNetworkDeviceDiscovery()) << monitor << "try to ping" << monitor->networkDeviceInfo().address().toString(); + PingReply *reply = m_ping->ping(monitor->networkDeviceInfo().address(), monitor->pingRetries()); + monitor->setCurrentPingReply(reply); monitor->setLastConnectionAttempt(currentDateTime); - PingReply *reply = m_ping->ping(monitor->networkDeviceInfo().address()); + connect(reply, &PingReply::retry, monitor, [=](PingReply::Error error, uint retryCount){ + Q_UNUSED(error) + Q_UNUSED(retryCount) + monitor->setLastConnectionAttempt(QDateTime::currentDateTime()); + }); + connect(reply, &PingReply::finished, monitor, [=](){ + monitor->setCurrentPingReply(nullptr); processMonitorPingResult(reply, monitor); }); } diff --git a/libnymea-core/hardware/network/networkdevicediscoveryimpl.h b/libnymea-core/hardware/network/networkdevicediscoveryimpl.h index d5ce9cb5..e8dc9de8 100644 --- a/libnymea-core/hardware/network/networkdevicediscoveryimpl.h +++ b/libnymea-core/hardware/network/networkdevicediscoveryimpl.h @@ -71,7 +71,7 @@ public: void unregisterMonitor(const MacAddress &macAddress) override; void unregisterMonitor(NetworkDeviceMonitor *networkDeviceMonitor) override; - PingReply *ping(const QHostAddress &address) override; + PingReply *ping(const QHostAddress &address, uint retries = 3) override; MacAddressDatabaseReply *lookupMacAddress(const QString &macAddress) override; MacAddressDatabaseReply *lookupMacAddress(const MacAddress &macAddress) override; diff --git a/libnymea-core/hardware/network/networkdevicemonitorimpl.cpp b/libnymea-core/hardware/network/networkdevicemonitorimpl.cpp index a00f797f..b40bfe62 100644 --- a/libnymea-core/hardware/network/networkdevicemonitorimpl.cpp +++ b/libnymea-core/hardware/network/networkdevicemonitorimpl.cpp @@ -29,6 +29,9 @@ * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ #include "networkdevicemonitorimpl.h" +#include "loggingcategories.h" + +Q_DECLARE_LOGGING_CATEGORY(dcNetworkDeviceDiscovery) namespace nymeaserver { @@ -73,6 +76,7 @@ void NetworkDeviceMonitorImpl::setReachable(bool reachable) if (m_reachable == reachable) return; + qCDebug(dcNetworkDeviceDiscovery()) << this << (reachable ? "is now reachable" : "is not reachable any more."); m_reachable = reachable; emit reachableChanged(m_reachable); } @@ -92,6 +96,26 @@ void NetworkDeviceMonitorImpl::setLastSeen(const QDateTime &lastSeen) emit lastSeenChanged(m_lastSeen); } +uint NetworkDeviceMonitorImpl::pingRetries() const +{ + return m_pingRetries; +} + +void NetworkDeviceMonitorImpl::setPingRetries(uint pingRetries) +{ + m_pingRetries = pingRetries; +} + +PingReply *NetworkDeviceMonitorImpl::currentPingReply() const +{ + return m_currentPingReply; +} + +void NetworkDeviceMonitorImpl::setCurrentPingReply(PingReply *reply) +{ + m_currentPingReply = reply; +} + QDateTime NetworkDeviceMonitorImpl::lastConnectionAttempt() const { return m_lastConnectionAttempt; diff --git a/libnymea-core/hardware/network/networkdevicemonitorimpl.h b/libnymea-core/hardware/network/networkdevicemonitorimpl.h index 57720ebe..1363fb80 100644 --- a/libnymea-core/hardware/network/networkdevicemonitorimpl.h +++ b/libnymea-core/hardware/network/networkdevicemonitorimpl.h @@ -35,6 +35,7 @@ #include #include "network/networkdevicemonitor.h" +#include "network/pingreply.h" namespace nymeaserver { @@ -57,16 +58,24 @@ public: QDateTime lastSeen() const override; void setLastSeen(const QDateTime &lastSeen); + uint pingRetries() const override; + void setPingRetries(uint pingRetries) override; + + PingReply *currentPingReply() const; + void setCurrentPingReply(PingReply *reply); + QDateTime lastConnectionAttempt() const; void setLastConnectionAttempt(const QDateTime &lastConnectionAttempt); + private: NetworkDeviceInfo m_networkDeviceInfo; MacAddress m_macAddress; bool m_reachable = false; QDateTime m_lastSeen; QDateTime m_lastConnectionAttempt; - + uint m_pingRetries = 5; + PingReply *m_currentPingReply = nullptr; }; } diff --git a/libnymea/network/networkdevicediscovery.h b/libnymea/network/networkdevicediscovery.h index 422ae79f..c5450add 100644 --- a/libnymea/network/networkdevicediscovery.h +++ b/libnymea/network/networkdevicediscovery.h @@ -60,7 +60,7 @@ public: virtual void unregisterMonitor(const MacAddress &macAddress) = 0; virtual void unregisterMonitor(NetworkDeviceMonitor *networkDeviceMonitor) = 0; - virtual PingReply *ping(const QHostAddress &address) = 0; + virtual PingReply *ping(const QHostAddress &address, uint retries = 3) = 0; virtual MacAddressDatabaseReply *lookupMacAddress(const QString &macAddress) = 0; virtual MacAddressDatabaseReply *lookupMacAddress(const MacAddress &macAddress) = 0; diff --git a/libnymea/network/networkdevicemonitor.h b/libnymea/network/networkdevicemonitor.h index e42d8f22..dcd8f459 100644 --- a/libnymea/network/networkdevicemonitor.h +++ b/libnymea/network/networkdevicemonitor.h @@ -53,6 +53,9 @@ public: virtual bool reachable() const = 0; virtual QDateTime lastSeen() const = 0; + virtual uint pingRetries() const = 0; + virtual void setPingRetries(uint pingRetries) = 0; + signals: void reachableChanged(bool reachable); void lastSeenChanged(const QDateTime &lastSeen); diff --git a/libnymea/network/ping.cpp b/libnymea/network/ping.cpp index a16202c7..bb1638ff 100644 --- a/libnymea/network/ping.cpp +++ b/libnymea/network/ping.cpp @@ -111,11 +111,21 @@ PingReply::Error Ping::error() const return m_error; } -PingReply *Ping::ping(const QHostAddress &hostAddress) +PingReply *Ping::ping(const QHostAddress &hostAddress, uint retries) { PingReply *reply = new PingReply(this); reply->m_targetHostAddress = hostAddress; reply->m_networkInterface = NetworkUtils::getInterfaceForHostaddress(hostAddress); + reply->m_reties = retries; + + connect(reply, &PingReply::timeout, this, [=](){ + // Note: this is not the ICMP timeout, here we actually got nothing from nobody... + finishReply(reply, PingReply::ErrorTimeout); + }); + + connect(reply, &PingReply::aborted, this, [=](){ + finishReply(reply, PingReply::ErrorAborted); + }); // Perform the reply in the next event loop to give the user time to do the reply connects m_replyQueue.enqueue(reply); @@ -206,10 +216,7 @@ void Ping::performPing(PingReply *reply) // Start reply timer and handle timeout m_pendingReplies.insert(reply->requestId(), reply); - reply->m_timer->start(8000); - connect(reply, &PingReply::timeout, this, [=](){ - finishReply(reply, PingReply::ErrorTimeout); - }); + reply->m_timer->start(m_timeoutDuration); } void Ping::verifyErrno(int error) @@ -289,10 +296,34 @@ quint16 Ping::calculateRequestId() void Ping::finishReply(PingReply *reply, PingReply::Error error) { - reply->m_error = error; - m_pendingReplies.remove(reply->requestId()); - emit reply->finished(); - reply->deleteLater(); + // Check if we should retry + if (reply->m_retryCount >= reply->m_reties || + error == PingReply::ErrorNoError || + error == PingReply::ErrorAborted || + error == PingReply::ErrorInvalidHostAddress || + error == PingReply::ErrorPermissionDenied) { + // No retry, we are done + reply->m_error = error; + reply->m_timer->stop(); + m_pendingReplies.remove(reply->requestId()); + emit reply->finished(); + reply->deleteLater(); + } else { + m_pendingReplies.remove(reply->requestId()); + reply->m_error = error; + reply->m_retryCount++; + reply->m_sequenceNumber++; + + qCDebug(dcPing()) << "Ping finished with error" << error << "Retry ping" << reply->targetHostAddress().toString() << reply->m_retryCount << "/" << reply->m_reties; + emit reply->retry(error, reply->retryCount()); + + // Note: will be restarted once actually sent trough the network + reply->m_timer->stop(); + + // Re-Enqueu the reply + m_replyQueue.enqueue(reply); + sendNextReply(); + } } void Ping::onSocketReadyRead(int socketDescriptor) @@ -360,7 +391,7 @@ void Ping::onSocketReadyRead(int socketDescriptor) int lookupId = QHostInfo::lookupHost(senderAddress.toString(), this, SLOT(onHostLookupFinished(QHostInfo))); m_pendingHostLookups.insert(lookupId, reply); - qCDebug(dcPingTraffic()) << "Received ICMP response" << reply->targetHostAddress().toString() << ICMP_PACKET_SIZE << "[Bytes]" + qCDebug(dcPing()) << "Received ICMP response" << reply->targetHostAddress().toString() << ICMP_PACKET_SIZE << "[Bytes]" << "ID:" << QString("0x%1").arg(responsePacket->icmp_id, 4, 16, QChar('0')) << "Sequence:" << htons(responsePacket->icmp_seq) << "Time:" << reply->duration() << "[ms]"; diff --git a/libnymea/network/ping.h b/libnymea/network/ping.h index 3ef536e2..dc7b4900 100644 --- a/libnymea/network/ping.h +++ b/libnymea/network/ping.h @@ -62,7 +62,7 @@ public: PingReply::Error error() const; - PingReply *ping(const QHostAddress &hostAddress); + PingReply *ping(const QHostAddress &hostAddress, uint retries = 3); signals: void availableChanged(bool available); @@ -76,6 +76,7 @@ private: // Config QByteArray m_payload = "ping from nymea"; PingReply::Error m_error = PingReply::ErrorNoError; + uint m_timeoutDuration = 5000; // Socket QSocketNotifier *m_socketNotifier = nullptr; diff --git a/libnymea/network/pingreply.cpp b/libnymea/network/pingreply.cpp index c3fa499e..8afb2752 100644 --- a/libnymea/network/pingreply.cpp +++ b/libnymea/network/pingreply.cpp @@ -63,6 +63,16 @@ QNetworkInterface PingReply::networkInterface() const return m_networkInterface; } +uint PingReply::retries() const +{ + return m_reties; +} + +uint PingReply::retryCount() const +{ + return m_retryCount; +} + double PingReply::duration() const { return m_duration; @@ -72,3 +82,10 @@ PingReply::Error PingReply::error() const { return m_error; } + +void PingReply::abort() +{ + m_timer->stop(); + m_error = ErrorAborted; + emit aborted(); +} diff --git a/libnymea/network/pingreply.h b/libnymea/network/pingreply.h index 8bf8ca19..72f8fcc4 100644 --- a/libnymea/network/pingreply.h +++ b/libnymea/network/pingreply.h @@ -51,6 +51,7 @@ class LIBNYMEA_EXPORT PingReply : public QObject public: enum Error { ErrorNoError, + ErrorAborted, ErrorInvalidResponse, ErrorNetworkDown, ErrorNetworkUnreachable, @@ -70,13 +71,21 @@ public: QString hostName() const; QNetworkInterface networkInterface() const; + uint retries() const; + uint retryCount() const; + double duration() const; Error error() const; +public slots: + void abort(); + signals: void finished(); void timeout(); + void retry(Error error, uint retryCount); + void aborted(); private: QTimer *m_timer = nullptr; @@ -86,6 +95,8 @@ private: QString m_hostName; QNetworkInterface m_networkInterface; + uint m_reties = 0; + uint m_retryCount = 0; uint m_timeout = 3; double m_duration = 0; Error m_error = ErrorNoError;