From 6ea8928b3983b11a0a3fd5ee7a39195fe84f9532 Mon Sep 17 00:00:00 2001 From: Zang MingJie Date: Thu, 4 Nov 2021 01:12:21 +0800 Subject: [PATCH] Fix ScheduleLambda and Add unit-tests --- src/include/platform/PlatformManager.h | 8 +- .../GenericPlatformManagerImpl_FreeRTOS.cpp | 12 ++ .../GenericPlatformManagerImpl_FreeRTOS.h | 1 + .../GenericPlatformManagerImpl_POSIX.cpp | 4 +- .../GenericPlatformManagerImpl_POSIX.h | 4 +- .../GenericPlatformManagerImpl_Zephyr.cpp | 4 +- .../GenericPlatformManagerImpl_Zephyr.h | 2 +- src/lib/support/LambdaBridge.h | 12 +- src/platform/Darwin/PlatformManagerImpl.cpp | 5 + src/platform/Darwin/PlatformManagerImpl.h | 1 + src/platform/PlatformEventSupport.cpp | 4 - src/platform/fake/PlatformManagerImpl.h | 56 ++++++++- src/platform/mbed/PlatformManagerImpl.cpp | 4 +- src/platform/mbed/PlatformManagerImpl.h | 3 +- src/system/BUILD.gn | 5 +- src/system/SystemLayer.h | 2 - src/system/tests/BUILD.gn | 1 + src/system/tests/TestSystemScheduleLambda.cpp | 106 ++++++++++++++++++ 18 files changed, 200 insertions(+), 34 deletions(-) create mode 100644 src/system/tests/TestSystemScheduleLambda.cpp diff --git a/src/include/platform/PlatformManager.h b/src/include/platform/PlatformManager.h index 5b8c71190348ab..3fc94b3475ee8c 100644 --- a/src/include/platform/PlatformManager.h +++ b/src/include/platform/PlatformManager.h @@ -133,6 +133,7 @@ class PlatformManager * before calling Shutdown. */ void RunEventLoop(); + void ProcessDeviceEvents(); /** * Process work items until StopEventLoopTask is called. @@ -222,9 +223,7 @@ class PlatformManager friend class Internal::GenericThreadStackManagerImpl_OpenThread_LwIP; template 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. @@ -351,6 +350,11 @@ inline void PlatformManager::RunEventLoop() static_cast(this)->_RunEventLoop(); } +inline void PlatformManager::ProcessDeviceEvents() +{ + static_cast(this)->_ProcessDeviceEvents(); +} + /** * @brief * Starts the stack on its own task with an associated event queue diff --git a/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.cpp b/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.cpp index e4dd42476fcd9f..784086552e6e39 100644 --- a/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.cpp +++ b/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.cpp @@ -187,6 +187,18 @@ void GenericPlatformManagerImpl_FreeRTOS::_RunEventLoop(void) } } +template +void GenericPlatformManagerImpl_FreeRTOS::_ProcessDeviceEvents() +{ + ChipDeviceEvent event; + BaseType_t eventReceived = xQueueReceive(mChipEventQueue, &event, 0); + while (eventReceived == pdTRUE) + { + Impl()->DispatchEvent(&event); + eventReceived = xQueueReceive(mChipEventQueue, &event, 0); + } +} + template CHIP_ERROR GenericPlatformManagerImpl_FreeRTOS::_StartEventLoopTask(void) { diff --git a/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h b/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h index ab114ecb8d91f3..29b654cad19426 100644 --- a/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h +++ b/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h @@ -71,6 +71,7 @@ class GenericPlatformManagerImpl_FreeRTOS : public GenericPlatformManagerImpl::_PostEvent(const ChipDev } template -void GenericPlatformManagerImpl_POSIX::ProcessDeviceEvents() +void GenericPlatformManagerImpl_POSIX::_ProcessDeviceEvents() { while (!mChipEventQueue.Empty()) { @@ -180,7 +180,7 @@ void GenericPlatformManagerImpl_POSIX::_RunEventLoop() SystemLayerSocketsLoop().HandleEvents(); - ProcessDeviceEvents(); + _ProcessDeviceEvents(); } while (mShouldRunEventLoop.load(std::memory_order_relaxed)); SystemLayerSocketsLoop().EventLoopEnds(); diff --git a/src/include/platform/internal/GenericPlatformManagerImpl_POSIX.h b/src/include/platform/internal/GenericPlatformManagerImpl_POSIX.h index a791c67a953527..48060a5152a965 100644 --- a/src/include/platform/internal/GenericPlatformManagerImpl_POSIX.h +++ b/src/include/platform/internal/GenericPlatformManagerImpl_POSIX.h @@ -91,6 +91,8 @@ class GenericPlatformManagerImpl_POSIX : public GenericPlatformManagerImpl(this); } - void ProcessDeviceEvents(); - DeviceSafeQueue mChipEventQueue; std::atomic mShouldRunEventLoop; static void * EventLoopTaskMain(void * arg); diff --git a/src/include/platform/internal/GenericPlatformManagerImpl_Zephyr.cpp b/src/include/platform/internal/GenericPlatformManagerImpl_Zephyr.cpp index 9b584d6f8570db..87722940a0b765 100644 --- a/src/include/platform/internal/GenericPlatformManagerImpl_Zephyr.cpp +++ b/src/include/platform/internal/GenericPlatformManagerImpl_Zephyr.cpp @@ -124,7 +124,7 @@ CHIP_ERROR GenericPlatformManagerImpl_Zephyr::_PostEvent(const ChipDe } template -void GenericPlatformManagerImpl_Zephyr::ProcessDeviceEvents() +void GenericPlatformManagerImpl_Zephyr::_ProcessDeviceEvents() { ChipDeviceEvent event; @@ -148,7 +148,7 @@ void GenericPlatformManagerImpl_Zephyr::_RunEventLoop(void) SystemLayerSocketsLoop().HandleEvents(); - ProcessDeviceEvents(); + _ProcessDeviceEvents(); } SystemLayerSocketsLoop().EventLoopEnds(); diff --git a/src/include/platform/internal/GenericPlatformManagerImpl_Zephyr.h b/src/include/platform/internal/GenericPlatformManagerImpl_Zephyr.h index 684c7cabddcaf1..4034ada3696ad5 100644 --- a/src/include/platform/internal/GenericPlatformManagerImpl_Zephyr.h +++ b/src/include/platform/internal/GenericPlatformManagerImpl_Zephyr.h @@ -76,6 +76,7 @@ class GenericPlatformManagerImpl_Zephyr : public GenericPlatformManagerImpl(this); } void SysUpdate(); void SysProcess(); - void ProcessDeviceEvents(); static void EventLoopTaskMain(void * thisPtr, void *, void *); }; diff --git a/src/lib/support/LambdaBridge.h b/src/lib/support/LambdaBridge.h index 32b0a17075fc80..768d6b70517906 100644 --- a/src/lib/support/LambdaBridge.h +++ b/src/lib/support/LambdaBridge.h @@ -17,6 +17,7 @@ #pragma once #include +#include #include @@ -35,17 +36,16 @@ class LambdaBridge 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 std::aligned_storage & body) { - (*reinterpret_cast(&body))(); - }; - memcpy(&mLambdaBody, &lambda, sizeof(Lambda)); + mLambdaProxy = [](const LambdaStorage & body) { (*reinterpret_cast(&body))(); }; + ::memcpy(&mLambdaBody, &lambda, sizeof(Lambda)); } void operator()() const { mLambdaProxy(mLambdaBody); } private: - void (*mLambdaProxy)(const std::aligned_storage & body); - std::aligned_storage mLambdaBody; + using LambdaStorage = std::aligned_storage::type; + void (*mLambdaProxy)(const LambdaStorage & body); + LambdaStorage mLambdaBody; }; static_assert(std::is_trivial::value, "LambdaBridge is not trivial"); diff --git a/src/platform/Darwin/PlatformManagerImpl.cpp b/src/platform/Darwin/PlatformManagerImpl.cpp index 348ef48a21221a..5524d2a066f129 100644 --- a/src/platform/Darwin/PlatformManagerImpl.cpp +++ b/src/platform/Darwin/PlatformManagerImpl.cpp @@ -108,6 +108,11 @@ void PlatformManagerImpl::_RunEventLoop() dispatch_semaphore_wait(mRunLoopSem, DISPATCH_TIME_FOREVER); } +void PlatformManagerImpl::_ProcessDeviceEvents() +{ + // Not implemented +} + CHIP_ERROR PlatformManagerImpl::_Shutdown() { // Call up to the base class _Shutdown() to perform the bulk of the shutdown. diff --git a/src/platform/Darwin/PlatformManagerImpl.h b/src/platform/Darwin/PlatformManagerImpl.h index 40eab2aa749b44..b7ad1b01c5ca0b 100644 --- a/src/platform/Darwin/PlatformManagerImpl.h +++ b/src/platform/Darwin/PlatformManagerImpl.h @@ -62,6 +62,7 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener CHIP_ERROR _StartEventLoopTask(); CHIP_ERROR _StopEventLoopTask(); void _RunEventLoop(); + void _ProcessDeviceEvents(); void _LockChipStack(){}; bool _TryLockChipStack() { return false; }; void _UnlockChipStack(){}; diff --git a/src/platform/PlatformEventSupport.cpp b/src/platform/PlatformEventSupport.cpp index f7ed770bdf8f6f..7863aeea817bf2 100644 --- a/src/platform/PlatformEventSupport.cpp +++ b/src/platform/PlatformEventSupport.cpp @@ -27,8 +27,6 @@ #include -#if CHIP_SYSTEM_CONFIG_USE_LWIP - namespace chip { namespace System { @@ -62,5 +60,3 @@ CHIP_ERROR PlatformEventing::StartTimer(System::Layer & aLayer, System::Clock::T } // namespace System } // namespace chip - -#endif // CHIP_SYSTEM_CONFIG_USE_LWIP diff --git a/src/platform/fake/PlatformManagerImpl.h b/src/platform/fake/PlatformManagerImpl.h index 4bbd9df6908fae..b47f616dedfea9 100644 --- a/src/platform/fake/PlatformManagerImpl.h +++ b/src/platform/fake/PlatformManagerImpl.h @@ -24,6 +24,8 @@ #include +#include + 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 mQueue; }; /** diff --git a/src/platform/mbed/PlatformManagerImpl.cpp b/src/platform/mbed/PlatformManagerImpl.cpp index 0dc462ccca1659..9c99b6fed7d267 100644 --- a/src/platform/mbed/PlatformManagerImpl.cpp +++ b/src/platform/mbed/PlatformManagerImpl.cpp @@ -130,7 +130,7 @@ CHIP_ERROR PlatformManagerImpl::_PostEvent(const ChipDeviceEvent * eventPtr) return CHIP_NO_ERROR; } -void PlatformManagerImpl::ProcessDeviceEvents() +void PlatformManagerImpl::_ProcessDeviceEvents() { mQueue.dispatch(0); } @@ -178,7 +178,7 @@ void PlatformManagerImpl::_RunEventLoop() SystemLayerSocketsLoop().HandleEvents(); - ProcessDeviceEvents(); + _ProcessDeviceEvents(); } SystemLayerSocketsLoop().EventLoopEnds(); diff --git a/src/platform/mbed/PlatformManagerImpl.h b/src/platform/mbed/PlatformManagerImpl.h index 59b119889c7f46..bc1c2ece931be4 100644 --- a/src/platform/mbed/PlatformManagerImpl.h +++ b/src/platform/mbed/PlatformManagerImpl.h @@ -86,13 +86,12 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener void _UnlockChipStack(); CHIP_ERROR _PostEvent(const ChipDeviceEvent * event); void _RunEventLoop(); + void _ProcessDeviceEvents(); CHIP_ERROR _StartEventLoopTask(); CHIP_ERROR _StopEventLoopTask(); CHIP_ERROR _StartChipTimer(System::Clock::Timeout duration); CHIP_ERROR _Shutdown(); - void ProcessDeviceEvents(); - // ===== Members for internal use by the following friends. friend PlatformManager & PlatformMgr(void); diff --git a/src/system/BUILD.gn b/src/system/BUILD.gn index e4538f9a930142..0aaa7a9e192674 100644 --- a/src/system/BUILD.gn +++ b/src/system/BUILD.gn @@ -123,6 +123,7 @@ static_library("system") { output_name = "libSystemLayer" sources = [ + "PlatformEventSupport.h", "SystemAlignSize.h", "SystemClock.cpp", "SystemClock.h", @@ -170,10 +171,6 @@ static_library("system") { } } - if (chip_system_config_use_lwip) { - sources += [ "PlatformEventSupport.h" ] - } - if (chip_with_nlfaultinjection) { sources += [ "SystemFaultInjection.cpp" ] public_deps += [ "${nlfaultinjection_root}:nlfaultinjection" ] diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h index b61acf22c6fe70..d69c2ddb773a44 100644 --- a/src/system/SystemLayer.h +++ b/src/system/SystemLayer.h @@ -25,8 +25,6 @@ #pragma once -#include - // Include configuration headers #include diff --git a/src/system/tests/BUILD.gn b/src/system/tests/BUILD.gn index 6315029f2c6915..432bc11d366ac7 100644 --- a/src/system/tests/BUILD.gn +++ b/src/system/tests/BUILD.gn @@ -26,6 +26,7 @@ chip_test_suite("tests") { "TestSystemErrorStr.cpp", "TestSystemObject.cpp", "TestSystemPacketBuffer.cpp", + "TestSystemScheduleLambda.cpp", "TestSystemTimer.cpp", "TestSystemWakeEvent.cpp", "TestTimeSource.cpp", diff --git a/src/system/tests/TestSystemScheduleLambda.cpp b/src/system/tests/TestSystemScheduleLambda.cpp new file mode 100644 index 00000000000000..501b9bc106d6fc --- /dev/null +++ b/src/system/tests/TestSystemScheduleLambda.cpp @@ -0,0 +1,106 @@ +/* + * 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. + */ + +#include + +#include +#include +#include +#include +#include + +// Test input data. + +static void CheckScheduleLambda(nlTestSuite * inSuite, void * aContext) +{ + bool * called = new bool(false); +#if CHIP_SYSTEM_CONFIG_USE_DISPATCH + chip::DeviceLayer::PlatformMgr().RunEventLoop(); + chip::DeviceLayer::SystemLayer().ScheduleLambda([called] { + *called = true; + chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); + }); +#else + chip::DeviceLayer::SystemLayer().ScheduleLambda([called] { *called = true; }); + while (!*called) + { + chip::DeviceLayer::PlatformMgrImpl().ProcessDeviceEvents(); + } +#endif + NL_TEST_ASSERT(inSuite, *called); + delete called; +} + +// Test Suite + +/** + * Test Suite. It lists all the test functions. + */ +// clang-format off +static const nlTest sTests[] = +{ + NL_TEST_DEF("System::TestScheduleLambda", CheckScheduleLambda), + NL_TEST_SENTINEL() +}; +// clang-format on + +static int TestSetup(void * aContext); +static int TestTeardown(void * aContext); + +// clang-format off +static nlTestSuite kTheSuite = +{ + "chip-system-schedule-lambda", + &sTests[0], + TestSetup, + TestTeardown +}; +// clang-format on + +/** + * Set up the test suite. + */ +static int TestSetup(void * aContext) +{ + if (chip::DeviceLayer::PlatformMgr().InitChipStack() != CHIP_NO_ERROR) + return FAILURE; + + return (SUCCESS); +} + +/** + * Tear down the test suite. + * Free memory reserved at TestSetup. + */ +static int TestTeardown(void * aContext) +{ + CHIP_ERROR err = chip::DeviceLayer::PlatformMgr().Shutdown(); + // RTOS shutdown is not implemented, ignore CHIP_ERROR_NOT_IMPLEMENTED + if (err != CHIP_NO_ERROR && err != CHIP_ERROR_NOT_IMPLEMENTED) + return FAILURE; + + return (SUCCESS); +} + +int TestSystemScheduleLambda(void) +{ + // Run test suit againt one lContext. + nlTestRunner(&kTheSuite, nullptr); + + return nlTestRunnerStats(&kTheSuite); +} + +CHIP_REGISTER_TEST_SUITE(TestSystemScheduleLambda)