sunspec: sweep orphan children against every connection at once

processDiscoveryResult() makes child things disappear when their model
is no longer announced. It used to confront them with the models of the
one connection that had just finished discovering, which is only half of
the picture on a thing carrying two: the inverter discovery would delete
the meter child, and the meter discovery the inverter and storage ones,
on every single reconnect.

The sweep now waits for every connection of the parent to have reported
a successful discovery, then confronts the children with the union of
their models. A grace period releases it if a connection never reports,
so a hung discovery cannot freeze the cleanup for good.

That grace period is inferred, not measured: it is set generously
because sweeping too late leaves a stale child showing as disconnected,
while sweeping too early deletes a thing of the user. Recorded as D-2 in
DIVERGENCES.md with what to measure to replace it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

sunspec/integrationpluginsunspec.cpp

@@ -56,6 +56,15 @@
 
 #include <QHostAddress>
 
+/* How long the orphan sweep waits for a connection that has not reported yet
+ * before going ahead without it.
+ *
+ * INFERRED, no measurement backs this value: see D-2 in DIVERGENCES.md. It is
+ * deliberately generous, because the two failure modes are not comparable.
+ * Sweeping too late leaves a stale child thing showing as disconnected;
+ * sweeping too early deletes a thing of the user. */
+static const int OrphanSweepGraceSeconds = 600;
+
 IntegrationPluginSunSpec::IntegrationPluginSunSpec()
 {
 
@@ -840,6 +849,20 @@ void IntegrationPluginSunSpec::processDiscoveryResult(Thing *thing, SunSpecConne
     if (connection->models().isEmpty())
         return;
 
+    /* With more than one connection under the same parent, one connection only
+     * ever sees half of the picture. Sweeping on that half would make the
+     * children of the other half disappear on every single discovery. So the
+     * sweep waits for every connection of this parent to have reported, and
+     * confronts the children with the union of their models. */
+    if (!orphanSweepAllowed(thing))
+        return;
+
+    QList<SunSpecModel *> knownModels;
+    foreach (SunSpecConnection *parentConnection, connectionsFor(thing->id())) {
+        if (m_discoveredConnections.contains(parentConnection))
+            knownModels.append(parentConnection->models());
+    }
+
     foreach (Thing *child, myThings().filterByParentId(thing->id())) {
         if (!m_modelIdParamTypeIds.contains(child->thingClassId()) || !m_modbusAddressParamTypeIds.contains(child->thingClassId()))
             continue;
@@ -848,7 +871,7 @@ void IntegrationPluginSunSpec::processDiscoveryResult(Thing *thing, SunSpecConne
         uint childModbusAddress = child->paramValue(m_modbusAddressParamTypeIds.value(child->thingClassId())).toUInt();
 
         bool modelFoundForChild = false;
-        foreach (SunSpecModel *model, connection->models()) {
+        foreach (SunSpecModel *model, knownModels) {
             if (childModelId == model->modelId() && childModbusAddress == model->modbusStartRegister()) {
                 modelFoundForChild = true;
                 break;
@@ -856,12 +879,51 @@ void IntegrationPluginSunSpec::processDiscoveryResult(Thing *thing, SunSpecConne
         }
 
         if (!modelFoundForChild) {
-            qCInfo(dcSunSpec()) << "The model for" << child << "does not seem to be available any more on" << connection << "Removing the device since it does not seem to exist ony more on this connection.";
+            qCInfo(dcSunSpec()) << "The model for" << child << "does not seem to be available any more on" << thing->name() << "Removing the device since it does not seem to exist ony more on this connection.";
             emit autoThingDisappeared(child->id());
         }
     }
 }
 
+bool IntegrationPluginSunSpec::orphanSweepAllowed(Thing *thing)
+{
+    bool everyConnectionReported = true;
+    foreach (SunSpecConnection *connection, connectionsFor(thing->id())) {
+        if (!m_discoveredConnections.contains(connection)) {
+            everyConnectionReported = false;
+            break;
+        }
+    }
+
+    if (everyConnectionReported) {
+        m_orphanSweepDeadlines.remove(thing->id());
+        return true;
+    }
+
+    /* Hold the sweep back, but not forever: a connection whose discovery never
+     * returns would otherwise freeze it for good, and a child thing left over
+     * from a previous setup would never be cleaned up.
+     *
+     * The grace period is deliberately generous. The two outcomes are not
+     * comparable: sweeping too late leaves a stale thing showing as
+     * disconnected, sweeping too early deletes a thing of the user. */
+    if (!m_orphanSweepDeadlines.contains(thing->id())) {
+        m_orphanSweepDeadlines.insert(thing->id(), QDateTime::currentDateTime().addSecs(OrphanSweepGraceSeconds));
+        qCDebug(dcSunSpec()) << "Holding the orphan sweep of" << thing->name()
+                             << "back until every connection has reported, or for"
+                             << OrphanSweepGraceSeconds << "s at the most.";
+        return false;
+    }
+
+    if (QDateTime::currentDateTime() < m_orphanSweepDeadlines.value(thing->id()))
+        return false;
+
+    qCWarning(dcSunSpec()) << "A connection of" << thing->name() << "never finished a discovery within"
+                           << OrphanSweepGraceSeconds << "s. Sweeping orphan children against the connections"
+                           << "that did report.";
+    return true;
+}
+
 
 /* Creates one SunSpec connection for a thing.
  *
@@ -1068,6 +1130,8 @@ void IntegrationPluginSunSpec::destroyConnections(const ThingId &thingId)
 
     if (m_meterConnections.contains(thingId))
         m_meterConnections.take(thingId)->deleteLater();
+
+    m_orphanSweepDeadlines.remove(thingId);
 }
 
 SunSpecConnection *IntegrationPluginSunSpec::connectionForChild(Thing *child) const

sunspec/integrationpluginsunspec.h

@@ -118,6 +118,15 @@ private:
     bool sunspecThingAlreadyAdded(uint modelId, uint modbusAddress, const ThingId &parentId);
     void processDiscoveryResult(Thing *thing, SunSpecConnection *connection);
 
+    /* Whether the children of a parent may be confronted with the models found
+     * so far. False while a connection of that parent has not reported yet, so
+     * that one connection does not sweep away the children of the other.
+     *
+     * INFERRED: no measurement backs this grace period. See D-2 in
+     * DIVERGENCES.md for how to replace it with one. */
+    bool orphanSweepAllowed(Thing *thing);
+    QHash<ThingId, QDateTime> m_orphanSweepDeadlines;
+
     // SunSpec things
     SunSpecConnection *createConnection(Thing *thing, uint slaveId, bool isPrimary);