Replace GetRegulatoryConfig with GetRegulatoryLocation to avoid incon…

…sistent result (#12827)
project-chip · Nov 13, 2023 · 1257600 · 1257600
1 parent 31e021c
commit 1257600
Show file tree

Hide file tree

Showing 11 changed files with 39 additions and 61 deletions.
diff --git a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp
@@ -67,7 +67,7 @@ CHIP_ERROR GeneralCommissioningAttrAccess::Read(const ConcreteReadAttributePath
     switch (aPath.mAttributeId)
     {
     case RegulatoryConfig::Id: {
-        return ReadIfSupported(&ConfigurationManager::GetRegulatoryConfig, aEncoder);
+        return ReadIfSupported(&ConfigurationManager::GetRegulatoryLocation, aEncoder);
     }
     case LocationCapability::Id: {
         return ReadIfSupported(&ConfigurationManager::GetLocationCapability, aEncoder);

diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp
@@ -1820,7 +1820,7 @@ void DeviceCommissioner::AdvanceCommissioningStage(CHIP_ERROR err)
         // TODO(cecille): Worthwhile to keep this around as part of the class?
         // TODO(cecille): Where is the country config actually set?
         ChipLogProgress(Controller, "Setting Regulatory Config");
-        uint32_t regulatoryLocation = EMBER_ZCL_REGULATORY_LOCATION_TYPE_OUTDOOR;
+        uint8_t regulatoryLocation = EMBER_ZCL_REGULATORY_LOCATION_TYPE_OUTDOOR;
 #if CONFIG_DEVICE_LAYER
         CHIP_ERROR status = DeviceLayer::ConfigurationMgr().GetRegulatoryLocation(regulatoryLocation);
 #else
@@ -1848,8 +1848,8 @@ void DeviceCommissioner::AdvanceCommissioningStage(CHIP_ERROR err)
 
         GeneralCommissioningCluster genCom;
         genCom.Associate(mDeviceBeingCommissioned, 0);
-        genCom.SetRegulatoryConfig(mSuccess.Cancel(), mFailure.Cancel(), static_cast<uint8_t>(regulatoryLocation), countryCode,
-                                   breadcrumb, kCommandTimeoutMs);
+        genCom.SetRegulatoryConfig(mSuccess.Cancel(), mFailure.Cancel(), regulatoryLocation, countryCode, breadcrumb,
+                                   kCommandTimeoutMs);
     }
     break;
     case CommissioningStage::kDeviceAttestation: {

diff --git a/src/include/platform/ConfigurationManager.h b/src/include/platform/ConfigurationManager.h
@@ -97,7 +97,7 @@ class ConfigurationManager
     virtual CHIP_ERROR GetSetupDiscriminator(uint16_t & setupDiscriminator)                         = 0;
     // Lifetime counter is monotonic counter that is incremented only in the case of a factory reset
     virtual CHIP_ERROR GetLifetimeCounter(uint16_t & lifetimeCounter)                  = 0;
-    virtual CHIP_ERROR GetRegulatoryLocation(uint32_t & location)                      = 0;
+    virtual CHIP_ERROR GetRegulatoryLocation(uint8_t & location)                       = 0;
     virtual CHIP_ERROR GetCountryCode(char * buf, size_t bufSize, size_t & codeLen)    = 0;
     virtual CHIP_ERROR GetBreadcrumb(uint64_t & breadcrumb)                            = 0;
     virtual CHIP_ERROR StoreSerialNumber(const char * serialNum, size_t serialNumLen)  = 0;
@@ -107,7 +107,7 @@ class ConfigurationManager
     virtual CHIP_ERROR StoreHardwareVersion(uint16_t hardwareVer)                      = 0;
     virtual CHIP_ERROR StoreSetupPinCode(uint32_t setupPinCode)                        = 0;
     virtual CHIP_ERROR StoreSetupDiscriminator(uint16_t setupDiscriminator)            = 0;
-    virtual CHIP_ERROR StoreRegulatoryLocation(uint32_t location)                      = 0;
+    virtual CHIP_ERROR StoreRegulatoryLocation(uint8_t location)                       = 0;
     virtual CHIP_ERROR StoreCountryCode(const char * code, size_t codeLen)             = 0;
     virtual CHIP_ERROR StoreBreadcrumb(uint64_t breadcrumb)                            = 0;
     virtual CHIP_ERROR GetRebootCount(uint32_t & rebootCount)                          = 0;
@@ -143,7 +143,6 @@ class ConfigurationManager
     virtual CHIP_ERROR GetSecondaryPairingHint(uint16_t & pairingHint)            = 0;
     virtual CHIP_ERROR GetSecondaryPairingInstruction(char * buf, size_t bufSize) = 0;
 
-    virtual CHIP_ERROR GetRegulatoryConfig(uint8_t & location);
     virtual CHIP_ERROR GetLocationCapability(uint8_t & location);
 
 protected:
@@ -190,11 +189,6 @@ extern ConfigurationManager & ConfigurationMgr();
  */
 extern void SetConfigurationMgr(ConfigurationManager * configurationManager);
 
-inline CHIP_ERROR ConfigurationManager::GetRegulatoryConfig(uint8_t & location)
-{
-    return GetLocationCapability(location);
-}
-
 inline CHIP_ERROR ConfigurationManager::GetLocationCapability(uint8_t & location)
 {
     location = EMBER_ZCL_REGULATORY_LOCATION_TYPE_INDOOR;

diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.cpp b/src/include/platform/internal/GenericConfigurationManagerImpl.cpp
@@ -309,15 +309,16 @@ CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreSetupDiscriminator
 }
 
 template <class ConfigClass>
-CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::GetRegulatoryLocation(uint32_t & location)
+CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::GetRegulatoryLocation(uint8_t & location)
 {
-    return ReadConfigValue(ConfigClass::kConfigKey_RegulatoryLocation, location);
+    return GetLocationCapability(location);
 }
 
 template <class ConfigClass>
-CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreRegulatoryLocation(uint32_t location)
+CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreRegulatoryLocation(uint8_t location)
 {
-    return WriteConfigValue(ConfigClass::kConfigKey_RegulatoryLocation, location);
+    uint32_t value = location;
+    return WriteConfigValue(ConfigClass::kConfigKey_RegulatoryLocation, value);
 }
 
 template <class ConfigClass>

diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.h b/src/include/platform/internal/GenericConfigurationManagerImpl.h
@@ -88,8 +88,8 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
     CHIP_ERROR GetInitialPairingInstruction(char * buf, size_t bufSize) override;
     CHIP_ERROR GetSecondaryPairingHint(uint16_t & pairingHint) override;
     CHIP_ERROR GetSecondaryPairingInstruction(char * buf, size_t bufSize) override;
-    CHIP_ERROR GetRegulatoryLocation(uint32_t & location) override;
-    CHIP_ERROR StoreRegulatoryLocation(uint32_t location) override;
+    CHIP_ERROR GetRegulatoryLocation(uint8_t & location) override;
+    CHIP_ERROR StoreRegulatoryLocation(uint8_t location) override;
     CHIP_ERROR GetCountryCode(char * buf, size_t bufSize, size_t & codeLen) override;
     CHIP_ERROR StoreCountryCode(const char * code, size_t codeLen) override;
     CHIP_ERROR GetBreadcrumb(uint64_t & breadcrumb) override;

diff --git a/src/platform/Linux/ConfigurationManagerImpl.cpp b/src/platform/Linux/ConfigurationManagerImpl.cpp
@@ -92,17 +92,17 @@ CHIP_ERROR ConfigurationManagerImpl::Init()
         SuccessOrExit(err);
     }
 
-    if (!PosixConfig::ConfigValueExists(PosixConfig::kCounterKey_RegulatoryConfig))
+    if (!PosixConfig::ConfigValueExists(PosixConfig::kConfigKey_RegulatoryLocation))
     {
-        uint32_t location = EMBER_ZCL_REGULATORY_LOCATION_TYPE_INDOOR_OUTDOOR;
-        err               = WriteConfigValue(PosixConfig::kCounterKey_RegulatoryConfig, location);
+        uint32_t location = EMBER_ZCL_REGULATORY_LOCATION_TYPE_INDOOR;
+        err               = WriteConfigValue(PosixConfig::kConfigKey_RegulatoryLocation, location);
         SuccessOrExit(err);
     }
 
-    if (!PosixConfig::ConfigValueExists(PosixConfig::kCounterKey_LocationCapability))
+    if (!PosixConfig::ConfigValueExists(PosixConfig::kConfigKey_LocationCapability))
     {
-        uint32_t location = EMBER_ZCL_REGULATORY_LOCATION_TYPE_INDOOR_OUTDOOR;
-        err               = WriteConfigValue(PosixConfig::kCounterKey_LocationCapability, location);
+        uint32_t location = EMBER_ZCL_REGULATORY_LOCATION_TYPE_INDOOR;
+        err               = WriteConfigValue(PosixConfig::kConfigKey_LocationCapability, location);
         SuccessOrExit(err);
     }
 
@@ -335,11 +335,11 @@ CHIP_ERROR ConfigurationManagerImpl::StoreBootReason(uint32_t bootReason)
     return WriteConfigValue(PosixConfig::kCounterKey_BootReason, bootReason);
 }
 
-CHIP_ERROR ConfigurationManagerImpl::GetRegulatoryConfig(uint8_t & location)
+CHIP_ERROR ConfigurationManagerImpl::GetRegulatoryLocation(uint8_t & location)
 {
     uint32_t value = 0;
 
-    CHIP_ERROR err = ReadConfigValue(PosixConfig::kCounterKey_RegulatoryConfig, value);
+    CHIP_ERROR err = ReadConfigValue(PosixConfig::kConfigKey_RegulatoryLocation, value);
 
     if (err == CHIP_NO_ERROR)
     {
@@ -354,7 +354,7 @@ CHIP_ERROR ConfigurationManagerImpl::GetLocationCapability(uint8_t & location)
 {
     uint32_t value = 0;
 
-    CHIP_ERROR err = ReadConfigValue(PosixConfig::kCounterKey_LocationCapability, value);
+    CHIP_ERROR err = ReadConfigValue(PosixConfig::kConfigKey_LocationCapability, value);
 
     if (err == CHIP_NO_ERROR)
     {

diff --git a/src/platform/Linux/ConfigurationManagerImpl.h b/src/platform/Linux/ConfigurationManagerImpl.h
@@ -43,7 +43,7 @@ class ConfigurationManagerImpl : public Internal::GenericConfigurationManagerImp
     CHIP_ERROR StoreTotalOperationalHours(uint32_t totalOperationalHours) override;
     CHIP_ERROR GetBootReason(uint32_t & bootReason) override;
     CHIP_ERROR StoreBootReason(uint32_t bootReason) override;
-    CHIP_ERROR GetRegulatoryConfig(uint8_t & location) override;
+    CHIP_ERROR GetRegulatoryLocation(uint8_t & location) override;
     CHIP_ERROR GetLocationCapability(uint8_t & location) override;
     static ConfigurationManagerImpl & GetDefaultInstance();
 

diff --git a/src/platform/Linux/PosixConfig.cpp b/src/platform/Linux/PosixConfig.cpp
@@ -59,20 +59,19 @@ const PosixConfig::Key PosixConfig::kConfigKey_SetupPinCode        = { kConfigNa
 const PosixConfig::Key PosixConfig::kConfigKey_SetupDiscriminator  = { kConfigNamespace_ChipFactory, "discriminator" };
 
 // Keys stored in the Chip-config namespace
-const PosixConfig::Key PosixConfig::kConfigKey_FabricId            = { kConfigNamespace_ChipConfig, "fabric-id" };
-const PosixConfig::Key PosixConfig::kConfigKey_ServiceConfig       = { kConfigNamespace_ChipConfig, "service-config" };
-const PosixConfig::Key PosixConfig::kConfigKey_PairedAccountId     = { kConfigNamespace_ChipConfig, "account-id" };
-const PosixConfig::Key PosixConfig::kConfigKey_ServiceId           = { kConfigNamespace_ChipConfig, "service-id" };
-const PosixConfig::Key PosixConfig::kConfigKey_FabricSecret        = { kConfigNamespace_ChipConfig, "fabric-secret" };
-const PosixConfig::Key PosixConfig::kConfigKey_GroupKeyIndex       = { kConfigNamespace_ChipConfig, "group-key-index" };
-const PosixConfig::Key PosixConfig::kConfigKey_LastUsedEpochKeyId  = { kConfigNamespace_ChipConfig, "last-ek-id" };
-const PosixConfig::Key PosixConfig::kConfigKey_FailSafeArmed       = { kConfigNamespace_ChipConfig, "fail-safe-armed" };
-const PosixConfig::Key PosixConfig::kConfigKey_WiFiStationSecType  = { kConfigNamespace_ChipConfig, "sta-sec-type" };
-const PosixConfig::Key PosixConfig::kConfigKey_RegulatoryLocation  = { kConfigNamespace_ChipConfig, "regulatory-location" };
-const PosixConfig::Key PosixConfig::kConfigKey_CountryCode         = { kConfigNamespace_ChipConfig, "country-code" };
-const PosixConfig::Key PosixConfig::kConfigKey_Breadcrumb          = { kConfigNamespace_ChipConfig, "breadcrumb" };
-const PosixConfig::Key PosixConfig::kCounterKey_RegulatoryConfig   = { kConfigNamespace_ChipConfig, "regulatory-config" };
-const PosixConfig::Key PosixConfig::kCounterKey_LocationCapability = { kConfigNamespace_ChipConfig, "location-capability" };
+const PosixConfig::Key PosixConfig::kConfigKey_FabricId           = { kConfigNamespace_ChipConfig, "fabric-id" };
+const PosixConfig::Key PosixConfig::kConfigKey_ServiceConfig      = { kConfigNamespace_ChipConfig, "service-config" };
+const PosixConfig::Key PosixConfig::kConfigKey_PairedAccountId    = { kConfigNamespace_ChipConfig, "account-id" };
+const PosixConfig::Key PosixConfig::kConfigKey_ServiceId          = { kConfigNamespace_ChipConfig, "service-id" };
+const PosixConfig::Key PosixConfig::kConfigKey_FabricSecret       = { kConfigNamespace_ChipConfig, "fabric-secret" };
+const PosixConfig::Key PosixConfig::kConfigKey_GroupKeyIndex      = { kConfigNamespace_ChipConfig, "group-key-index" };
+const PosixConfig::Key PosixConfig::kConfigKey_LastUsedEpochKeyId = { kConfigNamespace_ChipConfig, "last-ek-id" };
+const PosixConfig::Key PosixConfig::kConfigKey_FailSafeArmed      = { kConfigNamespace_ChipConfig, "fail-safe-armed" };
+const PosixConfig::Key PosixConfig::kConfigKey_WiFiStationSecType = { kConfigNamespace_ChipConfig, "sta-sec-type" };
+const PosixConfig::Key PosixConfig::kConfigKey_RegulatoryLocation = { kConfigNamespace_ChipConfig, "regulatory-location" };
+const PosixConfig::Key PosixConfig::kConfigKey_CountryCode        = { kConfigNamespace_ChipConfig, "country-code" };
+const PosixConfig::Key PosixConfig::kConfigKey_Breadcrumb         = { kConfigNamespace_ChipConfig, "breadcrumb" };
+const PosixConfig::Key PosixConfig::kConfigKey_LocationCapability = { kConfigNamespace_ChipConfig, "location-capability" };
 
 // Keys stored in the Chip-counters namespace
 const PosixConfig::Key PosixConfig::kCounterKey_RebootCount           = { kConfigNamespace_ChipCounters, "reboot-count" };

diff --git a/src/platform/Linux/PosixConfig.h b/src/platform/Linux/PosixConfig.h
@@ -75,13 +75,12 @@ class PosixConfig
     static const Key kConfigKey_RegulatoryLocation;
     static const Key kConfigKey_CountryCode;
     static const Key kConfigKey_Breadcrumb;
+    static const Key kConfigKey_LocationCapability;
 
     static const Key kCounterKey_RebootCount;
     static const Key kCounterKey_UpTime;
     static const Key kCounterKey_TotalOperationalHours;
     static const Key kCounterKey_BootReason;
-    static const Key kCounterKey_RegulatoryConfig;
-    static const Key kCounterKey_LocationCapability;
 
     static const char kGroupKeyNamePrefix[];
 

diff --git a/src/platform/fake/ConfigurationManagerImpl.h b/src/platform/fake/ConfigurationManagerImpl.h
@@ -75,8 +75,8 @@ class ConfigurationManagerImpl : public ConfigurationManager
     CHIP_ERROR GetInitialPairingInstruction(char * buf, size_t bufSize) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR GetSecondaryPairingHint(uint16_t & pairingHint) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR GetSecondaryPairingInstruction(char * buf, size_t bufSize) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
-    CHIP_ERROR GetRegulatoryLocation(uint32_t & location) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
-    CHIP_ERROR StoreRegulatoryLocation(uint32_t location) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
+    CHIP_ERROR GetRegulatoryLocation(uint8_t & location) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
+    CHIP_ERROR StoreRegulatoryLocation(uint8_t location) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR GetCountryCode(char * buf, size_t bufSize, size_t & codeLen) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR StoreCountryCode(const char * code, size_t codeLen) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR GetBreadcrumb(uint64_t & breadcrumb) override { return CHIP_ERROR_NOT_IMPLEMENTED; }

diff --git a/src/platform/tests/TestConfigurationMgr.cpp b/src/platform/tests/TestConfigurationMgr.cpp
@@ -153,20 +153,6 @@ static void TestConfigurationMgr_SetupDiscriminator(nlTestSuite * inSuite, void
     NL_TEST_ASSERT(inSuite, getSetupDiscriminator == setSetupDiscriminator);
 }
 
-static void TestConfigurationMgr_RegulatoryLocation(nlTestSuite * inSuite, void * inContext)
-{
-    CHIP_ERROR err    = CHIP_NO_ERROR;
-    uint32_t location = 0;
-
-    err = ConfigurationMgr().StoreRegulatoryLocation(12345);
-    NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
-
-    err = ConfigurationMgr().GetRegulatoryLocation(location);
-    NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
-
-    NL_TEST_ASSERT(inSuite, location == 12345);
-}
-
 static void TestConfigurationMgr_CountryCode(nlTestSuite * inSuite, void * inContext)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
@@ -255,7 +241,6 @@ static const nlTest sTests[] = {
     NL_TEST_DEF("Test ConfigurationMgr::HardwareVersion", TestConfigurationMgr_HardwareVersion),
     NL_TEST_DEF("Test ConfigurationMgr::SetupPinCode", TestConfigurationMgr_SetupPinCode),
     NL_TEST_DEF("Test ConfigurationMgr::SetupDiscriminator", TestConfigurationMgr_SetupDiscriminator),
-    NL_TEST_DEF("Test ConfigurationMgr::RegulatoryLocation", TestConfigurationMgr_RegulatoryLocation),
     NL_TEST_DEF("Test ConfigurationMgr::CountryCode", TestConfigurationMgr_CountryCode),
     NL_TEST_DEF("Test ConfigurationMgr::Breadcrumb", TestConfigurationMgr_Breadcrumb),
     NL_TEST_DEF("Test ConfigurationMgr::GetPrimaryMACAddress", TestConfigurationMgr_GetPrimaryMACAddress),