From 101b509f51d99c3e82b42db62b026b83b9876c43 Mon Sep 17 00:00:00 2001 From: Manuel Pietschmann Date: Sun, 16 Dec 2018 13:24:03 +0100 Subject: [PATCH] Refactor sensor search processing * Don't send some requests multiple times * Ensure only the selected device is processed * Ensure node descriptor and active endpoints are queried --- de_web_plugin.cpp | 57 ++++++++++++++++------ de_web_plugin_private.h | 6 ++- rest_sensors.cpp | 103 ++++++++++++++++++++++++++-------------- 3 files changed, 113 insertions(+), 53 deletions(-) diff --git a/de_web_plugin.cpp b/de_web_plugin.cpp index 9066aab262..57eb426150 100644 --- a/de_web_plugin.cpp +++ b/de_web_plugin.cpp @@ -11708,14 +11708,18 @@ void DeRestPluginPrivate::delayedFastEnddeviceProbe(const deCONZ::NodeEvent *eve #if DECONZ_LIB_VERSION >= 0x010900 // when macPoll = true core will handle ZDP descriptor queries bool macPoll = event && event->event() == deCONZ::NodeEvent::NodeMacDataRequest; -#else - bool macPoll = false; -#endif - - if (macPoll && fastProbeTimer->isActive()) + if (macPoll) { - fastProbeTimer->stop(); + if (event->node() && event->node()->address().ext() != sc->address.ext()) + { + return; + } + + DBG_Printf(DBG_INFO, "MAC poll fastEnddeviceProbe() 0x%016llX\n", macPoll, sc->address.ext()); } +#else +// bool macPoll = false; +#endif { Sensor *sensor = getSensorNodeForAddress(sc->address); @@ -11748,7 +11752,22 @@ void DeRestPluginPrivate::delayedFastEnddeviceProbe(const deCONZ::NodeEvent *eve return; } - if (!macPoll && node->nodeDescriptor().isNull()) + if (sc->timeout.isValid() && sc->timeout.elapsed() < 9000) + { + DBG_Printf(DBG_INFO, "wait response fastEnddeviceProbe() 0x%016llX, elapsed %d ms\n", sc->address.ext(), sc->timeout.elapsed()); + return; + } + + bool hasNodeDescriptor = false; + bool hasActiveEndpoints = false; + + for (auto const &ind : fastProbeIndications) + { + if (ind.clusterId() == ZDP_NODE_DESCRIPTOR_RSP_CLID) { hasNodeDescriptor = true; } + else if (ind.clusterId() == ZDP_ACTIVE_ENDPOINTS_RSP_CLID) { hasActiveEndpoints = true; } + } + + if (!hasNodeDescriptor) { DBG_Printf(DBG_INFO, "[1] get node descriptor for 0x%016llx\n", sc->address.ext()); deCONZ::ApsDataRequest apsReq; @@ -11761,7 +11780,7 @@ void DeRestPluginPrivate::delayedFastEnddeviceProbe(const deCONZ::NodeEvent *eve apsReq.setProfileId(ZDP_PROFILE_ID); apsReq.setRadius(0); apsReq.setClusterId(ZDP_NODE_DESCRIPTOR_CLID); - //apsReq.setTxOptions(deCONZ::ApsTxAcknowledgedTransmission); + apsReq.setTxOptions(deCONZ::ApsTxAcknowledgedTransmission); QDataStream stream(&apsReq.asdu(), QIODevice::WriteOnly); stream.setByteOrder(QDataStream::LittleEndian); @@ -11774,16 +11793,15 @@ void DeRestPluginPrivate::delayedFastEnddeviceProbe(const deCONZ::NodeEvent *eve if (apsCtrl && apsCtrl->apsdeDataRequest(apsReq) == deCONZ::Success) { queryTime = queryTime.addSecs(5); + sc->timeout.restart(); + sc->waitIndicationClusterId = ZDP_NODE_DESCRIPTOR_RSP_CLID; } return; } - if (sc->indClusterId == ZDP_ACTIVE_ENDPOINTS_RSP_CLID) - { - sc->endpoints = node->endpoints(); - } + sc->endpoints = node->endpoints(); - if (!macPoll && sc->endpoints.empty()) + if (!hasActiveEndpoints) { DBG_Printf(DBG_INFO, "[2] get active endpoints for 0x%016llx\n", sc->address.ext()); deCONZ::ApsDataRequest apsReq; @@ -11809,12 +11827,13 @@ void DeRestPluginPrivate::delayedFastEnddeviceProbe(const deCONZ::NodeEvent *eve if (apsCtrl && apsCtrl->apsdeDataRequest(apsReq) == deCONZ::Success) { queryTime = queryTime.addSecs(5); + sc->timeout.restart(); + sc->waitIndicationClusterId = ZDP_ACTIVE_ENDPOINTS_RSP_CLID; } return; } // simple descriptor for endpoint 0x01 - if (!macPoll && node->simpleDescriptors().size() != (int)node->endpoints().size()) { quint8 ep = 0; @@ -11824,9 +11843,11 @@ void DeRestPluginPrivate::delayedFastEnddeviceProbe(const deCONZ::NodeEvent *eve for (int j = 0; j < node->simpleDescriptors().size(); j++) { - if (node->simpleDescriptors()[j].endpoint() == ep) + const deCONZ::SimpleDescriptor &sd = node->simpleDescriptors()[j]; + if (sd.endpoint() == ep && sd.deviceId() != 0xffff) { ep = 0; + break; } } @@ -11857,6 +11878,8 @@ void DeRestPluginPrivate::delayedFastEnddeviceProbe(const deCONZ::NodeEvent *eve if (apsCtrl && apsCtrl->apsdeDataRequest(apsReq) == deCONZ::Success) { queryTime = queryTime.addSecs(1); + sc->timeout.restart(); + sc->waitIndicationClusterId = ZDP_SIMPLE_DESCRIPTOR_RSP_CLID; } return; @@ -11972,6 +11995,8 @@ void DeRestPluginPrivate::delayedFastEnddeviceProbe(const deCONZ::NodeEvent *eve if (apsCtrl && apsCtrl->apsdeDataRequest(apsReq) == deCONZ::Success) { queryTime = queryTime.addSecs(1); + sc->timeout.restart(); + sc->waitIndicationClusterId = apsReq.clusterId(); } return; } @@ -12079,6 +12104,8 @@ void DeRestPluginPrivate::delayedFastEnddeviceProbe(const deCONZ::NodeEvent *eve apsCtrl && apsCtrl->apsdeDataRequest(apsReq) == deCONZ::Success) { queryTime = queryTime.addSecs(1); + sc->timeout.restart(); + sc->waitIndicationClusterId = apsReq.clusterId(); } } else if (!sensor) diff --git a/de_web_plugin_private.h b/de_web_plugin_private.h index 00e3f19539..285c7f5514 100644 --- a/de_web_plugin_private.h +++ b/de_web_plugin_private.h @@ -1526,13 +1526,14 @@ public Q_SLOTS: public: SensorCandidate() : macCapabilities(0), - indClusterId(0) + waitIndicationClusterId(0) { } deCONZ::Address address; quint8 macCapabilities; - quint16 indClusterId; + QTime timeout; + quint16 waitIndicationClusterId; std::vector endpoints; std::vector rxCommands; }; @@ -1544,6 +1545,7 @@ public Q_SLOTS: SearchSensorsState searchSensorsState; deCONZ::Address fastProbeAddr; + std::vector fastProbeIndications; QVariantMap searchSensorsResult; QTimer *fastProbeTimer; int searchSensorsTimeout; diff --git a/rest_sensors.cpp b/rest_sensors.cpp index 7b448f8f14..6250c8a7e7 100644 --- a/rest_sensors.cpp +++ b/rest_sensors.cpp @@ -2222,6 +2222,7 @@ void DeRestPluginPrivate::searchSensorsTimerFired() if (searchSensorsTimeout == 0) { fastProbeAddr = deCONZ::Address(); + fastProbeIndications.clear(); searchSensorsState = SearchSensorsDone; } } @@ -2312,10 +2313,10 @@ void DeRestPluginPrivate::checkInstaModelId(Sensor *sensor) */ void DeRestPluginPrivate::handleIndicationSearchSensors(const deCONZ::ApsDataIndication &ind, deCONZ::ZclFrame &zclFrame) { -// if (searchSensorsState != SearchSensorsActive) -// { -// return; -// } + if (searchSensorsState != SearchSensorsActive) + { + return; + } if (ind.profileId() == ZDP_PROFILE_ID && ind.clusterId() == ZDP_DEVICE_ANNCE_CLID) { @@ -2355,14 +2356,22 @@ void DeRestPluginPrivate::handleIndicationSearchSensors(const deCONZ::ApsDataInd return; } + if (fastProbeAddr.hasExt() && fastProbeAddr.ext() != ext) + { + return; + } + DBG_Printf(DBG_INFO, "set fast probe address to 0x%016llX (0x%04X)\n", ext, nwk); fastProbeAddr.setExt(ext); fastProbeAddr.setNwk(nwk); if (!fastProbeTimer->isActive()) { - fastProbeTimer->start(1000); + fastProbeTimer->start(100); } + + fastProbeIndications.push_back(ind); + std::vector::iterator i = searchSensorsCandidates.begin(); std::vector::iterator end = searchSensorsCandidates.end(); @@ -2370,13 +2379,15 @@ void DeRestPluginPrivate::handleIndicationSearchSensors(const deCONZ::ApsDataInd { if (i->address.ext() == ext || i->address.nwk() == nwk) { + i->waitIndicationClusterId = 0xffff; + i->timeout = QTime(); i->address = fastProbeAddr; // nwk might have changed return; } } SensorCandidate sc; - sc.indClusterId = ind.clusterId(); + sc.waitIndicationClusterId = 0xffff; sc.address.setExt(ext); sc.address.setNwk(nwk); sc.macCapabilities = macCapabilities; @@ -2385,6 +2396,24 @@ void DeRestPluginPrivate::handleIndicationSearchSensors(const deCONZ::ApsDataInd } else if (ind.profileId() == ZDP_PROFILE_ID) { + if (ind.clusterId() == ZDP_MATCH_DESCRIPTOR_CLID) + { + return; + } + + if (!fastProbeAddr.hasExt()) + { + return; + } + + if (ind.srcAddress().hasExt() && fastProbeAddr.ext() != ind.srcAddress().ext()) + { + return; + } + else if (ind.srcAddress().hasNwk() && fastProbeAddr.nwk() != ind.srcAddress().nwk()) + { + return; + } std::vector::iterator i = searchSensorsCandidates.begin(); std::vector::iterator end = searchSensorsCandidates.end(); @@ -2393,12 +2422,22 @@ void DeRestPluginPrivate::handleIndicationSearchSensors(const deCONZ::ApsDataInd { if (i->address.ext() == fastProbeAddr.ext()) { - i->indClusterId = ind.clusterId(); + DBG_Printf(DBG_INFO, "ZDP indication search sensors 0x%016llX (0x%04X) cluster 0x%04X\n", ind.srcAddress().ext(), ind.srcAddress().nwk(), ind.clusterId()); - if (!fastProbeTimer->isActive()) + if (ind.clusterId() == i->waitIndicationClusterId && i->timeout.isValid()) { - fastProbeTimer->start(100); + i->timeout = QTime(); + i->waitIndicationClusterId = 0xffff; + } + + if (ind.clusterId() & 0x8000) + { + fastProbeIndications.push_back(ind); // remember responses } + + fastProbeTimer->stop(); + fastProbeTimer->start(5); + break; } } return; @@ -2428,26 +2467,10 @@ void DeRestPluginPrivate::handleIndicationSearchSensors(const deCONZ::ApsDataInd return; } - if (zclFrame.commandId() != deCONZ::ZclReadAttributesResponseId) + if (zclFrame.commandId() != deCONZ::ZclReadAttributesResponseId && zclFrame.commandId() != deCONZ::ZclReportAttributesId) { return; } - else if (fastProbeAddr.hasExt()) - { - std::vector::const_iterator i = searchSensorsCandidates.begin(); - std::vector::const_iterator end = searchSensorsCandidates.end(); - - for (; i != end; ++i) - { - if (i->address.ext() == fastProbeAddr.ext()) - { - if (!fastProbeTimer->isActive()) - { - fastProbeTimer->start(5); - } - } - } - } break; // ok case IAS_ZONE_CLUSTER_ID: @@ -2467,7 +2490,7 @@ void DeRestPluginPrivate::handleIndicationSearchSensors(const deCONZ::ApsDataInd return; } - SensorCandidate *sc = 0; + SensorCandidate *sc = nullptr; { std::vector::iterator i = searchSensorsCandidates.begin(); std::vector::iterator end = searchSensorsCandidates.end(); @@ -2488,6 +2511,23 @@ void DeRestPluginPrivate::handleIndicationSearchSensors(const deCONZ::ApsDataInd } } + if (sc && fastProbeAddr.hasExt() && sc->address.ext() == fastProbeAddr.ext()) + { + if (!fastProbeTimer->isActive()) + { + fastProbeTimer->start(5); + } + + if (ind.profileId() == ZLL_PROFILE_ID || ind.profileId() == HA_PROFILE_ID) + { + if (ind.clusterId() == sc->waitIndicationClusterId && sc->timeout.isValid()) + { + sc->timeout = QTime(); + sc->waitIndicationClusterId = 0xffff; + } + } + } + quint8 macCapabilities = 0; deCONZ::Address indAddress; if (!sc) @@ -2546,15 +2586,6 @@ void DeRestPluginPrivate::handleIndicationSearchSensors(const deCONZ::ApsDataInd sc2.macCapabilities = macCapabilities; searchSensorsCandidates.push_back(sc2); sc = &searchSensorsCandidates.back(); - - if (!fastProbeAddr.hasExt() && searchSensorsState == SearchSensorsActive) - { - fastProbeAddr = indAddress; - if (!fastProbeTimer->isActive()) - { - fastProbeTimer->start(1000); - } - } } if (!sc) // we need a valid candidate from device announce or cache