Merge PR #716: NetworkDeviceMonitor: Improve ping and network device monitor behavior

AGENTS.md

@@ -0,0 +1,40 @@
+# Repository Guidelines
+
+## Project Structure & Module Organization
+- `server/`: nymead entry points, service glue, and startup helpers.  
+- `libnymea/`, `libnymea-core/`: reusable device APIs, JSON-RPC helpers, shared Qt utilities.  
+- `plugins/`: protocol modules, each with scoped sources, resources, and metadata (`*.json`).  
+- `tests/`: `auto/` (QtTest suites), `libnymea-tests/` (fixtures), `tools/` (harness utilities).  
+- `doc/`, `icons/`, `translations/`, `data/`: docs, assets, localization, sample configs; `debian*/`: packaging. Keep new code beside the layer it extends to avoid cross-module coupling.
+
+## Build, Test, and Development Commands
+```bash
+qmake nymea.pro && make -j"$(nproc)"            # full build
+make install INSTALL_ROOT=$PWD/stage            # staged install
+./nymead -c data/nymead.conf                    # run locally
+qmake tests/tests.pro && make check             # run QtTest suites
+```
+Select the required Qt version before calling qmake (Qt 5 or Qt 6). Component-only work can be built from its subdir (e.g., `cd server && qmake server.pro`). Enable verbose logs with `QT_LOGGING_RULES="*.debug=true"`.
+
+## Coding Style & Naming Conventions
+- Qt style: 4-space indent, braces on their own line, `CamelCase` classes, `lowerCamelCase` functions/members.
+- Prefer Qt containers/signals over STL; wrap log output in `NYMEA_LOGGING_CATEGORY`.  
+- C++/Qt: follow Qt conventions (camelCase, `m_` member prefixes, grouped/alphabetised includes, Qt containers, Qt logging macros).
+- Keep plugin IDs, translation filenames, and resource prefixes aligned with their directory names.  
+- When user-visible text changes, refresh translations via `tools/update-translations.sh` (runs `lupdate`/`lrelease`).
+
+## Testing Guidelines
+- Tests sit in `tests/auto/<topic>/test*.cpp` and use `QTEST_MAIN`.  
+- `qmake tests/tests.pro` generates makefiles; `make check` executes every target (use `make -C tests/<suite>` for a slice).  
+- Mirror production directories when creating suites, mock I/O with helpers in `tests/tools/`, and cover both success paths and primary failure modes before sending a PR.
+
+## Commit & Pull Request Guidelines
+- Follow history conventions: imperative subjects under ~72 chars (“Add backup configuration…”).  
+- Bodies should mention motivation and key testing steps; tag related issues.  
+- PRs require scope summary, validation proof (`Tests: qmake tests && make check`), and screenshots or API traces whenever behavior changes.  
+- Call out configuration or packaging impacts (e.g., added plugin JSON, changed defaults) so reviewers can replicate.
+
+## Security & Configuration Tips
+- Grant network discovery rights after manual installs: `sudo setcap cap_net_admin,cap_net_raw=eip /path/to/nymead`.  
+- Store secrets in local overrides such as `/etc/nymea/*.conf` instead of `data/`.  
+- Review logging before submission to ensure credentials and tokens remain redacted, and describe any new capability or permission requirements inside the PR.

libnymea-core/hardware/network/networkdevicediscoveryimpl.cpp

@@ -515,15 +515,12 @@ void NetworkDeviceDiscoveryImpl::processMonitorPingResult(PingReply *reply, Netw
                                                         << "-->"
                                                         << reply->targetHostAddress().toString();
 
-                    removeFromNetworkDeviceCache(oldAddress);
-
                     NetworkDeviceInfo info = m_networkInfoCache.at(i);
+                    removeFromNetworkDeviceCache(oldAddress);
                     info.setAddress(reply->targetHostAddress());
 
                     monitor->setNetworkDeviceInfo(info);
-                    m_networkInfoCache[i] = info;
-                    m_networkInfoCache.sortNetworkDevices();
-                    saveNetworkDeviceCache(info);
+                    updateCache(info);
                     break;
                 }
             }
@@ -812,14 +809,11 @@ void NetworkDeviceDiscoveryImpl::processArpTraffic(const QNetworkInterface &inte
                                                     << "-->"
                                                     << address.toString();
 
-                removeFromNetworkDeviceCache(oldAddress);
-
                 NetworkDeviceInfo info = m_networkInfoCache.at(i);
+                removeFromNetworkDeviceCache(oldAddress);
                 info.setAddress(address);
 
-                m_networkInfoCache[i] = info;
-                m_networkInfoCache.sortNetworkDevices();
-                saveNetworkDeviceCache(info);
+                updateCache(info);
 
                 foreach (NetworkDeviceMonitorImpl *monitor, m_monitors.keys()) {
                     if (monitor->macAddress() == macAddress) {

libnymea-core/hardware/network/networkdevicemonitorimpl.cpp

@@ -73,7 +73,15 @@ NetworkDeviceInfo::MonitorMode NetworkDeviceMonitorImpl::monitorMode() const
 
 void NetworkDeviceMonitorImpl::setMonitorMode(NetworkDeviceInfo::MonitorMode monitorMode)
 {
+    if (m_monitorMode == monitorMode)
+        return;
+
     m_monitorMode = monitorMode;
+
+    if (m_networkDeviceInfo.monitorMode() != monitorMode) {
+        m_networkDeviceInfo.setMonitorMode(monitorMode);
+        emit networkDeviceInfoChanged(m_networkDeviceInfo);
+    }
 }
 
 NetworkDeviceInfo NetworkDeviceMonitorImpl::networkDeviceInfo() const
@@ -126,7 +134,11 @@ uint NetworkDeviceMonitorImpl::pingRetries() const
 
 void NetworkDeviceMonitorImpl::setPingRetries(uint pingRetries)
 {
+    if (m_pingRetries == pingRetries)
+        return;
+
     m_pingRetries = pingRetries;
+    emit pingRetriesChanged(m_pingRetries);
 }
 
 PingReply *NetworkDeviceMonitorImpl::currentPingReply() const
@@ -154,12 +166,12 @@ bool NetworkDeviceMonitorImpl::isMyNetworkDeviceInfo(const NetworkDeviceInfo &ne
     bool myNetworkDevice = false;
     switch (m_monitorMode) {
     case NetworkDeviceInfo::MonitorModeMac:
-        if (!m_macAddress.isNull() && networkDeviceInfo.macAddressInfos().count() == 1 && networkDeviceInfo.macAddressInfos().hasMacAddress(m_macAddress))
+        if (!m_macAddress.isNull() && networkDeviceInfo.macAddressInfos().hasMacAddress(m_macAddress))
             myNetworkDevice = true;
 
         break;
     case NetworkDeviceInfo::MonitorModeHostName:
-        if (!m_hostName.isEmpty() && networkDeviceInfo.hostName() == m_hostName)
+        if (!m_hostName.isEmpty() && networkDeviceInfo.hostName().compare(m_hostName, Qt::CaseInsensitive) == 0)
             myNetworkDevice = true;
 
         break;
@@ -172,12 +184,14 @@ bool NetworkDeviceMonitorImpl::isMyNetworkDeviceInfo(const NetworkDeviceInfo &ne
     return myNetworkDevice;
 }
 
-bool NetworkDeviceMonitorImpl::operator==(NetworkDeviceMonitorImpl *other) const
+bool NetworkDeviceMonitorImpl::operator==(const NetworkDeviceMonitorImpl &other) const
 {
-    return m_macAddress == other->macAddress() && m_hostName == other->hostName() && m_address == other->address();
+    return m_macAddress == other.macAddress()
+            && m_hostName == other.hostName()
+            && m_address == other.address();
 }
 
-bool NetworkDeviceMonitorImpl::operator!=(NetworkDeviceMonitorImpl *other) const
+bool NetworkDeviceMonitorImpl::operator!=(const NetworkDeviceMonitorImpl &other) const
 {
     return !operator==(other);
 }

libnymea-core/hardware/network/networkdevicemonitorimpl.h

@@ -75,8 +75,8 @@ public:
 
     bool isMyNetworkDeviceInfo(const NetworkDeviceInfo &networkDeviceInfo) const;
 
-    bool operator==(NetworkDeviceMonitorImpl *other) const;
-    bool operator!=(NetworkDeviceMonitorImpl *other) const;
+    bool operator==(const NetworkDeviceMonitorImpl &other) const;
+    bool operator!=(const NetworkDeviceMonitorImpl &other) const;
 
 private:
     MacAddress m_macAddress;

libnymea/network/networkdeviceinfo.cpp

@@ -31,6 +31,10 @@
 #include "networkdeviceinfo.h"
 #include "macaddress.h"
 
+#include <QMetaType>
+
+static const int networkDeviceInfoMetaTypeId = qRegisterMetaType<NetworkDeviceInfo>("NetworkDeviceInfo");
+
 NetworkDeviceInfo::NetworkDeviceInfo()
 {
 

libnymea/network/networkdeviceinfo.h

@@ -36,6 +36,7 @@
 #include <QDateTime>
 #include <QHostAddress>
 #include <QNetworkInterface>
+#include <QMetaType>
 
 #include "libnymea.h"
 #include "macaddressinfos.h"
@@ -104,4 +105,6 @@ private:
 
 QDebug operator<<(QDebug debug, const NetworkDeviceInfo &networkDeviceInfo);
 
+Q_DECLARE_METATYPE(NetworkDeviceInfo)
+
 #endif // NETWORKDEVICEINFO_H

libnymea/network/ping.cpp

@@ -191,6 +191,13 @@ void Ping::performPing(PingReply *reply)
 
     // Get host ip address
     struct hostent *hostname = gethostbyname(reply->targetHostAddress().toString().toLocal8Bit().constData());
+    if (!hostname) {
+        m_error = PingReply::ErrorHostNameLookupFailed;
+        qCWarning(dcPing()) << "Failed to resolve host" << reply->targetHostAddress().toString() << hstrerror(h_errno);
+        finishReply(reply, m_error);
+        return;
+    }
+
     struct sockaddr_in pingAddress;
     memset(&pingAddress, 0, sizeof(pingAddress));
     pingAddress.sin_family = hostname->h_addrtype;
@@ -421,6 +428,12 @@ void Ping::cleanUpReply(PingReply *reply)
         QHostInfo::abortHostLookup(lookupId);
         m_pendingHostNameLookups.remove(lookupId);
     }
+
+    if (m_pendingHostAddressLookups.values().contains(reply)) {
+        int lookupId = m_pendingHostAddressLookups.key(reply);
+        QHostInfo::abortHostLookup(lookupId);
+        m_pendingHostAddressLookups.remove(lookupId);
+    }
 }
 
 void Ping::onSocketReadyRead(int socketDescriptor)
@@ -493,6 +506,8 @@ void Ping::onSocketReadyRead(int socketDescriptor)
                               << "Time:" << reply->duration() << "[ms]";
 
             if (reply->doHostLookup()) {
+                // Prevent timeout while waiting for the hostname lookup to finish
+                reply->m_timer->stop();
                 // 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_pendingHostNameLookups.insert(lookupId, reply);