Pull up LayerLwIP::ScheduleLambda to SystemLayer (#11361)

* Pull up LayerLwIP::ScheduleLambda to SystemLayer (#11187)

* Fix ScheduleLambda and Add unit-tests

src/include/platform/CHIPDeviceEvent.h

@@ -314,10 +314,7 @@ typedef void (*AsyncWorkFunct)(intptr_t arg);
 
 #include <ble/BleConfig.h>
 #include <inet/InetLayer.h>
-#include <system/SystemEvent.h>
-#include <system/SystemLayer.h>
-#include <system/SystemObject.h>
-#include <system/SystemPacketBuffer.h>
+#include <lib/support/LambdaBridge.h>
 
 namespace chip {
 namespace DeviceLayer {
@@ -332,7 +329,7 @@ struct ChipDeviceEvent final
     union
     {
         ChipDevicePlatformEvent Platform;
-        System::LambdaBridge LambdaEvent;
+        LambdaBridge LambdaEvent;
         struct
         {
             ::chip::System::EventType Type;

src/include/platform/PlatformManager.h

@@ -25,12 +25,9 @@
 
 #include <platform/CHIPDeviceBuildConfig.h>
 #include <platform/CHIPDeviceEvent.h>
+#include <system/PlatformEventSupport.h>
 #include <system/SystemLayer.h>
 
-#if CHIP_SYSTEM_CONFIG_USE_LWIP
-#include <system/LwIPEventSupport.h>
-#endif // CHIP_SYSTEM_CONFIG_USE_LWIP
-
 namespace chip {
 
 namespace Dnssd {
@@ -225,9 +222,7 @@ class PlatformManager
     friend class Internal::GenericThreadStackManagerImpl_OpenThread_LwIP;
     template <class>
     friend class Internal::GenericConfigurationManagerImpl;
-#if CHIP_SYSTEM_CONFIG_USE_LWIP
     friend class System::PlatformEventing;
-#endif // CHIP_SYSTEM_CONFIG_USE_LWIP
 
     /*
      * PostEvent can be called safely on any thread without locking the stack.

src/include/platform/internal/GenericPlatformManagerImpl.cpp

@@ -227,7 +227,7 @@ void GenericPlatformManagerImpl<ImplClass>::_DispatchEvent(const ChipDeviceEvent
         break;
 
     case DeviceEventType::kChipLambdaEvent:
-        event->LambdaEvent.LambdaProxy(static_cast<const void *>(event->LambdaEvent.LambdaBody));
+        event->LambdaEvent();
         break;
 
     case DeviceEventType::kCallWorkFunct:

src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.cpp

@@ -71,6 +71,8 @@ CHIP_ERROR GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_InitChipStack(void)
         ExitNow(err = CHIP_ERROR_NO_MEMORY);
     }
 
+    mShouldRunEventLoop.store(false);
+
     // Call up to the base class _InitChipStack() to perform the bulk of the initialization.
     err = GenericPlatformManagerImpl<ImplClass>::_InitChipStack();
     SuccessOrExit(err);
@@ -119,12 +121,17 @@ void GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_RunEventLoop(void)
     CHIP_ERROR err;
     ChipDeviceEvent event;
 
-    VerifyOrDie(mEventLoopTask != NULL);
-
     // Lock the CHIP stack.
     Impl()->LockChipStack();
 
-    while (true)
+    bool oldShouldRunEventLoop = false;
+    if (!mShouldRunEventLoop.compare_exchange_strong(oldShouldRunEventLoop /* expected */, true /* desired */))
+    {
+        ChipLogError(DeviceLayer, "Error trying to run the event loop while it is already running");
+        return;
+    }
+
+    while (mShouldRunEventLoop.load())
     {
         TickType_t waitTime;
 
@@ -251,8 +258,8 @@ CHIP_ERROR GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_Shutdown(void)
 template <class ImplClass>
 CHIP_ERROR GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_StopEventLoopTask(void)
 {
-    VerifyOrDieWithMsg(false, DeviceLayer, "StopEventLoopTask is not implemented");
-    return CHIP_ERROR_NOT_IMPLEMENTED;
+    mShouldRunEventLoop.store(false);
+    return CHIP_NO_ERROR;
 }
 
 } // namespace Internal

src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h

@@ -39,6 +39,8 @@
 #include "task.h"
 #endif
 
+#include <atomic>
+
 namespace chip {
 namespace DeviceLayer {
 namespace Internal {
@@ -92,6 +94,7 @@ class GenericPlatformManagerImpl_FreeRTOS : public GenericPlatformManagerImpl<Im
     StaticQueue_t mEventQueueStruct;
 #endif
 
+    std::atomic<bool> mShouldRunEventLoop;
 #if defined(CHIP_CONFIG_FREERTOS_USE_STATIC_TASK) && CHIP_CONFIG_FREERTOS_USE_STATIC_TASK
     StackType_t mEventLoopStack[CHIP_DEVICE_CONFIG_CHIP_TASK_STACK_SIZE / sizeof(StackType_t)];
     StaticTask_t mventLoopTaskStruct;

src/include/platform/internal/GenericPlatformManagerImpl_Zephyr.cpp

@@ -61,6 +61,8 @@ CHIP_ERROR GenericPlatformManagerImpl_Zephyr<ImplClass>::_InitChipStack(void)
     k_msgq_init(&mChipEventQueue, reinterpret_cast<char *>(&mChipEventRingBuffer), sizeof(ChipDeviceEvent),
                 CHIP_DEVICE_CONFIG_MAX_EVENT_QUEUE_SIZE);
 
+    mShouldRunEventLoop = false;
+
     // Call up to the base class _InitChipStack() to perform the bulk of the initialization.
     err = GenericPlatformManagerImpl<ImplClass>::_InitChipStack();
     SuccessOrExit(err);
@@ -97,8 +99,8 @@ CHIP_ERROR GenericPlatformManagerImpl_Zephyr<ImplClass>::_StartChipTimer(System:
 template <class ImplClass>
 CHIP_ERROR GenericPlatformManagerImpl_Zephyr<ImplClass>::_StopEventLoopTask(void)
 {
-    VerifyOrDieWithMsg(false, DeviceLayer, "StopEventLoopTask is not implemented");
-    return CHIP_ERROR_NOT_IMPLEMENTED;
+    mShouldRunEventLoop = false;
+    return CHIP_NO_ERROR;
 }
 
 template <class ImplClass>
@@ -137,8 +139,15 @@ void GenericPlatformManagerImpl_Zephyr<ImplClass>::_RunEventLoop(void)
 {
     Impl()->LockChipStack();
 
+    if (mShouldRunEventLoop)
+    {
+        ChipLogError(DeviceLayer, "Error trying to run the event loop while it is already running");
+        return;
+    }
+    mShouldRunEventLoop = true;
+
     SystemLayerSocketsLoop().EventLoopBegins();
-    while (true)
+    while (mShouldRunEventLoop)
     {
         SystemLayerSocketsLoop().PrepareEvents();
 

src/include/platform/internal/GenericPlatformManagerImpl_Zephyr.h

@@ -91,6 +91,7 @@ class GenericPlatformManagerImpl_Zephyr : public GenericPlatformManagerImpl<Impl
     void SysProcess();
     void ProcessDeviceEvents();
 
+    volatile bool mShouldRunEventLoop;
     static void EventLoopTaskMain(void * thisPtr, void *, void *);
 };
 

src/lib/support/LambdaBridge.h

@@ -0,0 +1,53 @@
+/*
+ *    Copyright (c) 2021 Project CHIP Authors
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+#pragma once
+
+#include <string.h>
+#include <type_traits>
+
+#include <lib/core/CHIPConfig.h>
+
+namespace chip {
+
+class LambdaBridge
+{
+public:
+    // Use initialize instead of constructor because this class has to be trivial
+    template <typename Lambda>
+    void Initialize(const Lambda & lambda)
+    {
+        // memcpy is used to move the lambda into the event queue, so it must be trivially copyable
+        static_assert(std::is_trivially_copyable<Lambda>::value, "lambda must be trivially copyable");
+        static_assert(sizeof(Lambda) <= CHIP_CONFIG_LAMBDA_EVENT_SIZE, "lambda too large");
+        static_assert(CHIP_CONFIG_LAMBDA_EVENT_ALIGN % alignof(Lambda) == 0, "lambda align too large");
+
+        // Implicit cast a capture-less lambda into a raw function pointer.
+        mLambdaProxy = [](const LambdaStorage & body) { (*reinterpret_cast<const Lambda *>(&body))(); };
+        ::memcpy(&mLambdaBody, &lambda, sizeof(Lambda));
+    }
+
+    void operator()() const { mLambdaProxy(mLambdaBody); }
+
+private:
+    using LambdaStorage = std::aligned_storage_t<CHIP_CONFIG_LAMBDA_EVENT_SIZE, CHIP_CONFIG_LAMBDA_EVENT_ALIGN>;
+    void (*mLambdaProxy)(const LambdaStorage & body);
+    LambdaStorage mLambdaBody;
+};
+
+static_assert(std::is_trivial<LambdaBridge>::value, "LambdaBridge is not trivial");
+
+} // namespace chip

src/platform/BUILD.gn

@@ -284,8 +284,8 @@ if (chip_device_platform != "none") {
       "GeneralUtils.cpp",
       "Globals.cpp",
       "LockTracker.cpp",
-      "LwIPEventSupport.cpp",
       "PersistedStorage.cpp",
+      "PlatformEventSupport.cpp",
       "TestIdentity.cpp",
     ]
 

src/platform/LwIPEventSupport.cpp → src/platform/PlatformEventSupport.cpp

@@ -27,18 +27,16 @@
 
 #include <platform/PlatformManager.h>
 
-#if CHIP_SYSTEM_CONFIG_USE_LWIP
-
 namespace chip {
 namespace System {
 
 using namespace ::chip::DeviceLayer;
 
-CHIP_ERROR PlatformEventing::ScheduleLambdaBridge(System::Layer & aLayer, const LambdaBridge & bridge)
+CHIP_ERROR PlatformEventing::ScheduleLambdaBridge(System::Layer & aLayer, LambdaBridge && bridge)
 {
     ChipDeviceEvent event;
     event.Type        = DeviceEventType::kChipLambdaEvent;
-    event.LambdaEvent = bridge;
+    event.LambdaEvent = std::move(bridge);
 
     return PlatformMgr().PostEvent(&event);
 }
@@ -62,5 +60,3 @@ CHIP_ERROR PlatformEventing::StartTimer(System::Layer & aLayer, System::Clock::T
 
 } // namespace System
 } // namespace chip
-
-#endif // CHIP_SYSTEM_CONFIG_USE_LWIP

src/platform/fake/PlatformManagerImpl.h

@@ -24,6 +24,8 @@
 
 #include <platform/PlatformManager.h>
 
+#include <queue>
+
 namespace chip {
 namespace DeviceLayer {
 
@@ -43,21 +45,62 @@ class PlatformManagerImpl final : public PlatformManager
     // ===== Methods that implement the PlatformManager abstract interface.
 
     CHIP_ERROR _InitChipStack() { return CHIP_NO_ERROR; }
+    CHIP_ERROR _Shutdown() { return CHIP_NO_ERROR; }
 
     CHIP_ERROR _AddEventHandler(EventHandlerFunct handler, intptr_t arg = 0) { return CHIP_ERROR_NOT_IMPLEMENTED; }
     void _RemoveEventHandler(EventHandlerFunct handler, intptr_t arg = 0) {}
     void _ScheduleWork(AsyncWorkFunct workFunct, intptr_t arg = 0) {}
-    void _RunEventLoop() {}
+
+    void _RunEventLoop()
+    {
+        do
+        {
+            ProcessDeviceEvents();
+        } while (mShouldRunEventLoop);
+    }
+
+    void ProcessDeviceEvents()
+    {
+        while (!mQueue.empty())
+        {
+            const ChipDeviceEvent & event = mQueue.front();
+            _DispatchEvent(&event);
+            mQueue.pop();
+        }
+    }
+
     CHIP_ERROR _StartEventLoopTask() { return CHIP_ERROR_NOT_IMPLEMENTED; }
-    CHIP_ERROR _StopEventLoopTask() { return CHIP_ERROR_NOT_IMPLEMENTED; }
-    CHIP_ERROR _PostEvent(const ChipDeviceEvent * event) { return CHIP_NO_ERROR; }
-    void _DispatchEvent(const ChipDeviceEvent * event) {}
+
+    CHIP_ERROR _StopEventLoopTask()
+    {
+        mShouldRunEventLoop = false;
+        return CHIP_NO_ERROR;
+    }
+
+    CHIP_ERROR _PostEvent(const ChipDeviceEvent * event)
+    {
+        mQueue.emplace(*event);
+        return CHIP_NO_ERROR;
+    }
+
+    void _DispatchEvent(const ChipDeviceEvent * event)
+    {
+        switch (event->Type)
+        {
+        case DeviceEventType::kChipLambdaEvent:
+            event->LambdaEvent();
+            break;
+
+        default:
+            break;
+        }
+    }
+
     CHIP_ERROR _StartChipTimer(System::Clock::Timeout duration) { return CHIP_ERROR_NOT_IMPLEMENTED; }
 
     void _LockChipStack() {}
     bool _TryLockChipStack() { return true; }
     void _UnlockChipStack() {}
-    CHIP_ERROR _Shutdown() { return CHIP_ERROR_NOT_IMPLEMENTED; }
 
     CHIP_ERROR _GetCurrentHeapFree(uint64_t & currentHeapFree);
     CHIP_ERROR _GetCurrentHeapUsed(uint64_t & currentHeapUsed);
@@ -75,6 +118,9 @@ class PlatformManagerImpl final : public PlatformManager
     friend class Internal::BLEManagerImpl;
 
     static PlatformManagerImpl sInstance;
+
+    bool mShouldRunEventLoop = true;
+    std::queue<ChipDeviceEvent> mQueue;
 };
 
 /**

src/system/BUILD.gn

@@ -123,6 +123,7 @@ static_library("system") {
   output_name = "libSystemLayer"
 
   sources = [
+    "PlatformEventSupport.h",
     "SystemAlignSize.h",
     "SystemClock.cpp",
     "SystemClock.h",
@@ -131,6 +132,7 @@ static_library("system") {
     "SystemError.h",
     "SystemEvent.h",
     "SystemFaultInjection.h",
+    "SystemLayer.cpp",
     "SystemLayer.h",
     "SystemLayerImpl${chip_system_config_event_loop}.cpp",
     "SystemLayerImpl${chip_system_config_event_loop}.h",
@@ -169,10 +171,6 @@ static_library("system") {
     }
   }
 
-  if (chip_system_config_use_lwip) {
-    sources += [ "LwIPEventSupport.h" ]
-  }
-
   if (chip_with_nlfaultinjection) {
     sources += [ "SystemFaultInjection.cpp" ]
     public_deps += [ "${nlfaultinjection_root}:nlfaultinjection" ]

src/system/LwIPEventSupport.h → src/system/PlatformEventSupport.h

@@ -29,7 +29,7 @@ class Object;
 class PlatformEventing
 {
 public:
-    static CHIP_ERROR ScheduleLambdaBridge(System::Layer & aLayer, const LambdaBridge & bridge);
+    static CHIP_ERROR ScheduleLambdaBridge(System::Layer & aLayer, LambdaBridge && bridge);
     static CHIP_ERROR PostEvent(System::Layer & aLayer, Object & aTarget, EventType aType, uintptr_t aArgument);
     static CHIP_ERROR StartTimer(System::Layer & aLayer, System::Clock::Timeout aTimeout);
 };