Don't destroy things when removing ZigBee nodes from the network

This makes it easier to remove/rejoin zigbee nodes without losing all magic and other setup related to a thing. Comes with the cost of having to manually remove a thing when removing a thing from the ZigBee network for good. That should be a bearable cost though, as removing a Thing will still remove the associated node in one go.
nymea · Sep 26, 2023 · 431c981 · 431c981
1 parent 482224d
commit 431c981
Show file tree

Hide file tree

Showing 24 changed files with 280 additions and 185 deletions.
diff --git a/common/zigbeeintegrationplugin.cpp b/common/zigbeeintegrationplugin.cpp
@@ -51,6 +51,7 @@
 #include <zcl/measurement/zigbeeclusteroccupancysensing.h>
 #include <zcl/ota/zigbeeclusterota.h>
 #include <zcl/closures/zigbeeclusterwindowcovering.h>
+#include <zcl/closures/zigbeeclusterdoorlock.h>
 
 #include <QColor>
 #include <QNetworkRequest>
@@ -83,10 +84,16 @@ void ZigbeeIntegrationPlugin::handleRemoveNode(ZigbeeNode *node, const QUuid &ne
 {
     Q_UNUSED(networkUuid)
     foreach (Thing *thing, m_thingNodes.keys(node)) {
-        emit autoThingDisappeared(thing->id());
 
-        // Removing it from our map to prevent a loop that would ask the zigbee network to remove this node (see thingRemoved())
-        m_thingNodes.remove(thing);
+        // Not removing thing as this destroys magic too easily if a device decides to leave and rejoin for whatever reason
+        // While in theory that's not a use case, in practice it turns out that this is a common thing to do when fiddling with
+        // ZigBee network instabilites. Instead we're just marking things as disconnected.
+
+//        emit autoThingDisappeared(thing->id());
+//        // Removing it from our map to prevent a loop that would ask the zigbee network to remove this node (see thingRemoved())
+//        m_thingNodes.remove(thing);
+
+        thing->setStateValue("connected", false);
     }
 }
 
@@ -113,39 +120,7 @@ ZigbeeNode* ZigbeeIntegrationPlugin::manageNode(Thing *thing)
         return nullptr;
     }
 
-    m_thingNodes.insert(thing, node);
-
-    // Update connected state
-    thing->setStateValue("connected", node->reachable());
-    connect(node, &ZigbeeNode::reachableChanged, thing, [thing](bool reachable){
-        thing->setStateValue("connected", reachable);
-    });
-
-    // Update signal strength
-    thing->setStateValue("signalStrength", qRound(node->lqi() * 100.0 / 255.0));
-    connect(node, &ZigbeeNode::lqiChanged, thing, [thing](quint8 lqi){
-        uint signalStrength = qRound(lqi * 100.0 / 255.0);
-        thing->setStateValue("signalStrength", signalStrength);
-    });
-
-    connect(node, &ZigbeeNode::lastSeenChanged, this, [=](){
-        while (!m_delayedWriteRequests.value(node).isEmpty()) {
-            DelayedAttributeWriteRequest request = m_delayedWriteRequests[node].takeFirst();
-            ZigbeeClusterReply *reply = request.cluster->writeAttributes(request.records, request.manufacturerCode);
-            connect(reply, &ZigbeeClusterReply::finished, this, [=](){
-                if (reply->error() != ZigbeeClusterReply::ErrorNoError) {
-                    qCWarning(m_dc) << "Error writing attributes on" << thing->name();
-                }
-            });
-        }
-        while (!m_delayedReadRequests.value(node).isEmpty()) {
-            DelayedAttributeReadRequest request = m_delayedReadRequests[node].takeFirst();
-            ZigbeeClusterReply *reply = request.cluster->readAttributes(request.attributes, request.manufacturerCode);
-            if (reply->error() != ZigbeeClusterReply::ErrorNoError) {
-                qCWarning(m_dc) << "Error writing attributes on" << thing->name();
-            }
-        }
-    });
+    setupNode(node, thing);
 
     return node;
 }
@@ -160,7 +135,7 @@ ZigbeeNode *ZigbeeIntegrationPlugin::nodeForThing(Thing *thing)
     return m_thingNodes.value(thing);
 }
 
-void ZigbeeIntegrationPlugin::createThing(const ThingClassId &thingClassId, ZigbeeNode *node, const ParamList &additionalParams)
+Thing *ZigbeeIntegrationPlugin::createThing(const ThingClassId &thingClassId, ZigbeeNode *node, const ParamList &additionalParams)
 {
     ThingDescriptor descriptor(thingClassId);
     QString deviceClassName = supportedThings().findById(thingClassId).displayName();
@@ -172,7 +147,15 @@ void ZigbeeIntegrationPlugin::createThing(const ThingClassId &thingClassId, Zigb
     params.append(Param(tc.paramTypes().findByName("ieeeAddress").id(), node->extendedAddress().toString()));
     params.append(additionalParams);
     descriptor.setParams(params);
-    emit autoThingsAppeared({descriptor});
+
+    Thing *existingThing = myThings().findByParams(params);
+    if (!existingThing) {
+        emit autoThingsAppeared({descriptor});
+    } else {
+        qCInfo(m_dc) << "Thing for node" << node << "already existing. Not recreating.";
+        setupNode(node, existingThing);
+    }
+    return existingThing;
 }
 
 void ZigbeeIntegrationPlugin::bindCluster(ZigbeeNodeEndpoint *endpoint, ZigbeeClusterLibrary::ClusterId clusterId, int retries)
@@ -599,6 +582,27 @@ void ZigbeeIntegrationPlugin::configureWindowCoveringInputClusterLiftPercentageA
     });
 }
 
+void ZigbeeIntegrationPlugin::configureDoorLockInputClusterAttributeReporting(ZigbeeNodeEndpoint *endpoint)
+{
+    ZigbeeClusterLibrary::AttributeReportingConfiguration reportingConfig;
+    reportingConfig.attributeId = ZigbeeClusterDoorLock::AttributeLockState;
+    reportingConfig.dataType = Zigbee::Enum8;
+    reportingConfig.minReportingInterval = 60;
+    reportingConfig.maxReportingInterval = 120;
+    reportingConfig.reportableChange = ZigbeeDataType(static_cast<quint8>(1)).data();
+
+    qCDebug(m_dc()) << "Configuring attribute reporting for door lock cluster lock state";
+    ZigbeeClusterReply *reportingReply = endpoint->getInputCluster(ZigbeeClusterLibrary::ClusterIdDoorLock)->configureReporting({reportingConfig});
+    connect(reportingReply, &ZigbeeClusterReply::finished, this, [=](){
+        if (reportingReply->error() != ZigbeeClusterReply::ErrorNoError) {
+            qCWarning(m_dc()) << "Failed to door lock cluster door state attribute reporting" << reportingReply->error();
+        } else {
+            qCDebug(m_dc()) << "Attribute reporting configuration finished for door lock cluster lock state" << ZigbeeClusterLibrary::parseAttributeReportingStatusRecords(reportingReply->responseFrame().payload);
+        }
+    });
+
+}
+
 void ZigbeeIntegrationPlugin::connectToPowerConfigurationInputCluster(Thing *thing, ZigbeeNodeEndpoint *endpoint, qreal maxVoltage, qreal minVoltage)
 {
     ZigbeeClusterPowerConfiguration *powerCluster = endpoint->inputCluster<ZigbeeClusterPowerConfiguration>(ZigbeeClusterLibrary::ClusterIdPowerConfiguration);
@@ -1077,6 +1081,10 @@ void ZigbeeIntegrationPlugin::connectToOtaOutputCluster(Thing *thing, ZigbeeNode
         otaCluster->setProperty("lastFirmwareCheck", QDateTime::currentDateTime());
 
         ZigbeeNode *node = nodeForThing(thing);
+        if (!node) {
+            qCWarning(m_dc) << "Node for thing" << thing << "not found. Cannot continue with OTA";
+            return;
+        }
         FirmwareIndexEntry newInfo = checkFirmwareAvailability(m_firmwareIndex, manufacturerCode, imageType, currentFileVersion, node->modelName());
         ZigbeeClusterOta::FileVersion currentParsed = ZigbeeClusterOta::parseFileVersion(currentFileVersion);
         thing->setStateValue("currentVersion", QString("%0.%1.%2.%3")
@@ -1657,6 +1665,48 @@ void ZigbeeIntegrationPlugin::updateFirmwareIndex()
     });
 }
 
+void ZigbeeIntegrationPlugin::setupNode(ZigbeeNode *node, Thing *thing)
+{
+    m_thingNodes.insert(thing, node);
+
+    // Delaying the connection setup to only set state values after the thing setup finished
+    QTimer::singleShot(0, thing, [=](){
+        // Update connected state
+        thing->setStateValue("connected", node->reachable());
+        connect(node, &ZigbeeNode::reachableChanged, thing, [thing](bool reachable){
+            thing->setStateValue("connected", reachable);
+        });
+
+        // Update signal strength
+        thing->setStateValue("signalStrength", qRound(node->lqi() * 100.0 / 255.0));
+        connect(node, &ZigbeeNode::lqiChanged, thing, [thing](quint8 lqi){
+            uint signalStrength = qRound(lqi * 100.0 / 255.0);
+            thing->setStateValue("signalStrength", signalStrength);
+        });
+
+        connect(node, &ZigbeeNode::lastSeenChanged, this, [=](){
+            while (!m_delayedWriteRequests.value(node).isEmpty()) {
+                DelayedAttributeWriteRequest request = m_delayedWriteRequests[node].takeFirst();
+                ZigbeeClusterReply *reply = request.cluster->writeAttributes(request.records, request.manufacturerCode);
+                connect(reply, &ZigbeeClusterReply::finished, this, [=](){
+                    if (reply->error() != ZigbeeClusterReply::ErrorNoError) {
+                        qCWarning(m_dc) << "Error writing attributes on" << thing->name();
+                    }
+                });
+            }
+            while (!m_delayedReadRequests.value(node).isEmpty()) {
+                DelayedAttributeReadRequest request = m_delayedReadRequests[node].takeFirst();
+                ZigbeeClusterReply *reply = request.cluster->readAttributes(request.attributes, request.manufacturerCode);
+                if (reply->error() != ZigbeeClusterReply::ErrorNoError) {
+                    qCWarning(m_dc) << "Error writing attributes on" << thing->name();
+                }
+            }
+        });
+
+        createConnections(thing);
+    });
+}
+
 ZigbeeIntegrationPlugin::FirmwareIndexEntry ZigbeeIntegrationPlugin::firmwareInfo(quint16 manufacturerId, quint16 imageType, quint32 fileVersion) const
 {
     foreach (const FirmwareIndexEntry &entry, m_firmwareIndex) {

diff --git a/common/zigbeeintegrationplugin.h b/common/zigbeeintegrationplugin.h
@@ -73,7 +73,9 @@ class ZigbeeIntegrationPlugin: public IntegrationPlugin, public ZigbeeHandler
     Thing *thingForNode(ZigbeeNode *node);
     ZigbeeNode *nodeForThing(Thing *thing);
 
-    virtual void createThing(const ThingClassId &thingClassId, ZigbeeNode *node, const ParamList &additionalParams = ParamList());
+    virtual Thing *createThing(const ThingClassId &thingClassId, ZigbeeNode *node, const ParamList &additionalParams = ParamList());
+    virtual void createConnections(Thing *thing) = 0;
+
 
     void bindCluster(ZigbeeNodeEndpoint *endpoint, ZigbeeClusterLibrary::ClusterId clusterId, int retries = 3);
 
@@ -94,6 +96,7 @@ class ZigbeeIntegrationPlugin: public IntegrationPlugin, public ZigbeeHandler
     void configureFanControlInputClusterAttributeReporting(ZigbeeNodeEndpoint *endpoint);
     void configureIasZoneInputClusterAttributeReporting(ZigbeeNodeEndpoint *endpoint);
     void configureWindowCoveringInputClusterLiftPercentageAttributeReporting(ZigbeeNodeEndpoint *endpoint);
+    void configureDoorLockInputClusterAttributeReporting(ZigbeeNodeEndpoint *endpoint);
 
     void connectToPowerConfigurationInputCluster(Thing *thing, ZigbeeNodeEndpoint *endpoint, qreal maxVoltage = 0, qreal minVoltage = 0);
     void connectToThermostatCluster(Thing *thing, ZigbeeNodeEndpoint *endpoint);
@@ -145,6 +148,7 @@ private slots:
     virtual void updateFirmwareIndex();
 
 private:
+    void setupNode(ZigbeeNode *node, Thing *thing);
     FirmwareIndexEntry firmwareInfo(quint16 manufacturerId, quint16 imageType, quint32 fileVersion) const;
     QString firmwareFileName(const FirmwareIndexEntry &info) const;
     FetchFirmwareReply *fetchFirmware(const FirmwareIndexEntry &info);

diff --git a/zigbeedevelco/integrationpluginzigbeedevelco.cpp b/zigbeedevelco/integrationpluginzigbeedevelco.cpp
@@ -173,14 +173,23 @@ void IntegrationPluginZigbeeDevelco::setupThing(ThingSetupInfo *info)
         info->finish(Thing::ThingErrorHardwareNotAvailable);
         return;
     }
+
+    info->finish(Thing::ThingErrorNoError);
+}
+
+void IntegrationPluginZigbeeDevelco::createConnections(Thing *thing)
+{
     ZigbeeNode *node = nodeForThing(thing);
+    if (!node) {
+        qCWarning(dcZigbeeDevelco()) << "Node for thing" << thing << "not found.";
+        return;
+    }
 
     if (thing->thingClassId() == ioModuleThingClassId) {
         // Set the version from the manufacturer specific attribute in base cluster
         ZigbeeNodeEndpoint *primaryEndpoint = node->getEndpoint(IO_MODULE_EP_INPUT1);
         if (!primaryEndpoint) {
             qCWarning(dcZigbeeDevelco()) << "Failed to set up IO module" << thing << ". Could not find endpoint for version parsing.";
-            info->finish(Thing::ThingErrorSetupFailed);
             return;
         }
 
@@ -326,7 +335,6 @@ void IntegrationPluginZigbeeDevelco::setupThing(ThingSetupInfo *info)
         ZigbeeNodeEndpoint *sensorEndpoint = node->getEndpoint(DEVELCO_EP_TEMPERATURE_SENSOR);
         if (!sensorEndpoint) {
             qCWarning(dcZigbeeDevelco()) << "Failed to set up air quality sensor" << thing << ". Could not find endpoint for version parsing.";
-            info->finish(Thing::ThingErrorSetupFailed);
             return;
         }
 
@@ -430,14 +438,13 @@ void IntegrationPluginZigbeeDevelco::setupThing(ThingSetupInfo *info)
         connectToTemperatureMeasurementInputCluster(thing, temperatureSensorEndpoint);
         connectToIlluminanceMeasurementInputCluster(thing, illuminanceSensorEndpoint);
     }
-
-    info->finish(Thing::ThingErrorNoError);
 }
 
 void IntegrationPluginZigbeeDevelco::postSetupThing(Thing *thing)
 {
     if (thing->thingClassId() == ioModuleThingClassId) {
-        if (nodeForThing(thing)->reachable()) {
+        ZigbeeNode *node = nodeForThing(thing);
+        if (node && node->reachable()) {
             readIoModuleOutputPowerStates(thing);
             readIoModuleInputPowerStates(thing);
         }
@@ -453,6 +460,11 @@ void IntegrationPluginZigbeeDevelco::executeAction(ThingActionInfo *info)
 
     Thing *thing = info->thing();
     ZigbeeNode *node = nodeForThing(info->thing());
+    if (!node) {
+        qCWarning(dcZigbeeDevelco()) << "Node for thing" << thing << "not found.";
+        info->finish(Thing::ThingErrorHardwareNotAvailable);
+        return;
+    }
 
     if (thing->thingClassId() == ioModuleThingClassId) {
         // Identify

diff --git a/zigbeedevelco/integrationpluginzigbeedevelco.h b/zigbeedevelco/integrationpluginzigbeedevelco.h
@@ -100,6 +100,8 @@ class IntegrationPluginZigbeeDevelco: public ZigbeeIntegrationPlugin
     void executeAction(ThingActionInfo *info) override;
 
 private:    
+    void createConnections(Thing *thing) override;
+
     QString parseDevelcoVersionString(ZigbeeNodeEndpoint *endpoint);
 
     void initIoModule(ZigbeeNode *node);

diff --git a/zigbeeeurotronic/integrationpluginzigbeeeurotronic.cpp b/zigbeeeurotronic/integrationpluginzigbeeeurotronic.cpp
@@ -79,7 +79,17 @@ void IntegrationPluginZigbeeEurotronic::setupThing(ThingSetupInfo *info)
         return;
     }
 
+    info->finish(Thing::ThingErrorNoError);
+}
+
+void IntegrationPluginZigbeeEurotronic::createConnections(Thing *thing)
+{
     ZigbeeNode *node = nodeForThing(thing);
+    if (!node) {
+        qCWarning(dcZigbeeEurotronic()) << "Node for thing" << thing << "not found.";
+        return;
+    }
+
     ZigbeeNodeEndpoint *endpoint = node->getEndpoint(0x01);
     thing->setStateValue("currentVersion", endpoint->deviceVersion());
 
@@ -89,7 +99,6 @@ void IntegrationPluginZigbeeEurotronic::setupThing(ThingSetupInfo *info)
     ZigbeeClusterThermostat *thermostatCluster = endpoint->inputCluster<ZigbeeClusterThermostat>(ZigbeeClusterLibrary::ClusterIdThermostat);
     if (!thermostatCluster) {
         qCWarning(dcZigbeeEurotronic()) << "Failed to read thermostat cluster";
-        info->finish(Thing::ThingErrorHardwareFailure);
         return;
     }
 
@@ -107,8 +116,6 @@ void IntegrationPluginZigbeeEurotronic::setupThing(ThingSetupInfo *info)
         }
     });
     thermostatCluster->readAttributes({0x4008}, 0x1037);
-
-    info->finish(Thing::ThingErrorNoError);
 }
 
 void IntegrationPluginZigbeeEurotronic::executeAction(ThingActionInfo *info)
@@ -143,6 +150,11 @@ void IntegrationPluginZigbeeEurotronic::sendNextAction()
     }
 
     ZigbeeNode *node = nodeForThing(info->thing());
+    if (!node) {
+        qCWarning(dcZigbeeEurotronic()) << "Node for thing" << info->thing() << "not found.";
+        info->finish(Thing::ThingErrorHardwareNotAvailable, QT_TR_NOOP("ZigBee node not found in network."));
+        return;
+    }
     ZigbeeNodeEndpoint *endpoint = node->getEndpoint(0x01);
 
     ZigbeeClusterThermostat *thermostatCluster = endpoint->inputCluster<ZigbeeClusterThermostat>(ZigbeeClusterLibrary::ClusterIdThermostat);

diff --git a/zigbeeeurotronic/integrationpluginzigbeeeurotronic.h b/zigbeeeurotronic/integrationpluginzigbeeeurotronic.h
@@ -64,6 +64,8 @@ class IntegrationPluginZigbeeEurotronic: public ZigbeeIntegrationPlugin
     void executeAction(ThingActionInfo *info) override;
 
 private:
+    void createConnections(Thing *thing) override;
+
     void sendNextAction();
 
 private: