From cb32df22fb1b68c65756148c7b3b624aa831fcce Mon Sep 17 00:00:00 2001 From: Kevin Schoedel Date: Wed, 21 Jul 2021 18:11:05 -0400 Subject: [PATCH] Make CHIP_ERROR a class type on nrfconnect #### 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 nrfconnect. #### Testing Existing tests should confirm no change to functionality. --- .../lighting-app/nrfconnect/main/main.cpp | 33 ++++++---- examples/lock-app/nrfconnect/main/main.cpp | 31 ++++++---- examples/shell/shell_common/cmd_otcli.cpp | 2 +- ...nericThreadStackManagerImpl_OpenThread.cpp | 5 +- src/platform/Zephyr/BLEManagerImpl.cpp | 61 ++++++++----------- src/platform/nrfconnect/CHIPPlatformConfig.h | 2 + 6 files changed, 68 insertions(+), 66 deletions(-) diff --git a/examples/lighting-app/nrfconnect/main/main.cpp b/examples/lighting-app/nrfconnect/main/main.cpp index d382ed2f71c2f0..6edfb06939199e 100644 --- a/examples/lighting-app/nrfconnect/main/main.cpp +++ b/examples/lighting-app/nrfconnect/main/main.cpp @@ -21,6 +21,7 @@ #include #include +#include #include @@ -40,58 +41,64 @@ int main(void) chip::rpc::Init(); #endif - int ret = 0; + int ret = 0; + CHIP_ERROR err = CHIP_NO_ERROR; #ifdef CONFIG_USB ret = usb_enable(nullptr); if (ret) { LOG_ERR("Failed to initialize USB device"); + err = chip::System::MapErrorZephyr(ret); goto exit; } #endif - 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(); + if (ret != 0) + { + err = chip::System::MapErrorZephyr(ret); + } exit: - LOG_ERR("Exited with code %d", ret); - return ret; + LOG_ERR("Exited with code %" CHIP_ERROR_FORMAT, chip::ChipError::FormatError(err)); + return err == CHIP_NO_ERROR ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/examples/lock-app/nrfconnect/main/main.cpp b/examples/lock-app/nrfconnect/main/main.cpp index 5902f9f0e4dee1..6c12a703b0d39e 100644 --- a/examples/lock-app/nrfconnect/main/main.cpp +++ b/examples/lock-app/nrfconnect/main/main.cpp @@ -32,49 +32,54 @@ using namespace ::chip::DeviceLayer; int main() { - int ret = 0; + 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(); + if (ret != 0) + { + err = chip::System::MapErrorZephyr(ret); + } exit: - LOG_ERR("Exited with code %d", ret); - return ret; + LOG_ERR("Exited with code %" CHIP_ERROR_FORMAT, chip::ChipError::FormatError(err)); + return err == CHIP_NO_ERROR ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/examples/shell/shell_common/cmd_otcli.cpp b/examples/shell/shell_common/cmd_otcli.cpp index 8dbf9b6066f660..985a6159dd0960 100644 --- a/examples/shell/shell_common/cmd_otcli.cpp +++ b/examples/shell/shell_common/cmd_otcli.cpp @@ -164,7 +164,7 @@ static const shell_command_t cmds_otcli_root = { &cmd_otcli_dispatch, "otcli", " static int OnOtCliOutput(void * aContext, const char * aFormat, va_list aArguments) { int rval = vsnprintf(sTxBuffer, sTxLength, aFormat, aArguments); - VerifyOrExit(rval >= 0 && rval < sTxLength, rval = CHIP_ERROR_BUFFER_TOO_SMALL); + VerifyOrExit(rval >= 0 && rval < sTxLength, rval = ChipError::AsInteger(CHIP_ERROR_BUFFER_TOO_SMALL)); return streamer_write(streamer_get(), (const char *) sTxBuffer, rval); exit: return rval; diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp index 400458bcdd8cc1..738ac022b5cd4c 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp @@ -1429,7 +1429,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_DnsBrowse(const char fullServiceName[chip::Mdns::kMdnsTypeAndProtocolMaxSize + 1 + SrpClient::kDefaultDomainNameSize + 1]; snprintf(fullServiceName, sizeof(fullServiceName), "%s.%s", aServiceName, SrpClient::kDefaultDomainName); - error = otDnsClientBrowse(mOTInst, fullServiceName, OnDnsBrowseResult, aContext, defaultConfig); + error = MapOpenThreadError(otDnsClientBrowse(mOTInst, fullServiceName, OnDnsBrowseResult, aContext, defaultConfig)); exit: @@ -1503,7 +1503,8 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_DnsResolve(cons char fullServiceName[chip::Mdns::kMdnsTypeAndProtocolMaxSize + 1 + SrpClient::kDefaultDomainNameSize + 1]; snprintf(fullServiceName, sizeof(fullServiceName), "%s.%s", aServiceName, SrpClient::kDefaultDomainName); - error = otDnsClientResolveService(mOTInst, aInstanceName, fullServiceName, OnDnsResolveResult, aContext, defaultConfig); + error = MapOpenThreadError( + otDnsClientResolveService(mOTInst, aInstanceName, fullServiceName, OnDnsResolveResult, aContext, defaultConfig)); exit: diff --git a/src/platform/Zephyr/BLEManagerImpl.cpp b/src/platform/Zephyr/BLEManagerImpl.cpp index 87e9c4a9e07c78..8dcad3a2ba6dc0 100644 --- a/src/platform/Zephyr/BLEManagerImpl.cpp +++ b/src/platform/Zephyr/BLEManagerImpl.cpp @@ -108,8 +108,6 @@ BLEManagerImpl BLEManagerImpl::sInstance; CHIP_ERROR BLEManagerImpl::_Init() { - CHIP_ERROR err; - mServiceMode = ConnectivityManager::kCHIPoBLEServiceMode_Enabled; mFlags.ClearAll().Set(Flags::kAdvertisingEnabled, CHIP_DEVICE_CONFIG_CHIPOBLE_ENABLE_ADVERTISING_AUTOSTART); mFlags.Set(Flags::kFastAdvertisingEnabled, true); @@ -118,8 +116,8 @@ CHIP_ERROR BLEManagerImpl::_Init() memset(mSubscribedConns, 0, sizeof(mSubscribedConns)); InitRandomStaticAddress(); - err = bt_enable(NULL); - VerifyOrExit(err == CHIP_NO_ERROR, err = MapErrorZephyr(err)); + int err = bt_enable(NULL); + VerifyOrReturnError(err == 0, MapErrorZephyr(err)); memset(&mConnCallbacks, 0, sizeof(mConnCallbacks)); mConnCallbacks.connected = HandleConnect; @@ -128,13 +126,11 @@ CHIP_ERROR BLEManagerImpl::_Init() bt_conn_cb_register(&mConnCallbacks); // Initialize the CHIP BleLayer. - err = BleLayer::Init(this, this, &SystemLayer); - SuccessOrExit(err); + ReturnErrorOnFailure(BleLayer::Init(this, this, &SystemLayer)); PlatformMgr().ScheduleWork(DriveBLEState, 0); -exit: - return err; + return CHIP_NO_ERROR; } void BLEManagerImpl::DriveBLEState(intptr_t arg) @@ -205,8 +201,7 @@ struct BLEManagerImpl::ServiceData CHIP_ERROR BLEManagerImpl::StartAdvertising(void) { - CHIP_ERROR err; - + int err = 0; const char * deviceName = bt_get_name(); const uint8_t advFlags = BT_LE_AD_GENERAL | BT_LE_AD_NO_BREDR; const bool isAdvertisingRerun = mFlags.Has(Flags::kAdvertising); @@ -229,8 +224,7 @@ CHIP_ERROR BLEManagerImpl::StartAdvertising(void) // Initialize service data static_assert(sizeof(serviceData) == 9, "Size of BLE advertisement data changed! Was that intentional?"); chip::Encoding::LittleEndian::Put16(serviceData.uuid, UUID16_CHIPoBLEService.val); - err = ConfigurationMgr().GetBLEDeviceIdentificationInfo(serviceData.deviceIdInfo); - SuccessOrExit(err); + ReturnErrorOnFailure(ConfigurationMgr().GetBLEDeviceIdentificationInfo(serviceData.deviceIdInfo)); if (!isAdvertisingRerun) { @@ -240,16 +234,16 @@ CHIP_ERROR BLEManagerImpl::StartAdvertising(void) // Generate new private BLE address bt_le_oob bleOobInfo; err = bt_le_oob_get_local(advParams.id, &bleOobInfo); - VerifyOrExit(err == CHIP_NO_ERROR, err = MapErrorZephyr(err)); + VerifyOrReturnError(err == 0, MapErrorZephyr(err)); #endif // CONFIG_BT_PRIVACY } // Restart advertising err = bt_le_adv_stop(); - VerifyOrExit(err == CHIP_NO_ERROR, err = MapErrorZephyr(err)); + VerifyOrReturnError(err == 0, MapErrorZephyr(err)); err = bt_le_adv_start(&advParams, ad, ARRAY_SIZE(ad), nullptr, 0u); - VerifyOrExit(err == CHIP_NO_ERROR, err = MapErrorZephyr(err)); + VerifyOrReturnError(err == 0, MapErrorZephyr(err)); // Transition to the Advertising state... if (!mFlags.Has(Flags::kAdvertising)) @@ -281,16 +275,13 @@ CHIP_ERROR BLEManagerImpl::StartAdvertising(void) } } -exit: - return err; + return CHIP_NO_ERROR; } CHIP_ERROR BLEManagerImpl::StopAdvertising(void) { - CHIP_ERROR err = CHIP_NO_ERROR; - - err = bt_le_adv_stop(); - VerifyOrExit(err == CHIP_NO_ERROR, err = MapErrorZephyr(err)); + int err = bt_le_adv_stop(); + VerifyOrReturnError(err == 0, MapErrorZephyr(err)); // Transition to the not Advertising state... if (mFlags.Has(Flags::kAdvertising)) @@ -315,16 +306,14 @@ CHIP_ERROR BLEManagerImpl::StopAdvertising(void) SystemLayer.CancelTimer(HandleBLEAdvertisementIntervalChange, this); } -exit: - return err; + return CHIP_NO_ERROR; } CHIP_ERROR BLEManagerImpl::_SetCHIPoBLEServiceMode(CHIPoBLEServiceMode val) { - CHIP_ERROR err = CHIP_NO_ERROR; - - VerifyOrExit(val != ConnectivityManager::kCHIPoBLEServiceMode_NotSupported, err = CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrExit(mServiceMode != ConnectivityManager::kCHIPoBLEServiceMode_NotSupported, err = CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE); + VerifyOrReturnError(val != ConnectivityManager::kCHIPoBLEServiceMode_NotSupported, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(mServiceMode != ConnectivityManager::kCHIPoBLEServiceMode_NotSupported, + CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE); if (val != mServiceMode) { @@ -332,15 +321,13 @@ CHIP_ERROR BLEManagerImpl::_SetCHIPoBLEServiceMode(CHIPoBLEServiceMode val) PlatformMgr().ScheduleWork(DriveBLEState, 0); } -exit: - return err; + return CHIP_NO_ERROR; } CHIP_ERROR BLEManagerImpl::_SetAdvertisingEnabled(bool val) { - CHIP_ERROR err = CHIP_NO_ERROR; - - VerifyOrExit(mServiceMode != ConnectivityManager::kCHIPoBLEServiceMode_NotSupported, err = CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE); + VerifyOrReturnError(mServiceMode != ConnectivityManager::kCHIPoBLEServiceMode_NotSupported, + CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE); if (mFlags.Has(Flags::kAdvertisingEnabled) != val) { @@ -350,8 +337,7 @@ CHIP_ERROR BLEManagerImpl::_SetAdvertisingEnabled(bool val) PlatformMgr().ScheduleWork(DriveBLEState, 0); } -exit: - return err; + return CHIP_NO_ERROR; } CHIP_ERROR BLEManagerImpl::_SetAdvertisingMode(BLEAdvertisingMode mode) @@ -604,7 +590,7 @@ uint16_t BLEManagerImpl::_NumConnections(void) bool BLEManagerImpl::CloseConnection(BLE_CONNECTION_OBJECT conId) { ChipLogProgress(DeviceLayer, "Closing BLE GATT connection (ConnId %02" PRIx16 ")", bt_conn_index(conId)); - return bt_conn_disconnect(conId, BT_HCI_ERR_REMOTE_USER_TERM_CONN) == CHIP_NO_ERROR; + return bt_conn_disconnect(conId, BT_HCI_ERR_REMOTE_USER_TERM_CONN) == 0; } uint16_t BLEManagerImpl::GetMTU(BLE_CONNECTION_OBJECT conId) const @@ -628,6 +614,7 @@ bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUU PacketBufferHandle pBuf) { CHIP_ERROR err = CHIP_NO_ERROR; + int status = 0; uint8_t index = bt_conn_index(conId); bt_gatt_notify_params * params = &mNotifyParams[index]; @@ -642,8 +629,8 @@ bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUU params->func = HandleTXCompleted; params->user_data = nullptr; - err = bt_gatt_notify_cb(conId, params); - VerifyOrExit(err == CHIP_NO_ERROR, err = MapErrorZephyr(err)); + status = bt_gatt_notify_cb(conId, params); + VerifyOrExit(status == 0, err = MapErrorZephyr(status)); exit: if (err != CHIP_NO_ERROR) diff --git a/src/platform/nrfconnect/CHIPPlatformConfig.h b/src/platform/nrfconnect/CHIPPlatformConfig.h index 9b9639c4b22f08..607ba16737f5e6 100644 --- a/src/platform/nrfconnect/CHIPPlatformConfig.h +++ b/src/platform/nrfconnect/CHIPPlatformConfig.h @@ -36,6 +36,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