Improve permit join handling

- Prevent closing the network when opened by another client - Code simplification of logic - Send close network broadcast only once
dresden-elektronik · Sep 6, 2021 · a432c05 · a432c05
1 parent 6c8811f
commit a432c05
Show file tree

Hide file tree

Showing 9 changed files with 88 additions and 115 deletions.
diff --git a/de_web_plugin_private.h b/de_web_plugin_private.h
@@ -93,7 +93,7 @@ using namespace deCONZ::literals;
 
 #define MAX_UNLOCK_GATEWAY_TIME 600
 #define MAX_RECOVER_ENTRY_AGE 600
-#define PERMIT_JOIN_SEND_INTERVAL (1000 * 1800)
+#define PERMIT_JOIN_SEND_INTERVAL (1000 * 60)
 #define SET_ENDPOINTCONFIG_DURATION (1000 * 16) // time deCONZ needs to update Endpoints
 #define OTA_LOW_PRIORITY_TIME (60 * 2)
 #define CHECK_SENSOR_FAST_ROUNDS 3
@@ -1245,7 +1245,7 @@ class DeRestPluginPrivate : public QObject
     bool setInternetDiscoveryInterval(int minutes);
     // Permit join
     void initPermitJoin();
-    bool setPermitJoinDuration(uint8_t duration);
+    void setPermitJoinDuration(int duration);
 
     // Otau
     void initOtau();
@@ -1293,8 +1293,8 @@ public Q_SLOTS:
     void inetProxyHostLookupDone(const QHostInfo &host);
     void inetProxyCheckHttpVia(const QString &via);
     void scheduleTimerFired();
+    void permitJoin(int seconds);
     void permitJoinTimerFired();
-    void resendPermitJoinTimerFired();
     void otauTimerFired();
     void lockGatewayTimerFired();
     void openClientTimerFired();
@@ -1714,8 +1714,8 @@ public Q_SLOTS:
     int gwAnnounceInterval; // used by internet discovery [minutes]
     QString gwAnnounceUrl;
     int gwAnnounceVital; // 0 not tried, > 0 success attemps, < 0 failed attemps
-    uint8_t gwPermitJoinDuration; // global permit join state (last set)
-    int gwPermitJoinResend; // permit join of values > 255
+    int gwPermitJoinDuration = 0; // global permit join state (last set)
+    QString permitJoinApiKey;
     uint16_t gwNetworkOpenDuration; // user setting how long network remains open
     QString gwWifi;     // configured | not-configured | not-available | new-configured | deactivated
     QString gwWifiActive;
@@ -1856,11 +1856,9 @@ public Q_SLOTS:
     QTimer *lockGatewayTimer;
 
     // permit join
-    // used by searchLights()
     QTimer *permitJoinTimer;
-    QTime permitJoinLastSendTime;
+    QElapsedTimer permitJoinLastSendTime;
     bool permitJoinFlag; // indicates that permitJoin changed from greater than 0 to 0
-    QTimer *resendPermitJoinTimer;
 
     // schedules
     QTimer *scheduleTimer;

diff --git a/permitJoin.cpp b/permitJoin.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016-2020 dresden elektronik ingenieurtechnik gmbh.
+ * Copyright (c) 2016-2021 dresden elektronik ingenieurtechnik gmbh.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -9,6 +9,7 @@
  */
 #include "de_web_plugin.h"
 #include "de_web_plugin_private.h"
+#include "zdp/zdp.h"
 
 /*! Inits permit join manager.
 
@@ -22,32 +23,24 @@ void DeRestPluginPrivate::initPermitJoin()
     connect(permitJoinTimer, SIGNAL(timeout()),
             this, SLOT(permitJoinTimerFired()));
     permitJoinTimer->start(1000);
-    permitJoinLastSendTime = QTime::currentTime();
-
-    resendPermitJoinTimer = new QTimer(this);
-    resendPermitJoinTimer->setSingleShot(true);
-    connect(resendPermitJoinTimer, SIGNAL(timeout()),
-            this, SLOT(resendPermitJoinTimerFired()));
 }
 
 /*! Sets the permit join interval
 
     \param duration specifies the interval in which joining is enabled
                - 0 disabled
-               - 1..254 duration in seconds until joining will be disabled
-               - 255 always permit
- * \return
+               - >0 duration in seconds until joining will be disabled
+
  */
-bool DeRestPluginPrivate::setPermitJoinDuration(uint8_t duration)
+void DeRestPluginPrivate::setPermitJoinDuration(int duration)
 {
     if (gwPermitJoinDuration != duration)
     {
         gwPermitJoinDuration = duration;
     }
 
     // force resend
-    permitJoinLastSendTime = QTime();
-    return true;
+    permitJoinLastSendTime.invalidate();
 }
 
 /*! Handle broadcasting of permit join interval.
@@ -63,16 +56,25 @@ void DeRestPluginPrivate::permitJoinTimerFired()
         return;
     }
 
-    if ((gwPermitJoinDuration > 0) && (gwPermitJoinDuration < 255))
+    if (gwPermitJoinDuration > 0)
     {
+        gwPermitJoinDuration--;
+
         if (!permitJoinFlag)
         {
             permitJoinFlag = true;
-            enqueueEvent(Event(RConfig, REventPermitjoinEnabled, 0));
+            enqueueEvent(Event(RConfig, REventPermitjoinEnabled, gwPermitJoinDuration));
+        }
+        else
+        {
+            enqueueEvent(Event(RConfig, REventPermitjoinRunning, gwPermitJoinDuration));
         }
-        gwPermitJoinDuration--;
 
-        if ((gwPermitJoinDuration % 10) == 0)
+        if (DEV_TestManaged())
+        {
+
+        }
+        else if ((gwPermitJoinDuration % 10) == 0) // TODO bad this needs to go
         {
             // try to add light nodes even if they existed in deCONZ bevor and therefore
             // no node added event will be triggert in this phase
@@ -88,7 +90,7 @@ void DeRestPluginPrivate::permitJoinTimerFired()
                 i++;
             }
         }
-        else if ((gwPermitJoinDuration % 15) == 0)
+        else if ((gwPermitJoinDuration % 15) == 0) // TODO bad this needs to go
         {
             for (LightNode &l : nodes)
             {
@@ -102,12 +104,6 @@ void DeRestPluginPrivate::permitJoinTimerFired()
         updateEtag(gwConfigEtag); // update Etag so that webApp can count down permitJoin duration
     }
 
-    if (gwPermitJoinDuration == 0 && permitJoinFlag)
-    {
-        permitJoinFlag = false;
-        enqueueEvent(Event(RConfig, REventPermitjoinDisabled, 0));
-    }
-
     if (!isInNetwork())
     {
         return;
@@ -118,19 +114,14 @@ void DeRestPluginPrivate::permitJoinTimerFired()
     {
         // workaround since the firmware reports cached value instead hot value
         apsCtrl->setPermitJoin(gwPermitJoinDuration);
-        permitJoinLastSendTime = {}; // force broadcast
+        permitJoinLastSendTime.invalidate(); // force broadcast
     }
 
-//    if (gwPermitJoinDuration == 0 && otauLastBusyTimeDelta() < (60 * 5))
-//    {
-//        // don't pollute channel while OTA is running
-//        return;
-//    }
-
-    QTime now = QTime::currentTime();
-    int diff = permitJoinLastSendTime.msecsTo(now);
+    if (!permitJoinFlag)
+    {
 
-    if (!permitJoinLastSendTime.isValid() || ((diff > PERMIT_JOIN_SEND_INTERVAL) && !gwdisablePermitJoinAutoOff))
+    }
+    else if (!permitJoinLastSendTime.isValid() || (permitJoinLastSendTime.elapsed() > PERMIT_JOIN_SEND_INTERVAL && !gwdisablePermitJoinAutoOff))
     {
         deCONZ::ApsDataRequest apsReq;
         quint8 tcSignificance = 0x01;
@@ -147,25 +138,23 @@ void DeRestPluginPrivate::permitJoinTimerFired()
         QDataStream stream(&apsReq.asdu(), QIODevice::WriteOnly);
         stream.setByteOrder(QDataStream::LittleEndian);
 
-        stream << (uint8_t)now.second(); // seqno
-        stream << gwPermitJoinDuration;
-        stream << tcSignificance;
+        static_assert (PERMIT_JOIN_SEND_INTERVAL / 1000 < 180, "permit join send interval < 180 seconds");
+        static_assert (PERMIT_JOIN_SEND_INTERVAL / 1000 > 30, "permit join send interval > 30 seconds");
 
-        DBG_Assert(apsCtrl != 0);
+        int duration = qMin(gwPermitJoinDuration, (PERMIT_JOIN_SEND_INTERVAL / 1000) + 5);
 
-        if (!apsCtrl)
-        {
-            return;
-        }
+        stream << ZDP_NextSequenceNumber();
+        stream << static_cast<quint8>(duration);
+        stream << tcSignificance;
 
         // set for own node
-        apsCtrl->setPermitJoin(gwPermitJoinDuration);
+        apsCtrl->setPermitJoin(duration);
 
         // broadcast
         if (apsCtrl->apsdeDataRequest(apsReq) == deCONZ::Success)
         {
-            DBG_Printf(DBG_INFO, "send permit join, duration: %d\n", gwPermitJoinDuration);
-            permitJoinLastSendTime = now;
+            DBG_Printf(DBG_INFO, "send permit join, duration: %d\n", duration);
+            permitJoinLastSendTime.restart();
 
             if (gwPermitJoinDuration > 0)
             {
@@ -176,46 +165,28 @@ void DeRestPluginPrivate::permitJoinTimerFired()
         {
             DBG_Printf(DBG_INFO, "send permit join failed\n");
         }
+    }
 
+    if (gwPermitJoinDuration == 0 && permitJoinFlag)
+    {
+        permitJoinApiKey.clear();
+        permitJoinFlag = false;
+        enqueueEvent(Event(RConfig, REventPermitjoinDisabled, 0));
     }
 }
 
-/*! Check if permitJoin is > 60 seconds then resend permitjoin with 60 seconds
- */
-void DeRestPluginPrivate::resendPermitJoinTimerFired()
+void DeRestPluginPrivate::permitJoin(int seconds)
 {
-    resendPermitJoinTimer->stop();
-    if (gwPermitJoinDuration <= 1)
+    if (seconds > 0)
     {
-        if (gwPermitJoinResend > 0)
-        {
-            if (gwPermitJoinResend >= 60)
-            {
-                setPermitJoinDuration(60);
-            }
-            else
-            {
-                setPermitJoinDuration(gwPermitJoinResend);
-            }
-            gwPermitJoinResend -= 60;
-            updateEtag(gwConfigEtag);
-            if (gwPermitJoinResend <= 0)
-            {
-                gwPermitJoinResend = 0;
-                return;
-            }
-
-        }
-        else if (gwPermitJoinResend == 0)
-        {
-            setPermitJoinDuration(0);
-            return;
-        }
+        int tmp = gwNetworkOpenDuration; // preserve configured duration
+        gwNetworkOpenDuration = seconds;
+        startSearchSensors();
+        startSearchLights();
+        gwNetworkOpenDuration = tmp;
     }
-    else if (gwPermitJoinResend == 0)
+    else
     {
-        setPermitJoinDuration(0);
-        return;
+        gwPermitJoinDuration = 0;
     }
-    resendPermitJoinTimer->start(1000);
 }
diff --git a/resource.cpp b/resource.cpp
@@ -31,6 +31,7 @@ const char *REventDeviceAlarm = "event/devicealarm";
 const char *REventDeviceAnnounce = "event/device.anounce";
 const char *REventPermitjoinEnabled = "event/permit.join.enabled";
 const char *REventPermitjoinDisabled = "event/permit.join.disabled";
+const char *REventPermitjoinRunning = "event/permit.join.running";
 const char *REventPoll = "event/poll";
 const char *REventDDFReload = "event/ddf.reload";
 const char *REventDDFInitRequest = "event/ddf.init.req";

diff --git a/resource.h b/resource.h
@@ -59,6 +59,7 @@ extern const char *REventDeviceAlarm;
 extern const char *REventDeviceAnnounce;
 extern const char *REventPermitjoinEnabled;
 extern const char *REventPermitjoinDisabled;
+extern const char *REventPermitjoinRunning;
 extern const char *REventPoll;
 extern const char *REventDDFReload;
 extern const char *REventDDFInitRequest;

diff --git a/rest_configuration.cpp b/rest_configuration.cpp
@@ -52,7 +52,6 @@ void DeRestPluginPrivate::initConfig()
     gwRfConnected = false; // will be detected later
     gwRfConnectedExpected = (deCONZ::appArgumentNumeric("--auto-connect", 1) == 1) ? true : false;
     gwPermitJoinDuration = 0;
-    gwPermitJoinResend = 0;
     gwNetworkOpenDuration = 60;
     gwWifiState = WifiStateInitMgmt;
     gwWifiMgmt = 0;
@@ -924,8 +923,8 @@ void DeRestPluginPrivate::configToMap(const ApiRequest &req, QVariantMap &map)
 
     if (req.apiVersion() >= ApiVersion_1_DDEL)
     {
-        map["permitjoin"] = static_cast<double>(gwPermitJoinDuration);
-        map["permitjoinfull"] = static_cast<double>(gwPermitJoinResend);
+        map["permitjoin"] = static_cast<double>(qMin(gwPermitJoinDuration, 254));
+        map["permitjoinfull"] = static_cast<double>(gwPermitJoinDuration);
         map["otauactive"] = isOtauActive();
         map["otaustate"] = (isOtauBusy() ? "busy" : (isOtauActive() ? "idle" : "off"));
         map["groupdelay"] = static_cast<double>(gwGroupSendDelay);
@@ -1715,9 +1714,19 @@ int DeRestPluginPrivate::modifyConfig(const ApiRequest &req, ApiResponse &rsp)
             return REQ_READY_SEND;
         }
 
-        if (gwPermitJoinResend != seconds)
+        if (seconds == 0)
         {
-            gwPermitJoinResend = seconds;
+            // workaround that only initial caller can disable permit join
+            if (req.apikey() == permitJoinApiKey)
+            {
+                gwPermitJoinDuration = 0;
+                changed = true;
+            }
+        }
+        else if (gwPermitJoinDuration != seconds)
+        {
+            permitJoinApiKey = req.apikey();
+            gwPermitJoinDuration = seconds;
             changed = true;
         }
 

diff --git a/rest_lights.cpp b/rest_lights.cpp
@@ -152,15 +152,14 @@ int DeRestPluginPrivate::getAllLights(const ApiRequest &req, ApiResponse &rsp)
  */
 int DeRestPluginPrivate::searchNewLights(const ApiRequest &req, ApiResponse &rsp)
 {
-    Q_UNUSED(req);
-
     if (!isInNetwork())
     {
         rsp.list.append(errorToMap(ERR_NOT_CONNECTED, QLatin1String("/lights"), QLatin1String("Not connected")));
         rsp.httpStatus = HttpStatusServiceUnavailable;
         return REQ_READY_SEND;
     }
 
+    permitJoinApiKey = req.apikey();
     startSearchLights();
     {
         QVariantMap rspItem;
@@ -3161,23 +3160,16 @@ void DeRestPluginPrivate::startSearchLights()
     }
 
     searchLightsTimeout = gwNetworkOpenDuration;
-    gwPermitJoinResend = searchLightsTimeout;
-    if (!resendPermitJoinTimer->isActive())
-    {
-        resendPermitJoinTimer->start(100);
-    }
+    setPermitJoinDuration(searchLightsTimeout);
 }
 
 /*! Handler for search lights active state.
  */
 void DeRestPluginPrivate::searchLightsTimerFired()
 {
-    if (gwPermitJoinResend == 0)
+    if (gwPermitJoinDuration == 0)
     {
-        if (gwPermitJoinDuration == 0)
-        {
-            searchLightsTimeout = 0; // done
-        }
+        searchLightsTimeout = 0; // done
     }
 
     if (searchLightsTimeout > 0)