From 3bd289dae3960d2eda9085a2f6663e01edada8cb Mon Sep 17 00:00:00 2001 From: Sid Hsu <3542660+hsusid@users.noreply.github.com> Date: Thu, 27 Jan 2022 03:36:28 +0800 Subject: [PATCH] Update LE, stack overflow, and other fixes for CYW30739. (#14306) * Fix Bluetooth LE service data. * Dispatch Matter app server initialization to Matter thread. The stack size of the app thread is not sufficient for the new chip::Server::Init implementation. Therefore, we move the server initialization to the Matter thread. * Add the chip_openthread_ftd arg. * Add Atm category prefix and task symbol to logs. --- examples/lighting-app/cyw30739/BUILD.gn | 1 - examples/lighting-app/cyw30739/args.gni | 2 ++ examples/lighting-app/cyw30739/src/main.cpp | 28 +++++++++------ src/platform/CYW30739/BLEManagerImpl.cpp | 27 ++++++-------- src/platform/CYW30739/BUILD.gn | 6 +++- .../CYW30739/CHIPDevicePlatformConfig.h | 1 - src/platform/CYW30739/Logging.cpp | 35 +++++++++++++++---- src/platform/CYW30739/PlatformManagerImpl.h | 5 ++- .../CYW30739/ThreadStackManagerImpl.h | 2 ++ 9 files changed, 69 insertions(+), 38 deletions(-) diff --git a/examples/lighting-app/cyw30739/BUILD.gn b/examples/lighting-app/cyw30739/BUILD.gn index 8912b0594c2c76..a073a3a108a397 100644 --- a/examples/lighting-app/cyw30739/BUILD.gn +++ b/examples/lighting-app/cyw30739/BUILD.gn @@ -53,7 +53,6 @@ cyw30739_executable("lighting_app") { "${chip_root}/examples/lighting-app/lighting-common", "${chip_root}/examples/shell/shell_common:shell_common", "${chip_root}/src/lib", - "${chip_root}/third_party/openthread/repo:libopenthread-ftd", ] include_dirs = [ "include" ] diff --git a/examples/lighting-app/cyw30739/args.gni b/examples/lighting-app/cyw30739/args.gni index 149f7d42b18b5f..8361c808a27a37 100644 --- a/examples/lighting-app/cyw30739/args.gni +++ b/examples/lighting-app/cyw30739/args.gni @@ -16,3 +16,5 @@ import("//build_overrides/chip.gni") import("${chip_root}/src/platform/CYW30739/args.gni") cyw30739_sdk_target = get_label_info(":sdk", "label_no_toolchain") + +chip_openthread_ftd = true diff --git a/examples/lighting-app/cyw30739/src/main.cpp b/examples/lighting-app/cyw30739/src/main.cpp index 3a99588bc7f4f6..9ec4cc5c6e3a63 100644 --- a/examples/lighting-app/cyw30739/src/main.cpp +++ b/examples/lighting-app/cyw30739/src/main.cpp @@ -37,6 +37,7 @@ using namespace ::chip::Credentials; using namespace ::chip::DeviceLayer; using namespace ::chip::Shell; +static void InitApp(intptr_t args); static void EventHandler(const ChipDeviceEvent * event, intptr_t arg); static void HandleThreadStateChangeEvent(const ChipDeviceEvent * event); static void LightManagerCallback(LightingManager::Actor_t actor, LightingManager::Action_t action, uint8_t value); @@ -114,17 +115,7 @@ APPLICATION_START() } #endif - PlatformMgrImpl().AddEventHandler(EventHandler, 0); - - LightMgr().Init(); - LightMgr().SetCallbacks(LightManagerCallback, NULL); - - /* Start CHIP datamodel server */ - chip::Server::GetInstance().Init(); - - SetDeviceAttestationCredentialsProvider(Examples::GetExampleDACProvider()); - - ConfigurationMgr().LogDeviceConfig(); + PlatformMgr().ScheduleWork(InitApp, 0); const int ret = Engine::Root().Init(); if (!chip::ChipError::IsSuccess(ret)) @@ -138,6 +129,21 @@ APPLICATION_START() assert(!wiced_rtos_check_for_stack_overflow()); } +void InitApp(intptr_t args) +{ + ConfigurationMgr().LogDeviceConfig(); + + PlatformMgrImpl().AddEventHandler(EventHandler, 0); + + /* Start CHIP datamodel server */ + chip::Server::GetInstance().Init(); + + SetDeviceAttestationCredentialsProvider(Examples::GetExampleDACProvider()); + + LightMgr().Init(); + LightMgr().SetCallbacks(LightManagerCallback, NULL); +} + void EventHandler(const ChipDeviceEvent * event, intptr_t arg) { switch (event->Type) diff --git a/src/platform/CYW30739/BLEManagerImpl.cpp b/src/platform/CYW30739/BLEManagerImpl.cpp index eabb24068e56a0..4c777db3e195c1 100644 --- a/src/platform/CYW30739/BLEManagerImpl.cpp +++ b/src/platform/CYW30739/BLEManagerImpl.cpp @@ -20,7 +20,7 @@ /** * @file * Provides an implementation of the BLEManager singleton object - * for the P64343W platform. + * for the CYW30739 platform. */ /* this file behaves like a config.h, comes first */ @@ -778,24 +778,27 @@ void BLEManagerImpl::SetAdvertisingData(void) wiced_bt_ble_advert_elem_t adv_elem[4]; uint8_t num_elem = 0; uint8_t flag = BTM_BLE_GENERAL_DISCOVERABLE_FLAG | BTM_BLE_BREDR_NOT_SUPPORTED; - uint8_t chip_service_uuid[2] = { BIT16_TO_8(__UUID16_CHIPoBLEService) }; - ChipBLEDeviceIdentificationInfo mDeviceIdInfo; uint16_t deviceDiscriminator = 0; uint8_t localDeviceNameLen; - uint8_t service_data[9]; - uint8_t * p = service_data; uint8_t * rpa = wiced_btm_get_private_bda(); + struct + { + uint16_t uuid; + ChipBLEDeviceIdentificationInfo info; + } service_data = { + .uuid = __UUID16_CHIPoBLEService, + }; // Initialize the CHIP BLE Device Identification Information block that will be sent as payload // within the BLE service advertisement data. - err = ConfigurationMgr().GetBLEDeviceIdentificationInfo(mDeviceIdInfo); + err = ConfigurationMgr().GetBLEDeviceIdentificationInfo(service_data.info); SuccessOrExit(err); // Verify device name was not already set if (!sInstance.mFlags.Has(Flags::kFlag_DeviceNameSet)) { /* Default device name is CHIP- */ - deviceDiscriminator = mDeviceIdInfo.GetDeviceDiscriminator(); + deviceDiscriminator = service_data.info.GetDeviceDiscriminator(); memset(sInstance.mDeviceName, 0, kMaxDeviceNameLength); snprintf(sInstance.mDeviceName, kMaxDeviceNameLength, "%s%04u", CHIP_DEVICE_CONFIG_BLE_DEVICE_NAME_PREFIX, @@ -827,16 +830,8 @@ void BLEManagerImpl::SetAdvertisingData(void) /* Second element is the service data for CHIP service */ adv_elem[num_elem].advert_type = BTM_BLE_ADVERT_TYPE_SERVICE_DATA; adv_elem[num_elem].len = sizeof(service_data); - adv_elem[num_elem].p_data = service_data; + adv_elem[num_elem].p_data = (uint8_t *) &service_data; num_elem++; - UINT8_TO_STREAM(p, chip_service_uuid[0]); - UINT8_TO_STREAM(p, chip_service_uuid[1]); - UINT8_TO_STREAM(p, 0); // CHIP BLE Opcode == 0x00 (Uncommissioned) - UINT16_TO_STREAM(p, deviceDiscriminator); - UINT8_TO_STREAM(p, mDeviceIdInfo.DeviceVendorId[0]); - UINT8_TO_STREAM(p, mDeviceIdInfo.DeviceVendorId[1]); - UINT8_TO_STREAM(p, mDeviceIdInfo.DeviceProductId[0]); - UINT8_TO_STREAM(p, mDeviceIdInfo.DeviceProductId[1]); adv_elem[num_elem].advert_type = BTM_BLE_ADVERT_TYPE_NAME_COMPLETE; adv_elem[num_elem].len = localDeviceNameLen; diff --git a/src/platform/CYW30739/BUILD.gn b/src/platform/CYW30739/BUILD.gn index b09c70c9748272..d5c2f29692cfd9 100644 --- a/src/platform/CYW30739/BUILD.gn +++ b/src/platform/CYW30739/BUILD.gn @@ -69,8 +69,12 @@ static_library("CYW30739") { deps += [ "${chip_root}/third_party/openthread/platforms:libopenthread-platform", - "${openthread_root}/src/core:libopenthread_core_headers", ] + if (chip_openthread_ftd) { + deps += [ "${chip_root}/third_party/openthread/repo:libopenthread-ftd" ] + } else { + deps += [ "${chip_root}/third_party/openthread/repo:libopenthread-mtd" ] + } if (chip_mdns == "platform") { sources += [ "../OpenThread/DnssdImpl.cpp" ] diff --git a/src/platform/CYW30739/CHIPDevicePlatformConfig.h b/src/platform/CYW30739/CHIPDevicePlatformConfig.h index f19d20c703a88b..3b48f0d956b4e3 100644 --- a/src/platform/CYW30739/CHIPDevicePlatformConfig.h +++ b/src/platform/CYW30739/CHIPDevicePlatformConfig.h @@ -55,7 +55,6 @@ // -------------------- Thread Configuration -------------------- #ifndef CHIP_DEVICE_CONFIG_ENABLE_THREAD #define CHIP_DEVICE_CONFIG_ENABLE_THREAD 1 -#define CHIP_DEVICE_CONFIG_THREAD_FTD 1 #define CHIP_DEVICE_CONFIG_THREAD_TASK_STACK_SIZE 0x00001000 #define CHIP_DEVICE_CONFIG_THREAD_SRP_MAX_SERVICES 5 #define CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT 1 diff --git a/src/platform/CYW30739/Logging.cpp b/src/platform/CYW30739/Logging.cpp index 982666594ca291..d49d4e919b3cb1 100644 --- a/src/platform/CYW30739/Logging.cpp +++ b/src/platform/CYW30739/Logging.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -12,22 +13,42 @@ namespace Platform { void LogV(const char * module, uint8_t category, const char * msg, va_list v) { + const char * category_str; switch (category) { - case chip::Logging::LogCategory::kLogCategory_Error: - printf("Error"); + case LogCategory::kLogCategory_None: + return; + case LogCategory::kLogCategory_Error: + category_str = "Err"; break; - case chip::Logging::LogCategory::kLogCategory_Progress: - printf("InfoP"); + case LogCategory::kLogCategory_Progress: + category_str = "Prg"; break; - case chip::Logging::LogCategory::kLogCategory_Detail: - printf("InfoD"); + case LogCategory::kLogCategory_Detail: + category_str = "Dtl"; break; + case LogCategory::kLogCategory_Automation: + category_str = "Atm"; + break; + } + + char task_symbol; + if (DeviceLayer::PlatformMgrImpl().IsCurrentTask()) + { + task_symbol = 'M'; + } + else if (DeviceLayer::ThreadStackMgrImpl().IsCurrentTask()) + { + task_symbol = 'T'; + } + else + { + task_symbol = 'A'; } static char buffer[256]; vsnprintf(buffer, sizeof(buffer), msg, v); - printf(" CHIP:%s: %s\n", module, buffer); + printf("%s%c CHIP:%s: %s\n", category_str, task_symbol, module, buffer); assert(!wiced_rtos_check_for_stack_overflow()); } diff --git a/src/platform/CYW30739/PlatformManagerImpl.h b/src/platform/CYW30739/PlatformManagerImpl.h index 9b0523db9bd913..c10f35a8413b5f 100644 --- a/src/platform/CYW30739/PlatformManagerImpl.h +++ b/src/platform/CYW30739/PlatformManagerImpl.h @@ -40,6 +40,9 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener // the implementation methods provided by this class. friend PlatformManager; +public: + inline bool IsCurrentTask(void) { return wiced_rtos_is_current_thread(mThread) == WICED_SUCCESS; } + private: // ===== Methods that implement the PlatformManager abstract interface. @@ -92,7 +95,7 @@ inline PlatformManager & PlatformMgr(void) * Returns the platform-specific implementation of the PlatformManager singleton object. * * Chip applications can use this to gain access to features of the PlatformManager - * that are specific to the ESP32 platform. + * that are specific to the CYW30739 platform. */ inline PlatformManagerImpl & PlatformMgrImpl(void) { diff --git a/src/platform/CYW30739/ThreadStackManagerImpl.h b/src/platform/CYW30739/ThreadStackManagerImpl.h index a8068ed1fc9962..ea82a860463470 100644 --- a/src/platform/CYW30739/ThreadStackManagerImpl.h +++ b/src/platform/CYW30739/ThreadStackManagerImpl.h @@ -44,6 +44,8 @@ class ThreadStackManagerImpl final : public ThreadStackManager, // ===== Methods that implement the ThreadStackManager abstract interface. CHIP_ERROR _InitThreadStack(); + inline bool IsCurrentTask(void) { return wiced_rtos_is_current_thread(mThread) == WICED_SUCCESS; } + protected: // ===== Methods that implement the ThreadStackManager abstract interface.