Address code review comments

- Use DeviceLayer namespace directly - Return actual error from failing ConfigurationManager calls - Refactor reading of country code from OTA
project-chip · Jan 24, 2022 · 7d1ad12 · 7d1ad12
1 parent 95c5c88
commit 7d1ad12
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 21 deletions.
diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp
@@ -36,7 +36,6 @@ using namespace app::Clusters::OtaSoftwareUpdateProvider;
 using namespace app::Clusters::OtaSoftwareUpdateProvider::Commands;
 using namespace app::Clusters::OtaSoftwareUpdateRequestor;
 using namespace app::Clusters::OtaSoftwareUpdateRequestor::Commands;
-using namespace DeviceLayer;
 using app::DataModel::Nullable;
 using bdx::TransferSession;
 
@@ -393,7 +392,7 @@ void OTARequestor::NotifyUpdateApplied(uint32_t version)
 
     // Log the VersionApplied event
     uint16_t productId;
-    VerifyOrReturn(ConfigurationMgr().GetProductId(productId) == CHIP_NO_ERROR);
+    VerifyOrReturn(DeviceLayer::ConfigurationMgr().GetProductId(productId) == CHIP_NO_ERROR);
     OtaRequestorServerOnVersionApplied(version, productId);
 
     ConnectToProvider(kNotifyUpdateApplied);
@@ -492,39 +491,30 @@ CHIP_ERROR OTARequestor::SendQueryImageRequest(OperationalDeviceProxy & devicePr
     QueryImage::Type args;
 
     uint16_t vendorId;
-    VerifyOrReturnError(ConfigurationMgr().GetVendorId(vendorId) == CHIP_NO_ERROR, CHIP_ERROR_READ_FAILED);
+    ReturnErrorOnFailure(DeviceLayer::ConfigurationMgr().GetVendorId(vendorId));
     args.vendorId = static_cast<VendorId>(vendorId);
 
-    VerifyOrReturnError(ConfigurationMgr().GetProductId(args.productId) == CHIP_NO_ERROR, CHIP_ERROR_READ_FAILED);
+    ReturnErrorOnFailure(DeviceLayer::ConfigurationMgr().GetProductId(args.productId));
 
-    VerifyOrReturnError(ConfigurationMgr().GetSoftwareVersion(args.softwareVersion) == CHIP_NO_ERROR, CHIP_ERROR_READ_FAILED);
+    ReturnErrorOnFailure(DeviceLayer::ConfigurationMgr().GetSoftwareVersion(args.softwareVersion));
 
     args.protocolsSupported = kProtocolsSupported;
     args.requestorCanConsent.SetValue(mOtaRequestorDriver->CanConsent());
 
     uint16_t hardwareVersion;
-    if (ConfigurationMgr().GetHardwareVersion(hardwareVersion) == CHIP_NO_ERROR)
+    if (DeviceLayer::ConfigurationMgr().GetHardwareVersion(hardwareVersion) == CHIP_NO_ERROR)
     {
         args.hardwareVersion.SetValue(hardwareVersion);
     }
 
-    char location[DeviceLayer::ConfigurationManager::kMaxLocationLength];
+    char location[DeviceLayer::ConfigurationManager::kMaxLocationLength] = { 'X', 'X' };
+    char countryCode[DeviceLayer::ConfigurationManager::kMaxLocationLength];
     size_t codeLen = 0;
-    if (ConfigurationMgr().GetCountryCode(location, sizeof(location), codeLen) == CHIP_NO_ERROR)
+    if ((DeviceLayer::ConfigurationMgr().GetCountryCode(countryCode, sizeof(countryCode), codeLen) == CHIP_NO_ERROR) && (codeLen > 0))
     {
-        if (codeLen == 0)
-        {
-            args.location.SetValue(CharSpan("XX", DeviceLayer::ConfigurationManager::kMaxLocationLength));
-        }
-        else
-        {
-            args.location.SetValue(CharSpan(location, DeviceLayer::ConfigurationManager::kMaxLocationLength));
-        }
-    }
-    else
-    {
-        args.location.SetValue(CharSpan("XX", DeviceLayer::ConfigurationManager::kMaxLocationLength));
+        strncpy(location, countryCode, codeLen);
     }
+    args.location.SetValue(CharSpan(location, strlen(location)));
 
     Controller::OtaSoftwareUpdateProviderCluster cluster;
     cluster.Associate(&deviceProxy, mProviderEndpointId);

diff --git a/src/app/clusters/ota-requestor/OTARequestor.h b/src/app/clusters/ota-requestor/OTARequestor.h
@@ -86,7 +86,7 @@ class OTARequestor : public OTARequestorInterface, public BDXDownloader::StateDe
         mBdxDownloader      = downloader;
 
         uint32_t version;
-        VerifyOrDie(DeviceLayer::ConfigurationMgr().GetSoftwareVersion(version) == CHIP_NO_ERROR);
+        ReturnOnFailure(DeviceLayer::ConfigurationMgr().GetSoftwareVersion(version));
         mCurrentVersion = version;
 
         OtaRequestorServerSetUpdateState(mCurrentUpdateState);