diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index 709b11d92451a0..0952ff7ec5402b 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -20,20 +20,6 @@ declare_args() { # Enable strict schema checks. chip_enable_schema_check = is_debug && (current_os == "linux" || current_os == "mac") - - # Mutex implementation: posix, freertos, zephyr. - chip_im_config_critical_section = "" -} - -# TODO: Add locker support for different platform -if (chip_im_config_critical_section == "") { - if (current_os == "freertos") { - chip_im_config_critical_section = "rtos" - } else if (current_os == "zephyr") { - chip_im_config_critical_section = "zephyr" - } else { - chip_im_config_critical_section = "posix" - } } config("app_config") { @@ -110,16 +96,6 @@ static_library("app") { "reporting/Engine.h", ] - if (chip_im_config_critical_section == "posix") { - sources += [ "CriticalSectionImplPosix.cpp" ] - } else if (chip_im_config_critical_section == "rtos") { - sources += [ "CriticalSectionImplRtos.cpp" ] - } else if (chip_im_config_critical_section == "zephyr") { - sources += [ "CriticalSectionImplZephyr.cpp" ] - } else { - assert(false, "Unknown IM critical section implementation.") - } - if (chip_ip_commissioning) { defines = [ "CONFIG_USE_CLUSTERS_FOR_IP_COMMISSIONING=1", diff --git a/src/app/CriticalSection.h b/src/app/CriticalSection.h deleted file mode 100644 index 7a0f4e3489dec2..00000000000000 --- a/src/app/CriticalSection.h +++ /dev/null @@ -1,28 +0,0 @@ -/* - * - * Copyright (c) 2021 Project CHIP Authors - * All rights reserved. - * - * 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 - -namespace chip { -namespace app { -void CriticalSectionEnter(); - -void CriticalSectionExit(); - -} // namespace app -} // namespace chip diff --git a/src/app/CriticalSectionImplPosix.cpp b/src/app/CriticalSectionImplPosix.cpp deleted file mode 100644 index 0ce22bed1eb36d..00000000000000 --- a/src/app/CriticalSectionImplPosix.cpp +++ /dev/null @@ -1,39 +0,0 @@ -/* - * - * Copyright (c) 2021 Project CHIP Authors - * All rights reserved. - * - * 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 - -namespace chip { -namespace app { - -static pthread_mutex_t eventLocker = PTHREAD_MUTEX_INITIALIZER; - -void CriticalSectionEnter() -{ - int err = pthread_mutex_lock(&eventLocker); - assert(err == 0); -} - -void CriticalSectionExit() -{ - int err = pthread_mutex_unlock(&eventLocker); - assert(err == 0); -} -} // namespace app -} // namespace chip diff --git a/src/app/CriticalSectionImplRtos.cpp b/src/app/CriticalSectionImplRtos.cpp deleted file mode 100644 index 641eb9f05b4f57..00000000000000 --- a/src/app/CriticalSectionImplRtos.cpp +++ /dev/null @@ -1,36 +0,0 @@ -/* - * - * Copyright (c) 2021 Project CHIP Authors - * All rights reserved. - * - * 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 "CriticalSection.h" -#include "FreeRTOS.h" -#include "task.h" - -namespace chip { -namespace app { -void CriticalSectionEnter() -{ - // TODO: Add freertos lock, it seems in EFR32, it needs portENTER_CRITICAL, in ESP32, it needs portENTER_CRITICAL(mux) - // taskENTER_CRITICAL(); -} - -void CriticalSectionExit() -{ - // taskEXIT_CRITICAL(); -} -} // namespace app -} // namespace chip diff --git a/src/app/CriticalSectionImplZephyr.cpp b/src/app/CriticalSectionImplZephyr.cpp deleted file mode 100644 index d3184e4adefe0c..00000000000000 --- a/src/app/CriticalSectionImplZephyr.cpp +++ /dev/null @@ -1,25 +0,0 @@ -/* - * - * Copyright (c) 2021 Project CHIP Authors - * All rights reserved. - * - * 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. - */ - -namespace chip { -namespace app { -void CriticalSectionEnter() {} - -void CriticalSectionExit() {} -} // namespace app -} // namespace chip diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp index 967f21b2506aef..ad99a4c8bd6932 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -16,7 +16,6 @@ * limitations under the License. */ -#include #include #include #include @@ -98,25 +97,28 @@ struct EventEnvelopeContext EventId mEventId = 0; }; -EventManagement::EventManagement(Messaging::ExchangeManager * apExchangeMgr, int aNumBuffers, - CircularEventBuffer * apCircularEventBuffer, - const LogStorageResources * const apLogStorageResources) -{ - Init(apExchangeMgr, aNumBuffers, apCircularEventBuffer, apLogStorageResources); -} - -void EventManagement::Init(Messaging::ExchangeManager * apExchangeManager, int aNumBuffers, +void EventManagement::Init(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer, const LogStorageResources * const apLogStorageResources) { + CHIP_ERROR err = CHIP_NO_ERROR; CircularEventBuffer * current = nullptr; CircularEventBuffer * prev = nullptr; CircularEventBuffer * next = nullptr; - VerifyOrDie(aNumBuffers > 0); + if (aNumBuffers == 0) + { + ChipLogError(EventLogging, "Invalid aNumBuffers"); + return; + } + if (mState != EventManagementStates::Shutdown) + { + ChipLogError(EventLogging, "Invalid EventManagement State"); + return; + } mpExchangeMgr = apExchangeManager; - for (int bufferIndex = 0; bufferIndex < aNumBuffers; bufferIndex++) + for (uint32_t bufferIndex = 0; bufferIndex < aNumBuffers; bufferIndex++) { next = (bufferIndex < aNumBuffers - 1) ? &apCircularEventBuffer[bufferIndex + 1] : nullptr; @@ -134,6 +136,12 @@ void EventManagement::Init(Messaging::ExchangeManager * apExchangeManager, int a mpEventBuffer = apCircularEventBuffer; mState = EventManagementStates::Idle; mBytesWritten = 0; + + err = chip::System::Mutex::Init(mAccessLock); + if (err != CHIP_NO_ERROR) + { + ChipLogError(EventLogging, "mutex init fails with error %s", ErrorStr(err)); + } } CHIP_ERROR EventManagement::CopyToNextBuffer(CircularEventBuffer * apEventBuffer) @@ -361,13 +369,12 @@ CHIP_ERROR EventManagement::ConstructEvent(EventLoadOutContext * apContext, Even return err; } -void EventManagement::CreateEventManagement(Messaging::ExchangeManager * apExchangeManager, int aNumBuffers, +void EventManagement::CreateEventManagement(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer, const LogStorageResources * const apLogStorageResources) { sInstance.Init(apExchangeManager, aNumBuffers, apCircularEventBuffer, apLogStorageResources); - static_assert(std::is_trivially_destructible::value, "EventManagement must be trivially destructible"); } /** @@ -375,11 +382,10 @@ void EventManagement::CreateEventManagement(Messaging::ExchangeManager * apExcha */ void EventManagement::DestroyEventManagement() { - CriticalSectionEnter(); + ScopedLock lock(sInstance); sInstance.mState = EventManagementStates::Shutdown; sInstance.mpEventBuffer = nullptr; sInstance.mpExchangeMgr = nullptr; - CriticalSectionExit(); } EventNumber CircularEventBuffer::VendEventNumber() @@ -462,15 +468,15 @@ CHIP_ERROR EventManagement::CopyAndAdjustDeltaTime(const TLVReader & aReader, si CHIP_ERROR EventManagement::LogEvent(EventLoggingDelegate * apDelegate, EventOptions & aEventOptions, EventNumber & aEventNumber) { CHIP_ERROR err = CHIP_NO_ERROR; - CriticalSectionEnter(); - - VerifyOrExit(mState != EventManagementStates::Shutdown, err = CHIP_ERROR_INCORRECT_STATE); - VerifyOrExit(aEventOptions.mpEventSchema != nullptr, err = CHIP_ERROR_INCORRECT_STATE); + { + ScopedLock lock(sInstance); - err = LogEventPrivate(apDelegate, aEventOptions, aEventNumber); + VerifyOrExit(mState != EventManagementStates::Shutdown, err = CHIP_ERROR_INCORRECT_STATE); + VerifyOrExit(aEventOptions.mpEventSchema != nullptr, err = CHIP_ERROR_INCORRECT_STATE); + err = LogEventPrivate(apDelegate, aEventOptions, aEventNumber); + } exit: - CriticalSectionExit(); ChipLogFunctError(err); return err; } @@ -692,7 +698,7 @@ CHIP_ERROR EventManagement::FetchEventsSince(TLVWriter & aWriter, ClusterInfo * EventLoadOutContext context(aWriter, aPriority, aEventNumber); CircularEventBuffer * buf = mpEventBuffer; - CriticalSectionEnter(); + ScopedLock lock(sInstance); while (!buf->IsFinalDestinationForPriority(aPriority)) { @@ -713,7 +719,6 @@ CHIP_ERROR EventManagement::FetchEventsSince(TLVWriter & aWriter, ClusterInfo * exit: aEventNumber = context.mCurrentEventNumber; - CriticalSectionExit(); return err; } @@ -832,7 +837,7 @@ void EventManagement::SetScheduledEventEndpoint(EventNumber * apEventEndpoints) { CircularEventBuffer * eventBuffer = mpEventBuffer; - CriticalSectionEnter(); + ScopedLock lock(sInstance); while (eventBuffer != nullptr) { @@ -842,8 +847,6 @@ void EventManagement::SetScheduledEventEndpoint(EventNumber * apEventEndpoints) } eventBuffer = eventBuffer->GetNextCircularEventBuffer(); } - - CriticalSectionExit(); } void CircularEventBuffer::Init(uint8_t * apBuffer, uint32_t aBufferLength, CircularEventBuffer * apPrev, diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h index 2bd875f6c1af8a..74682d3ba664f1 100644 --- a/src/app/EventManagement.h +++ b/src/app/EventManagement.h @@ -34,6 +34,7 @@ #include #include #include +#include #define CHIP_CONFIG_EVENT_GLOBAL_PRIORITY PriorityLevel::Debug @@ -241,8 +242,6 @@ class EventManagement public: /** * @brief - * EventManagement constructor - * * Initialize the EventManagement with an array of LogStorageResources. The * array must provide a resource for each valid priority level, the elements * of the array must be in increasing numerical value of priority (and in @@ -259,16 +258,8 @@ class EventManagement * @param[in] apLogStorageResources An array of LogStorageResources for each priority level. * */ - EventManagement(Messaging::ExchangeManager * apExchangeManager, int aNumBuffers, CircularEventBuffer * apCircularEventBuffer, - const LogStorageResources * const apLogStorageResources); - - void Init(Messaging::ExchangeManager * apExchangeManager, int aNumBuffers, CircularEventBuffer * apCircularEventBuffer, + void Init(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer, const LogStorageResources * const apLogStorageResources); - /** - * @brief - * EventManagement default constructor. Provided primarily to make the compiler happy. - */ - EventManagement(){}; static EventManagement & GetInstance(); @@ -292,12 +283,21 @@ class EventManagement * * @note This function must be called prior to the logging being used. */ - static void CreateEventManagement(Messaging::ExchangeManager * apExchangeManager, int aNumBuffers, + static void CreateEventManagement(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer, const LogStorageResources * const apLogStorageResources); static void DestroyEventManagement(); + class ScopedLock + { + public: + ScopedLock(EventManagement & aEventManagement) : mEventManagement(aEventManagement) { mEventManagement.mAccessLock.Lock(); } + ~ScopedLock() { mEventManagement.mAccessLock.Unlock(); } + + private: + EventManagement & mEventManagement; + }; /** * @brief * Log an event via a EventLoggingDelegate, with options. @@ -566,6 +566,7 @@ class EventManagement Messaging::ExchangeManager * mpExchangeMgr = nullptr; EventManagementStates mState = EventManagementStates::Shutdown; uint32_t mBytesWritten = 0; + System::Mutex mAccessLock; }; } // namespace app } // namespace chip diff --git a/src/inet/InetLayer.h b/src/inet/InetLayer.h index 0067c80f5f0d19..52793025fefada 100644 --- a/src/inet/InetLayer.h +++ b/src/inet/InetLayer.h @@ -92,7 +92,11 @@ #endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING #if CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING +#if defined(ESP_PLATFORM) +#include "freertos/FreeRTOS.h" +#else #include +#endif #include #endif // CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING diff --git a/src/platform/EFR32/SystemPlatformConfig.h b/src/platform/EFR32/SystemPlatformConfig.h index 2e3db33f356f95..33df2b3a177b2d 100644 --- a/src/platform/EFR32/SystemPlatformConfig.h +++ b/src/platform/EFR32/SystemPlatformConfig.h @@ -34,10 +34,6 @@ struct ChipDeviceEvent; } // namespace chip // ==================== Platform Adaptations ==================== - -#define CHIP_SYSTEM_CONFIG_POSIX_LOCKING 0 -#define CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING 0 -#define CHIP_SYSTEM_CONFIG_NO_LOCKING 1 #define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_EVENT_FUNCTIONS 1 #define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME 1 #define CHIP_SYSTEM_CONFIG_LWIP_EVENT_TYPE int diff --git a/src/platform/ESP32/SystemPlatformConfig.h b/src/platform/ESP32/SystemPlatformConfig.h index 2907302439b4fc..2253ec47fd3532 100644 --- a/src/platform/ESP32/SystemPlatformConfig.h +++ b/src/platform/ESP32/SystemPlatformConfig.h @@ -37,10 +37,6 @@ struct ChipDeviceEvent; } // namespace chip // ==================== Platform Adaptations ==================== - -#define CHIP_SYSTEM_CONFIG_POSIX_LOCKING 0 -#define CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING 0 -#define CHIP_SYSTEM_CONFIG_NO_LOCKING 1 #define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_EVENT_FUNCTIONS 1 #define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME 1 #define CHIP_SYSTEM_CONFIG_LWIP_EVENT_TYPE int diff --git a/src/platform/K32W/SystemPlatformConfig.h b/src/platform/K32W/SystemPlatformConfig.h index f514d498af21b6..059d2c7a2881aa 100644 --- a/src/platform/K32W/SystemPlatformConfig.h +++ b/src/platform/K32W/SystemPlatformConfig.h @@ -35,10 +35,6 @@ struct ChipDeviceEvent; } // namespace chip // ==================== Platform Adaptations ==================== - -#define CHIP_SYSTEM_CONFIG_POSIX_LOCKING 0 -#define CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING 0 -#define CHIP_SYSTEM_CONFIG_NO_LOCKING 1 #define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_EVENT_FUNCTIONS 1 #define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME 1 #define CHIP_SYSTEM_CONFIG_LWIP_EVENT_TYPE int diff --git a/src/platform/cc13x2_26x2/SystemPlatformConfig.h b/src/platform/cc13x2_26x2/SystemPlatformConfig.h index ff6c4212dfec25..c1dc8bc763e3aa 100644 --- a/src/platform/cc13x2_26x2/SystemPlatformConfig.h +++ b/src/platform/cc13x2_26x2/SystemPlatformConfig.h @@ -34,10 +34,6 @@ struct ChipDeviceEvent; } // namespace chip // ==================== Platform Adaptations ==================== - -#define CHIP_SYSTEM_CONFIG_POSIX_LOCKING 0 -#define CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING 0 -#define CHIP_SYSTEM_CONFIG_NO_LOCKING 1 #define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_EVENT_FUNCTIONS 1 #define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME 1 #define CHIP_SYSTEM_CONFIG_LWIP_EVENT_TYPE int diff --git a/src/platform/qpg6100/SystemPlatformConfig.h b/src/platform/qpg6100/SystemPlatformConfig.h index 452c11045c2d25..4842bb004808cc 100644 --- a/src/platform/qpg6100/SystemPlatformConfig.h +++ b/src/platform/qpg6100/SystemPlatformConfig.h @@ -33,10 +33,6 @@ struct ChipDeviceEvent; } // namespace chip // ==================== Platform Adaptations ==================== - -#define CHIP_SYSTEM_CONFIG_POSIX_LOCKING 0 -#define CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING 0 -#define CHIP_SYSTEM_CONFIG_NO_LOCKING 1 #define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_EVENT_FUNCTIONS 1 #define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME 1 #define CHIP_SYSTEM_CONFIG_LWIP_EVENT_TYPE int diff --git a/src/system/SystemMutex.cpp b/src/system/SystemMutex.cpp index 65c93398bfa267..4dfb351915c85e 100644 --- a/src/system/SystemMutex.cpp +++ b/src/system/SystemMutex.cpp @@ -82,7 +82,7 @@ DLL_EXPORT Error Mutex::Init(Mutex & aThis) #else aThis.mFreeRTOSSemaphore = xSemaphoreCreateMutex(); #endif - if (aThis.mFreeRTOSSemaphore == NULL) + if (aThis.mFreeRTOSSemaphore == nullptr) { aThis.mInitialized = 0; @@ -91,7 +91,7 @@ DLL_EXPORT Error Mutex::Init(Mutex & aThis) } else { - while (aThis.mFreeRTOSSemaphore == NULL) + while (aThis.mFreeRTOSSemaphore == nullptr) { vTaskDelay(1); diff --git a/src/system/SystemMutex.h b/src/system/SystemMutex.h index 1908ffe00158e5..2c65971ab7d191 100644 --- a/src/system/SystemMutex.h +++ b/src/system/SystemMutex.h @@ -39,9 +39,15 @@ #endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING #if CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING +#if defined(ESP_PLATFORM) +#include "freertos/FreeRTOS.h" +#include "freertos/semphr.h" +#include "freertos/task.h" +#else #include #include #include +#endif #endif // CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING namespace chip { @@ -78,8 +84,8 @@ class DLL_EXPORT Mutex #if (configSUPPORT_STATIC_ALLOCATION == 1) StaticSemaphore_t mFreeRTOSSemaphoreObj; #endif // (configSUPPORT_STATIC_ALLOCATION == 1) - volatile SemaphoreHandle_t mFreeRTOSSemaphore; - volatile int mInitialized; + volatile SemaphoreHandle_t mFreeRTOSSemaphore = nullptr; + volatile int mInitialized = 0; #endif // CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING Mutex(const Mutex &) = delete; diff --git a/src/system/system.gni b/src/system/system.gni index 94fee4ce15576f..7181389d241be8 100644 --- a/src/system/system.gni +++ b/src/system/system.gni @@ -43,7 +43,7 @@ if (chip_system_config_locking == "") { if (current_os != "freertos") { chip_system_config_locking = "posix" } else { - chip_system_config_locking = "none" + chip_system_config_locking = "freertos" } }