From 9ca9d47e822bbf858a881946476aaee672aacde4 Mon Sep 17 00:00:00 2001 From: Kevin Schoedel <67607049+kpschoedel@users.noreply.github.com> Date: Tue, 3 Aug 2021 09:36:43 -0400 Subject: [PATCH] Make CHIP_ERROR a class type on k32w, mbed, telink (#8745) #### Problem Having CHIP_ERROR be a class type would provide (a) type safety, and (b) the ability to trace the source of errors (issue #8340). #### Change overview - Enable `CHIP_CONFIG_ERROR_CLASS` on k32w, mbed, and telink. #### Testing Existing tests should confirm no change to functionality. --- examples/lighting-app/k32w/main/AppTask.cpp | 20 ++++++------- .../k32w/main/LightingManager.cpp | 4 +-- .../lighting-app/k32w/main/include/AppTask.h | 4 +-- examples/lighting-app/mbed/main/main.cpp | 19 ++++++++----- .../lighting-app/telink/include/AppTask.h | 4 +-- .../telink/include/LightingManager.h | 4 ++- examples/lighting-app/telink/src/AppTask.cpp | 18 ++++++------ .../telink/src/LightingManager.cpp | 6 ++-- examples/lighting-app/telink/src/main.cpp | 28 +++++++++---------- examples/lock-app/k32w/main/AppTask.cpp | 20 ++++++------- .../lock-app/k32w/main/BoltLockManager.cpp | 2 +- examples/lock-app/k32w/main/include/AppTask.h | 4 +-- examples/lock-app/mbed/main/main.cpp | 19 ++++++++----- examples/shell/k32w/main/main.cpp | 6 ++-- src/platform/K32W/CHIPPlatformConfig.h | 2 ++ src/platform/mbed/CHIPPlatformConfig.h | 2 ++ src/platform/mbed/MbedConfig.cpp | 4 +-- src/platform/telink/CHIPPlatformConfig.h | 2 ++ 18 files changed, 93 insertions(+), 75 deletions(-) diff --git a/examples/lighting-app/k32w/main/AppTask.cpp b/examples/lighting-app/k32w/main/AppTask.cpp index 61c0b0b045393e..fc5cb164275ec1 100644 --- a/examples/lighting-app/k32w/main/AppTask.cpp +++ b/examples/lighting-app/k32w/main/AppTask.cpp @@ -65,9 +65,9 @@ using namespace ::chip::DeviceLayer; AppTask AppTask::sAppTask; -int AppTask::StartAppTask() +CHIP_ERROR AppTask::StartAppTask() { - int err = CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; sAppEventQueue = xQueueCreate(APP_EVENT_QUEUE_SIZE, sizeof(AppEvent)); if (sAppEventQueue == NULL) @@ -80,7 +80,7 @@ int AppTask::StartAppTask() return err; } -int AppTask::Init() +CHIP_ERROR AppTask::Init() { CHIP_ERROR err = CHIP_NO_ERROR; @@ -118,11 +118,11 @@ int AppTask::Init() assert(err == CHIP_NO_ERROR); } - err = LightingMgr().Init(); - if (err != CHIP_NO_ERROR) + int status = LightingMgr().Init(); + if (status != 0) { K32W_LOG("LightingMgr().Init() failed"); - assert(err == CHIP_NO_ERROR); + assert(status == 0); } LightingMgr().SetCallbacks(ActionInitiated, ActionCompleted); @@ -147,10 +147,9 @@ int AppTask::Init() void AppTask::AppTaskMain(void * pvParameter) { - int err; AppEvent event; - err = sAppTask.Init(); + CHIP_ERROR err = sAppTask.Init(); if (err != CHIP_NO_ERROR) { K32W_LOG("AppTask.Init() failed"); @@ -381,7 +380,7 @@ void AppTask::ResetActionEventHandler(AppEvent * aEvent) void AppTask::LightActionEventHandler(AppEvent * aEvent) { LightingManager::Action_t action; - int err = CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; int32_t actor = 0; bool initiated = false; @@ -409,7 +408,8 @@ void AppTask::LightActionEventHandler(AppEvent * aEvent) } else { - err = APP_ERROR_UNHANDLED_EVENT; + err = APP_ERROR_UNHANDLED_EVENT; + action = LightingManager::INVALID_ACTION; } if (err == CHIP_NO_ERROR) diff --git a/examples/lighting-app/k32w/main/LightingManager.cpp b/examples/lighting-app/k32w/main/LightingManager.cpp index 5cfca552007ba5..f51d2db54ce083 100644 --- a/examples/lighting-app/k32w/main/LightingManager.cpp +++ b/examples/lighting-app/k32w/main/LightingManager.cpp @@ -30,8 +30,6 @@ TimerHandle_t sLightTimer; // FreeRTOS app sw timer. int LightingManager::Init() { - int err = CHIP_NO_ERROR; - // Create FreeRTOS sw timer for light timer. sLightTimer = xTimerCreate("LightTmr", // Just a text name, not used by the RTOS kernel @@ -52,7 +50,7 @@ int LightingManager::Init() mAutoTurnOn = false; mAutoTurnOnDuration = 0; - return err; + return 0; } void LightingManager::SetCallbacks(Callback_fn_initiated aActionInitiated_CB, Callback_fn_completed aActionCompleted_CB) diff --git a/examples/lighting-app/k32w/main/include/AppTask.h b/examples/lighting-app/k32w/main/include/AppTask.h index 0ed818be7ad797..5e86c5e920c143 100644 --- a/examples/lighting-app/k32w/main/include/AppTask.h +++ b/examples/lighting-app/k32w/main/include/AppTask.h @@ -40,7 +40,7 @@ class AppTask { public: - int StartAppTask(); + CHIP_ERROR StartAppTask(); static void AppTaskMain(void * pvParameter); void PostTurnOnActionRequest(int32_t aActor, LightingManager::Action_t aAction); @@ -51,7 +51,7 @@ class AppTask private: friend AppTask & GetAppTask(void); - int Init(); + CHIP_ERROR Init(); static void ActionInitiated(LightingManager::Action_t aAction, int32_t aActor); static void ActionCompleted(LightingManager::Action_t aAction); diff --git a/examples/lighting-app/mbed/main/main.cpp b/examples/lighting-app/mbed/main/main.cpp index 199a8c70ba2279..61239298a44dab 100644 --- a/examples/lighting-app/mbed/main/main.cpp +++ b/examples/lighting-app/mbed/main/main.cpp @@ -34,7 +34,8 @@ using namespace ::chip::DeviceLayer; int main() { - int ret = 0; + int ret = 0; + CHIP_ERROR err = CHIP_NO_ERROR; #ifdef MBED_CONF_MBED_TRACE_ENABLE mbed_trace_init(); @@ -52,25 +53,29 @@ int main() goto exit; } - ret = chip::Platform::MemoryInit(); - if (ret != CHIP_NO_ERROR) + err = chip::Platform::MemoryInit(); + if (err != CHIP_NO_ERROR) { ChipLogError(NotSpecified, "Platform::MemoryInit() failed"); + ret = EXIT_FAILURE; goto exit; } ChipLogProgress(NotSpecified, "Init CHIP Stack\r\n"); - ret = PlatformMgr().InitChipStack(); - if (ret != CHIP_NO_ERROR) + err = PlatformMgr().InitChipStack(); + if (err != CHIP_NO_ERROR) { ChipLogError(NotSpecified, "PlatformMgr().InitChipStack() failed"); + ret = EXIT_FAILURE; + goto exit; } ChipLogProgress(NotSpecified, "Starting CHIP task"); - ret = PlatformMgr().StartEventLoopTask(); - if (ret != CHIP_NO_ERROR) + err = PlatformMgr().StartEventLoopTask(); + if (err != CHIP_NO_ERROR) { ChipLogError(NotSpecified, "PlatformMgr().StartEventLoopTask() failed"); + ret = EXIT_FAILURE; goto exit; } diff --git a/examples/lighting-app/telink/include/AppTask.h b/examples/lighting-app/telink/include/AppTask.h index cf8b642291dcd1..a0a07b52e88a8c 100644 --- a/examples/lighting-app/telink/include/AppTask.h +++ b/examples/lighting-app/telink/include/AppTask.h @@ -30,7 +30,7 @@ struct k_timer; class AppTask { public: - int StartApp(); + CHIP_ERROR StartApp(); void PostLightingActionRequest(LightingManager::Action_t aAction); void PostEvent(AppEvent * event); @@ -39,7 +39,7 @@ class AppTask private: friend AppTask & GetAppTask(void); - int Init(); + CHIP_ERROR Init(); static void ActionInitiated(LightingManager::Action_t aAction, int32_t aActor); static void ActionCompleted(LightingManager::Action_t aAction, int32_t aActor); diff --git a/examples/lighting-app/telink/include/LightingManager.h b/examples/lighting-app/telink/include/LightingManager.h index 2be688a8c42cf6..f184d3eca1ae3f 100644 --- a/examples/lighting-app/telink/include/LightingManager.h +++ b/examples/lighting-app/telink/include/LightingManager.h @@ -20,6 +20,8 @@ #include "AppEvent.h" +#include + #include #include @@ -43,7 +45,7 @@ class LightingManager using LightingCallback_fn = void (*)(Action_t, int32_t); - int Init(const char * pwmDeviceName, uint32_t pwmChannel); + CHIP_ERROR Init(const char * pwmDeviceName, uint32_t pwmChannel); bool IsTurnedOn() const { return mState == kState_On; } uint8_t GetLevel() const { return mLevel; } bool InitiateAction(Action_t aAction, int32_t aActor, uint8_t size, uint8_t * value); diff --git a/examples/lighting-app/telink/src/AppTask.cpp b/examples/lighting-app/telink/src/AppTask.cpp index 06fa73b75d653b..de3acec9bb948a 100644 --- a/examples/lighting-app/telink/src/AppTask.cpp +++ b/examples/lighting-app/telink/src/AppTask.cpp @@ -73,9 +73,9 @@ using namespace ::chip::DeviceLayer; AppTask AppTask::sAppTask; -int AppTask::Init() +CHIP_ERROR AppTask::Init() { - int ret; + CHIP_ERROR ret; // Initialize LEDs LEDWidget::InitGpio(); @@ -86,7 +86,7 @@ int AppTask::Init() // Init lighting manager ret = LightingMgr().Init(LIGHTING_PWM_DEVICE, LIGHTING_PWM_CHANNEL); - if (ret != 0) + if (ret != CHIP_NO_ERROR) { LOG_ERR("Failed to int lighting manager"); return ret; @@ -107,24 +107,24 @@ int AppTask::Init() return ret; } - return 0; + return CHIP_NO_ERROR; } -int AppTask::StartApp() +CHIP_ERROR AppTask::StartApp() { - int ret = Init(); + CHIP_ERROR err = Init(); - if (ret) + if (err != CHIP_NO_ERROR) { LOG_ERR("AppTask.Init() failed"); - return ret; + return err; } AppEvent event = {}; while (true) { - ret = k_msgq_get(&sAppEventQueue, &event, K_MSEC(10)); + int ret = k_msgq_get(&sAppEventQueue, &event, K_MSEC(10)); while (!ret) { diff --git a/examples/lighting-app/telink/src/LightingManager.cpp b/examples/lighting-app/telink/src/LightingManager.cpp index 9e76cca7602796..e0c550b512a89e 100644 --- a/examples/lighting-app/telink/src/LightingManager.cpp +++ b/examples/lighting-app/telink/src/LightingManager.cpp @@ -28,7 +28,7 @@ LOG_MODULE_DECLARE(app); LightingManager LightingManager::sLight; -int LightingManager::Init(const char * pwmDeviceName, uint32_t pwmChannel) +CHIP_ERROR LightingManager::Init(const char * pwmDeviceName, uint32_t pwmChannel) { // We use a gpioPin instead of a LEDWidget here because we want to use PWM // and other features instead of just on/off. @@ -41,11 +41,11 @@ int LightingManager::Init(const char * pwmDeviceName, uint32_t pwmChannel) if (!mPwmDevice) { LOG_ERR("Cannot find PWM device %s", log_strdup(pwmDeviceName)); - return -ENODEV; + return ::chip::System::MapErrorZephyr(-ENODEV); } Set(false); - return 0; + return CHIP_NO_ERROR; } void LightingManager::SetCallbacks(LightingCallback_fn aActionInitiated_CB, LightingCallback_fn aActionCompleted_CB) diff --git a/examples/lighting-app/telink/src/main.cpp b/examples/lighting-app/telink/src/main.cpp index d2c4d35c38be17..7a75416abfce79 100644 --- a/examples/lighting-app/telink/src/main.cpp +++ b/examples/lighting-app/telink/src/main.cpp @@ -31,49 +31,49 @@ using namespace ::chip::DeviceLayer; int main(void) { - int ret = 0; + CHIP_ERROR err = CHIP_NO_ERROR; - ret = chip::Platform::MemoryInit(); - if (ret != CHIP_NO_ERROR) + err = chip::Platform::MemoryInit(); + if (err != CHIP_NO_ERROR) { LOG_ERR("Platform::MemoryInit() failed"); goto exit; } LOG_INF("Init CHIP stack"); - ret = PlatformMgr().InitChipStack(); - if (ret != CHIP_NO_ERROR) + err = PlatformMgr().InitChipStack(); + if (err != CHIP_NO_ERROR) { LOG_ERR("PlatformMgr().InitChipStack() failed"); goto exit; } LOG_INF("Starting CHIP task"); - ret = PlatformMgr().StartEventLoopTask(); - if (ret != CHIP_NO_ERROR) + err = PlatformMgr().StartEventLoopTask(); + if (err != CHIP_NO_ERROR) { LOG_ERR("PlatformMgr().StartEventLoopTask() failed"); goto exit; } LOG_INF("Init Thread stack"); - ret = ThreadStackMgr().InitThreadStack(); - if (ret != CHIP_NO_ERROR) + err = ThreadStackMgr().InitThreadStack(); + if (err != CHIP_NO_ERROR) { LOG_ERR("ThreadStackMgr().InitThreadStack() failed"); goto exit; } - ret = ConnectivityMgr().SetThreadDeviceType(ConnectivityManager::kThreadDeviceType_MinimalEndDevice); - if (ret != CHIP_NO_ERROR) + err = ConnectivityMgr().SetThreadDeviceType(ConnectivityManager::kThreadDeviceType_MinimalEndDevice); + if (err != CHIP_NO_ERROR) { LOG_ERR("ConnectivityMgr().SetThreadDeviceType() failed"); goto exit; } - ret = GetAppTask().StartApp(); + err = GetAppTask().StartApp(); exit: - LOG_ERR("Exited with code %d", ret); - return ret; + LOG_ERR("Exited with code %" CHIP_ERROR_FORMAT, ChipError::FormatError(err)); + return (err == CHIP_NO_ERROR) ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/examples/lock-app/k32w/main/AppTask.cpp b/examples/lock-app/k32w/main/AppTask.cpp index bc28e93a4904a0..cad8371f800d27 100644 --- a/examples/lock-app/k32w/main/AppTask.cpp +++ b/examples/lock-app/k32w/main/AppTask.cpp @@ -63,9 +63,9 @@ using namespace ::chip::DeviceLayer; AppTask AppTask::sAppTask; -int AppTask::StartAppTask() +CHIP_ERROR AppTask::StartAppTask() { - int err = CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; sAppEventQueue = xQueueCreate(kAppEventQueueSize, sizeof(AppEvent)); if (sAppEventQueue == NULL) @@ -77,7 +77,7 @@ int AppTask::StartAppTask() return err; } -int AppTask::Init() +CHIP_ERROR AppTask::Init() { CHIP_ERROR err = CHIP_NO_ERROR; @@ -114,11 +114,11 @@ int AppTask::Init() assert(err == CHIP_NO_ERROR); } - err = BoltLockMgr().Init(); - if (err != CHIP_NO_ERROR) + int status = BoltLockMgr().Init(); + if (status != 0) { K32W_LOG("BoltLockMgr().Init() failed"); - assert(err == CHIP_NO_ERROR); + assert(status == 0); } BoltLockMgr().SetCallbacks(ActionInitiated, ActionCompleted); @@ -150,10 +150,9 @@ int AppTask::Init() void AppTask::AppTaskMain(void * pvParameter) { - int err; AppEvent event; - err = sAppTask.Init(); + CHIP_ERROR err = sAppTask.Init(); if (err != CHIP_NO_ERROR) { K32W_LOG("AppTask.Init() failed"); @@ -385,7 +384,7 @@ void AppTask::ResetActionEventHandler(AppEvent * aEvent) void AppTask::LockActionEventHandler(AppEvent * aEvent) { BoltLockManager::Action_t action; - int err = CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; int32_t actor = 0; bool initiated = false; @@ -413,7 +412,8 @@ void AppTask::LockActionEventHandler(AppEvent * aEvent) } else { - err = CHIP_ERROR_INTERNAL; + err = CHIP_ERROR_INTERNAL; + action = BoltLockManager::INVALID_ACTION; } if (err == CHIP_NO_ERROR) diff --git a/examples/lock-app/k32w/main/BoltLockManager.cpp b/examples/lock-app/k32w/main/BoltLockManager.cpp index 7100d73eeedf81..8eeb041c14b79d 100644 --- a/examples/lock-app/k32w/main/BoltLockManager.cpp +++ b/examples/lock-app/k32w/main/BoltLockManager.cpp @@ -30,7 +30,7 @@ TimerHandle_t sLockTimer; // FreeRTOS app sw timer. int BoltLockManager::Init() { - int err = CHIP_NO_ERROR; + int err = 0; // Create FreeRTOS sw timer for Lock timer. diff --git a/examples/lock-app/k32w/main/include/AppTask.h b/examples/lock-app/k32w/main/include/AppTask.h index de732379d2eb47..82e73499cff2c1 100644 --- a/examples/lock-app/k32w/main/include/AppTask.h +++ b/examples/lock-app/k32w/main/include/AppTask.h @@ -32,7 +32,7 @@ class AppTask { public: - int StartAppTask(); + CHIP_ERROR StartAppTask(); static void AppTaskMain(void * pvParameter); void PostLockActionRequest(int32_t aActor, BoltLockManager::Action_t aAction); @@ -43,7 +43,7 @@ class AppTask private: friend AppTask & GetAppTask(void); - int Init(); + CHIP_ERROR Init(); static void ActionInitiated(BoltLockManager::Action_t aAction, int32_t aActor); static void ActionCompleted(BoltLockManager::Action_t aAction); diff --git a/examples/lock-app/mbed/main/main.cpp b/examples/lock-app/mbed/main/main.cpp index 199a8c70ba2279..61239298a44dab 100644 --- a/examples/lock-app/mbed/main/main.cpp +++ b/examples/lock-app/mbed/main/main.cpp @@ -34,7 +34,8 @@ using namespace ::chip::DeviceLayer; int main() { - int ret = 0; + int ret = 0; + CHIP_ERROR err = CHIP_NO_ERROR; #ifdef MBED_CONF_MBED_TRACE_ENABLE mbed_trace_init(); @@ -52,25 +53,29 @@ int main() goto exit; } - ret = chip::Platform::MemoryInit(); - if (ret != CHIP_NO_ERROR) + err = chip::Platform::MemoryInit(); + if (err != CHIP_NO_ERROR) { ChipLogError(NotSpecified, "Platform::MemoryInit() failed"); + ret = EXIT_FAILURE; goto exit; } ChipLogProgress(NotSpecified, "Init CHIP Stack\r\n"); - ret = PlatformMgr().InitChipStack(); - if (ret != CHIP_NO_ERROR) + err = PlatformMgr().InitChipStack(); + if (err != CHIP_NO_ERROR) { ChipLogError(NotSpecified, "PlatformMgr().InitChipStack() failed"); + ret = EXIT_FAILURE; + goto exit; } ChipLogProgress(NotSpecified, "Starting CHIP task"); - ret = PlatformMgr().StartEventLoopTask(); - if (ret != CHIP_NO_ERROR) + err = PlatformMgr().StartEventLoopTask(); + if (err != CHIP_NO_ERROR) { ChipLogError(NotSpecified, "PlatformMgr().StartEventLoopTask() failed"); + ret = EXIT_FAILURE; goto exit; } diff --git a/examples/shell/k32w/main/main.cpp b/examples/shell/k32w/main/main.cpp index 83ab4b42d80461..1abbc859afacff 100644 --- a/examples/shell/k32w/main/main.cpp +++ b/examples/shell/k32w/main/main.cpp @@ -64,6 +64,8 @@ unsigned int sleep(unsigned int seconds) extern "C" void main_task(void const * argument) { + int status = 0; + /* Call C++ constructors */ InitFunc * pFunc = &__init_array_start; for (; pFunc < &__init_array_end; ++pFunc) @@ -120,8 +122,8 @@ extern "C" void main_task(void const * argument) goto exit; } - ret = chip::Shell::streamer_init(chip::Shell::streamer_get()); - if (ret != 0) + status = chip::Shell::streamer_init(chip::Shell::streamer_get()); + if (status != 0) { K32W_LOG("Error during streamer_init"); goto exit; diff --git a/src/platform/K32W/CHIPPlatformConfig.h b/src/platform/K32W/CHIPPlatformConfig.h index 1ca4f84a858d4d..e8fb73d71437cc 100644 --- a/src/platform/K32W/CHIPPlatformConfig.h +++ b/src/platform/K32W/CHIPPlatformConfig.h @@ -41,6 +41,8 @@ #define CHIP_CONFIG_TIME_ENABLE_CLIENT 1 #define CHIP_CONFIG_TIME_ENABLE_SERVER 0 +#define CHIP_CONFIG_ERROR_CLASS 1 + // ==================== Security Adaptations ==================== #define CHIP_CONFIG_USE_OPENSSL_ECC 0 diff --git a/src/platform/mbed/CHIPPlatformConfig.h b/src/platform/mbed/CHIPPlatformConfig.h index 2818ee14aab3c6..b0d74d3119688e 100644 --- a/src/platform/mbed/CHIPPlatformConfig.h +++ b/src/platform/mbed/CHIPPlatformConfig.h @@ -37,6 +37,8 @@ #define CHIP_CONFIG_TIME_ENABLE_CLIENT 1 #define CHIP_CONFIG_TIME_ENABLE_SERVER 0 +#define CHIP_CONFIG_ERROR_CLASS 1 + // ==================== Security Adaptations ==================== #define CHIP_CONFIG_USE_OPENSSL_ECC 0 diff --git a/src/platform/mbed/MbedConfig.cpp b/src/platform/mbed/MbedConfig.cpp index bb519cbf79033e..d721bfec86d917 100644 --- a/src/platform/mbed/MbedConfig.cpp +++ b/src/platform/mbed/MbedConfig.cpp @@ -282,7 +282,7 @@ CHIP_ERROR MbedConfig::ReadCounter(Key counterId, uint32_t & value) { char key[50] = { 0 }; auto err = ConstructCounterKey(counterId, key, sizeof(key)); - if (err) + if (err != CHIP_NO_ERROR) { return err; } @@ -294,7 +294,7 @@ CHIP_ERROR MbedConfig::WriteCounter(Key counterId, uint32_t value) { char key[50] = { 0 }; auto err = ConstructCounterKey(counterId, key, sizeof(key)); - if (err) + if (err != CHIP_NO_ERROR) { return err; } diff --git a/src/platform/telink/CHIPPlatformConfig.h b/src/platform/telink/CHIPPlatformConfig.h index 620fe9e3a37cd3..37945a0a435157 100644 --- a/src/platform/telink/CHIPPlatformConfig.h +++ b/src/platform/telink/CHIPPlatformConfig.h @@ -33,6 +33,8 @@ #define CHIP_CONFIG_TIME_ENABLE_CLIENT 1 #define CHIP_CONFIG_TIME_ENABLE_SERVER 0 +#define CHIP_CONFIG_ERROR_CLASS 1 + // ==================== Security Adaptations ==================== #define CHIP_CONFIG_USE_OPENSSL_ECC 0