Rework Thread attachement event and unify implementation (#19474)

* Rework Thread attachement event and unify the event to the genericThreadStackManager * replace 0 by false * Restyled by clang-format Co-authored-by: Restyled.io <[email protected]>
project-chip · Jun 11, 2022 · fdab0fd · fdab0fd
1 parent 9fbc5a5
commit fdab0fd
Show file tree

Hide file tree

Showing 7 changed files with 26 additions and 87 deletions.
diff --git a/src/platform/ESP32/ThreadStackManagerImpl.cpp b/src/platform/ESP32/ThreadStackManagerImpl.cpp
@@ -91,24 +91,5 @@ void ThreadStackManagerImpl::_OnCHIPoBLEAdvertisingStop()
     // Intentionally empty.
 }
 
-void ThreadStackManagerImpl::_OnPlatformEvent(const ChipDeviceEvent * event)
-{
-    Internal::GenericThreadStackManagerImpl_OpenThread<ThreadStackManagerImpl>::_OnPlatformEvent(event);
-
-    if (event->Type == DeviceEventType::kThreadStateChange && event->ThreadStateChange.RoleChanged)
-    {
-        const bool isAttached = IsThreadAttached();
-        VerifyOrReturn(isAttached != mIsAttached);
-        ChipDeviceEvent attachEvent;
-        attachEvent.Type                            = DeviceEventType::kThreadConnectivityChange;
-        attachEvent.ThreadConnectivityChange.Result = isAttached ? kConnectivity_Established : kConnectivity_Lost;
-
-        CHIP_ERROR error = PlatformMgr().PostEvent(&attachEvent);
-        VerifyOrReturn(error == CHIP_NO_ERROR,
-                       ChipLogError(DeviceLayer, "Failed to post Thread connectivity change: %" CHIP_ERROR_FORMAT, error.Format()));
-        mIsAttached = isAttached;
-    }
-}
-
 } // namespace DeviceLayer
 } // namespace chip
diff --git a/src/platform/ESP32/ThreadStackManagerImpl.h b/src/platform/ESP32/ThreadStackManagerImpl.h
@@ -67,15 +67,12 @@ class ThreadStackManagerImpl final : public ThreadStackManager,
     void _ProcessThreadActivity();
     void _OnCHIPoBLEAdvertisingStart();
     void _OnCHIPoBLEAdvertisingStop();
-    void _OnPlatformEvent(const ChipDeviceEvent * event);
 
 private:
     friend ThreadStackManager & ::chip::DeviceLayer::ThreadStackMgr(void);
     friend ThreadStackManagerImpl & ::chip::DeviceLayer::ThreadStackMgrImpl(void);
     static ThreadStackManagerImpl sInstance;
     ThreadStackManagerImpl() = default;
-
-    bool mIsAttached = false;
 };
 
 /**

diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp
@@ -200,41 +200,38 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::_OnPlatformEvent(const
             }
         }
 #endif
-        Impl()->LockThreadStack();
 
-#if CHIP_DETAIL_LOGGING
-
-        LogOpenThreadStateChange(mOTInst, event->ThreadStateChange.OpenThread.Flags);
-
-#endif // CHIP_DETAIL_LOGGING
-
-#if CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT
-        if (event->ThreadStateChange.RoleChanged || event->ThreadStateChange.AddressChanged)
+        bool isThreadAttached = Impl()->_IsThreadAttached();
+        // Avoid sending muliple events if the attachement state didn't change (Child->router or disable->Detached)
+        if (event->ThreadStateChange.RoleChanged && (isThreadAttached != mIsAttached))
         {
-            bool isInterfaceUp;
-            isInterfaceUp = GenericThreadStackManagerImpl_OpenThread<ImplClass>::IsThreadInterfaceUpNoLock();
-            // Post an event signaling the change in Thread interface connectivity state.
+            ChipDeviceEvent attachEvent;
+            attachEvent.Clear();
+            attachEvent.Type                            = DeviceEventType::kThreadConnectivityChange;
+            attachEvent.ThreadConnectivityChange.Result = (isThreadAttached) ? kConnectivity_Established : kConnectivity_Lost;
+            CHIP_ERROR status                           = PlatformMgr().PostEvent(&attachEvent);
+            if (status == CHIP_NO_ERROR)
             {
-                ChipDeviceEvent event;
-                event.Clear();
-                event.Type                            = DeviceEventType::kThreadConnectivityChange;
-                event.ThreadConnectivityChange.Result = (isInterfaceUp) ? kConnectivity_Established : kConnectivity_Lost;
-                CHIP_ERROR status                     = PlatformMgr().PostEvent(&event);
-                if (status != CHIP_NO_ERROR)
-                {
-                    ChipLogError(DeviceLayer, "Failed to post Thread connectivity change: %" CHIP_ERROR_FORMAT, status.Format());
-                }
+                mIsAttached = isThreadAttached;
             }
-
-            // Refresh Multicast listening
-            if (GenericThreadStackManagerImpl_OpenThread<ImplClass>::IsThreadAttachedNoLock())
+            else
             {
-                ChipLogDetail(DeviceLayer, "Thread Attached updating Multicast address");
-                Server::GetInstance().RejoinExistingMulticastGroups();
+                ChipLogError(DeviceLayer, "Failed to post Thread connectivity change: %" CHIP_ERROR_FORMAT, status.Format());
             }
         }
+
+#if CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT
+        if (event->ThreadStateChange.AddressChanged && isThreadAttached)
+        {
+            // Refresh Multicast listening
+            ChipLogDetail(DeviceLayer, "Thread Attached updating Multicast address");
+            Server::GetInstance().RejoinExistingMulticastGroups();
+        }
 #endif // CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT
-        Impl()->UnlockThreadStack();
+
+#if CHIP_DETAIL_LOGGING
+        LogOpenThreadStateChange(mOTInst, event->ThreadStateChange.OpenThread.Flags);
+#endif // CHIP_DETAIL_LOGGING
     }
 }
 

diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h
@@ -151,6 +151,7 @@ class GenericThreadStackManagerImpl_OpenThread
 
     otInstance * mOTInst;
     uint64_t mOverrunCount = 0;
+    bool mIsAttached       = false;
 
     NetworkCommissioning::ThreadDriver::ScanCallback * mpScanCallback;
     NetworkCommissioning::Internal::WirelessDriver::ConnectCallback * mpConnectCallback;

diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread_LwIP.cpp b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread_LwIP.cpp
@@ -134,7 +134,7 @@ void GenericThreadStackManagerImpl_OpenThread_LwIP<ImplClass>::UpdateThreadInter
     LOCK_TCPIP_CORE();
     Impl()->LockThreadStack();
 
-    // Determine whether the device is attached to a Thread network.
+    // Determine whether the device Thread interface is up..
     isInterfaceUp = GenericThreadStackManagerImpl_OpenThread<ImplClass>::IsThreadInterfaceUpNoLock();
 
     // If needed, adjust the link state of the LwIP netif to reflect the state of the OpenThread stack.
@@ -152,19 +152,6 @@ void GenericThreadStackManagerImpl_OpenThread_LwIP<ImplClass>::UpdateThreadInter
             netif_set_link_down(mNetIf);
         }
 
-        // Post an event signaling the change in Thread interface connectivity state.
-        {
-            ChipDeviceEvent event;
-            event.Clear();
-            event.Type                            = DeviceEventType::kThreadConnectivityChange;
-            event.ThreadConnectivityChange.Result = (isInterfaceUp) ? kConnectivity_Established : kConnectivity_Lost;
-            CHIP_ERROR status                     = PlatformMgr().PostEvent(&event);
-            if (status != CHIP_NO_ERROR)
-            {
-                ChipLogError(DeviceLayer, "Failed to post Thread connectivity change: %" CHIP_ERROR_FORMAT, status.Format());
-            }
-        }
-
         // Presume the interface addresses are also changing.
         addrChange = true;
     }

diff --git a/src/platform/Zephyr/ThreadStackManagerImpl.cpp b/src/platform/Zephyr/ThreadStackManagerImpl.cpp
@@ -77,26 +77,5 @@ void ThreadStackManagerImpl::_UnlockThreadStack()
     openthread_api_mutex_unlock(openthread_get_default_context());
 }
 
-void ThreadStackManagerImpl::_OnPlatformEvent(const ChipDeviceEvent * event)
-{
-    Internal::GenericThreadStackManagerImpl_OpenThread<ThreadStackManagerImpl>::_OnPlatformEvent(event);
-
-    if (event->Type == DeviceEventType::kThreadStateChange && event->ThreadStateChange.RoleChanged)
-    {
-        const bool isAttached = IsThreadAttached();
-        VerifyOrReturn(isAttached != mIsAttached);
-
-        ChipDeviceEvent attachEvent;
-        attachEvent.Type                            = DeviceEventType::kThreadConnectivityChange;
-        attachEvent.ThreadConnectivityChange.Result = isAttached ? kConnectivity_Established : kConnectivity_Lost;
-
-        CHIP_ERROR error = PlatformMgr().PostEvent(&attachEvent);
-        VerifyOrReturn(error == CHIP_NO_ERROR,
-                       ChipLogError(DeviceLayer, "Failed to post Thread connectivity change: %" CHIP_ERROR_FORMAT, error.Format()));
-
-        mIsAttached = isAttached;
-    }
-}
-
 } // namespace DeviceLayer
 } // namespace chip
diff --git a/src/platform/Zephyr/ThreadStackManagerImpl.h b/src/platform/Zephyr/ThreadStackManagerImpl.h
@@ -74,7 +74,6 @@ class ThreadStackManagerImpl final : public ThreadStackManager,
     // ===== Methods that override the GenericThreadStackManagerImpl_OpenThread abstract interface.
 
     void _ProcessThreadActivity() {}
-    void _OnPlatformEvent(const ChipDeviceEvent * event);
 
     //} // namespace Internal
 
@@ -87,8 +86,6 @@ class ThreadStackManagerImpl final : public ThreadStackManager,
     static ThreadStackManagerImpl sInstance;
 
     // ===== Private members for use by this class only.
-
-    bool mIsAttached = false;
 };
 
 /**