From e1e91a19c3d8422d6af41871659a9de010caedc5 Mon Sep 17 00:00:00 2001 From: Patrick Schurig Date: Sun, 2 Aug 2026 09:02:35 +0200 Subject: [PATCH] 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) --- sunspec/integrationpluginsunspec.cpp | 68 +++++++++++++++++++++++++++- sunspec/integrationpluginsunspec.h | 9 ++++ 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/sunspec/integrationpluginsunspec.cpp b/sunspec/integrationpluginsunspec.cpp index df2ea08..e033fc6 100644 --- a/sunspec/integrationpluginsunspec.cpp +++ b/sunspec/integrationpluginsunspec.cpp @@ -56,6 +56,15 @@ #include +/* 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 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 diff --git a/sunspec/integrationpluginsunspec.h b/sunspec/integrationpluginsunspec.h index 9c60331..8ba7aea 100644 --- a/sunspec/integrationpluginsunspec.h +++ b/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 m_orphanSweepDeadlines; + // SunSpec things SunSpecConnection *createConnection(Thing *thing, uint slaveId, bool isPrimary);