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);