From 5471a5da34033da374ba3e317f8ebcb62ac0cfdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=BCrz?= Date: Thu, 29 Sep 2022 09:42:38 +0200 Subject: [PATCH] Network device discovery: create reply for each discovery request --- .../network/networkdevicediscoveryimpl.cpp | 74 +++++++++++---- .../networkdevicediscoveryreplyimpl.cpp | 89 +++++++++++++------ .../network/networkdevicediscoveryreplyimpl.h | 23 +++-- .../network/networkdevicediscoveryreply.h | 2 + 4 files changed, 133 insertions(+), 55 deletions(-) diff --git a/libnymea-core/hardware/network/networkdevicediscoveryimpl.cpp b/libnymea-core/hardware/network/networkdevicediscoveryimpl.cpp index 3b4a16ab..025e4420 100644 --- a/libnymea-core/hardware/network/networkdevicediscoveryimpl.cpp +++ b/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; diff --git a/libnymea-core/hardware/network/networkdevicediscoveryreplyimpl.cpp b/libnymea-core/hardware/network/networkdevicediscoveryreplyimpl.cpp index 63a6547f..ec644bf3 100644 --- a/libnymea-core/hardware/network/networkdevicediscoveryreplyimpl.cpp +++ b/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]); + } + } +} + } diff --git a/libnymea-core/hardware/network/networkdevicediscoveryreplyimpl.h b/libnymea-core/hardware/network/networkdevicediscoveryreplyimpl.h index 5c06ef3f..e3c3e144 100644 --- a/libnymea-core/hardware/network/networkdevicediscoveryreplyimpl.h +++ b/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 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 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(); }; } diff --git a/libnymea/network/networkdevicediscoveryreply.h b/libnymea/network/networkdevicediscoveryreply.h index 9417fdf5..bc7d205c 100644 --- a/libnymea/network/networkdevicediscoveryreply.h +++ b/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);