Set configuration manager (#11534)

* Add the setter for the ConfigurationManager.

Also remove the implementation getter.

* Remove uses of ConfigurationMgrImpl.

Promote the reboot count, boot reason, and operational hours methods
to the ConfigurationManager interface.

* Add a static instance getter to the ConfigurationManagerImpl.

* Set the default ConfigurationManager instance in the platform manager.

* Update the tests to set a ConfigurationManager instance.

* Allow subclasses of the platform ConfigurationManagerImpls.

* Re-fix Ameba's ConfigurationManagerImpl.

* Restyled by clang-format

* Improvements from pull request comments.

* Add VerifyOrDie to the ConfigurationMgr getter.
* Pass a pointer rather than a reference to ConfigurationMgr.
* Explicitly default the instance pointer to nullptr.
* Move the singleton implementation out of each platform into a shared file.

* Update Ameba platform to remove the new uses of ConfigurationMgrImpl.

Co-authored-by: Restyled.io <[email protected]>

src/controller/tests/TestDevice.cpp

@@ -24,6 +24,7 @@
 #include <lib/support/CHIPMem.h>
 #include <lib/support/UnitTestRegistration.h>
 #include <nlunit-test.h>
+#include <platform/CHIPDeviceLayer.h>
 #include <protocols/secure_channel/MessageCounterManager.h>
 #include <protocols/secure_channel/SessionIDAllocator.h>
 #include <system/SystemLayerImpl.h>
@@ -44,6 +45,7 @@ using TestTransportMgr = TransportMgr<Transport::UDP>;
 void TestDevice_EstablishSessionDirectly(nlTestSuite * inSuite, void * inContext)
 {
     Platform::MemoryInit();
+    chip::DeviceLayer::SetConfigurationMgr(&chip::DeviceLayer::ConfigurationManagerImpl::GetDefaultInstance());
     DeviceTransportMgr transportMgr;
     SessionManager sessionManager;
     ExchangeManager exchangeMgr;

src/include/platform/ConfigurationManager.h

@@ -100,6 +100,12 @@ class ConfigurationManager
     virtual CHIP_ERROR StoreRegulatoryLocation(uint32_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;
+    virtual CHIP_ERROR StoreRebootCount(uint32_t rebootCount)                          = 0;
+    virtual CHIP_ERROR GetTotalOperationalHours(uint32_t & totalOperationalHours)      = 0;
+    virtual CHIP_ERROR StoreTotalOperationalHours(uint32_t totalOperationalHours)      = 0;
+    virtual CHIP_ERROR GetBootReasons(uint32_t & bootReasons)                          = 0;
+    virtual CHIP_ERROR StoreBootReasons(uint32_t bootReasons)                          = 0;
 
     virtual CHIP_ERROR GetBLEDeviceIdentificationInfo(Ble::ChipBLEDeviceIdentificationInfo & deviceIdInfo) = 0;
 
@@ -151,20 +157,19 @@ class ConfigurationManager
 };
 
 /**
- * Returns a reference to the public interface of the ConfigurationManager singleton object.
+ * Returns a reference to a ConfigurationManager object.
  *
- * chip application should use this to access features of the ConfigurationManager object
- * that are common to all platforms.
+ * Applications should use this to access the features of the ConfigurationManager.
  */
 extern ConfigurationManager & ConfigurationMgr();
 
 /**
- * Returns the platform-specific implementation of the ConfigurationManager singleton object.
+ * Sets a reference to a ConfigurationManager object.
  *
- * chip applications can use this to gain access to features of the ConfigurationManager
- * that are specific to the selected platform.
+ * This must be called before any calls to ConfigurationMgr. If a nullptr is passed in,
+ * no changes will be made.
  */
-extern ConfigurationManagerImpl & ConfigurationMgrImpl();
+extern void SetConfigurationMgr(ConfigurationManager * configurationManager);
 
 } // namespace DeviceLayer
 } // namespace chip

src/include/platform/internal/GenericConfigurationManagerImpl.cpp

@@ -343,6 +343,42 @@ CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::StoreBreadcrumb(uint64_t
     return Impl()->WriteConfigValue(ImplClass::kConfigKey_Breadcrumb, breadcrumb);
 }
 
+template <class ImplClass>
+CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::GetRebootCount(uint32_t & rebootCount)
+{
+    return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+}
+
+template <class ImplClass>
+CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::StoreRebootCount(uint32_t rebootCount)
+{
+    return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+}
+
+template <class ImplClass>
+CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::GetTotalOperationalHours(uint32_t & totalOperationalHours)
+{
+    return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+}
+
+template <class ImplClass>
+CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::StoreTotalOperationalHours(uint32_t totalOperationalHours)
+{
+    return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+}
+
+template <class ImplClass>
+CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::GetBootReasons(uint32_t & bootReasons)
+{
+    return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+}
+
+template <class ImplClass>
+CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::StoreBootReasons(uint32_t bootReasons)
+{
+    return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+}
+
 #if CHIP_ENABLE_ROTATING_DEVICE_ID
 template <class ImplClass>
 CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::GetLifetimeCounter(uint16_t & lifetimeCounter)

src/include/platform/internal/GenericConfigurationManagerImpl.h

@@ -97,6 +97,12 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
     CHIP_ERROR StoreCountryCode(const char * code, size_t codeLen) override;
     CHIP_ERROR GetBreadcrumb(uint64_t & breadcrumb) override;
     CHIP_ERROR StoreBreadcrumb(uint64_t breadcrumb) override;
+    CHIP_ERROR GetRebootCount(uint32_t & rebootCount) override;
+    CHIP_ERROR StoreRebootCount(uint32_t rebootCount) override;
+    CHIP_ERROR GetTotalOperationalHours(uint32_t & totalOperationalHours) override;
+    CHIP_ERROR StoreTotalOperationalHours(uint32_t totalOperationalHours) override;
+    CHIP_ERROR GetBootReasons(uint32_t & bootReasons) override;
+    CHIP_ERROR StoreBootReasons(uint32_t bootReasons) override;
 #if !defined(NDEBUG)
     CHIP_ERROR RunUnitTests(void) override;
 #endif

src/platform/Ameba/BUILD.gn

@@ -21,6 +21,7 @@ assert(chip_device_platform == "ameba")
 static_library("Ameba") {
   sources = [
     "../FreeRTOS/SystemTimeSupport.cpp",
+    "../SingletonConfigurationManager.cpp",
     "AmebaConfig.cpp",
     "AmebaConfig.h",
     "BLEManagerImpl.cpp",

src/platform/Ameba/ConfigurationManagerImpl.cpp

@@ -36,9 +36,11 @@ namespace DeviceLayer {
 
 using namespace ::chip::DeviceLayer::Internal;
 
-/** Singleton instance of the ConfigurationManager implementation object.
- */
-ConfigurationManagerImpl ConfigurationManagerImpl::sInstance;
+ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
+{
+    static ConfigurationManagerImpl sInstance;
+    return sInstance;
+}
 
 CHIP_ERROR ConfigurationManagerImpl::Init()
 {

src/platform/Ameba/ConfigurationManagerImpl.h

@@ -34,22 +34,14 @@ namespace DeviceLayer {
 /**
  * Concrete implementation of the ConfigurationManager singleton object for the Ameba platform.
  */
-class ConfigurationManagerImpl final : public Internal::GenericConfigurationManagerImpl<ConfigurationManagerImpl>,
-                                       private Internal::AmebaConfig
+class ConfigurationManagerImpl : public Internal::GenericConfigurationManagerImpl<ConfigurationManagerImpl>,
+                                 private Internal::AmebaConfig
 {
 public:
-    CHIP_ERROR GetRebootCount(uint32_t & rebootCount);
-    CHIP_ERROR StoreRebootCount(uint32_t rebootCount);
-    CHIP_ERROR GetTotalOperationalHours(uint32_t & totalOperationalHours);
-    CHIP_ERROR StoreTotalOperationalHours(uint32_t totalOperationalHours);
-    CHIP_ERROR GetBootReasons(uint32_t & bootReasons);
-    CHIP_ERROR StoreBootReasons(uint32_t bootReasons);
+    // This returns an instance of this class.
+    static ConfigurationManagerImpl & GetDefaultInstance();
 
 private:
-    // Allow the ConfigurationManager interface class to delegate method calls to
-    // the implementation methods provided by this class.
-    friend class ConfigurationManager;
-
     // Allow the GenericConfigurationManagerImpl base class to access helper methods and types
     // defined on this class.
 #ifndef DOXYGEN_SHOULD_SKIP_THIS
@@ -64,42 +56,19 @@ class ConfigurationManagerImpl final : public Internal::GenericConfigurationMana
     void InitiateFactoryReset(void) override;
     CHIP_ERROR ReadPersistedStorageValue(::chip::Platform::PersistedStorage::Key key, uint32_t & value) override;
     CHIP_ERROR WritePersistedStorageValue(::chip::Platform::PersistedStorage::Key key, uint32_t value) override;
+    CHIP_ERROR GetRebootCount(uint32_t & rebootCount) override;
+    CHIP_ERROR StoreRebootCount(uint32_t rebootCount) override;
+    CHIP_ERROR GetTotalOperationalHours(uint32_t & totalOperationalHours) override;
+    CHIP_ERROR StoreTotalOperationalHours(uint32_t totalOperationalHours) override;
+    CHIP_ERROR GetBootReasons(uint32_t & bootReasons) override;
+    CHIP_ERROR StoreBootReasons(uint32_t bootReasons) override;
 
     // NOTE: Other public interface methods are implemented by GenericConfigurationManagerImpl<>.
 
-    // ===== Members for internal use by the following friends.
-
-    friend ConfigurationManager & ConfigurationMgr(void);
-    friend ConfigurationManagerImpl & ConfigurationMgrImpl(void);
-
-    static ConfigurationManagerImpl sInstance;
-
     // ===== Private members reserved for use by this class only.
 
     static void DoFactoryReset(intptr_t arg);
 };
 
-/**
- * Returns the public interface of the ConfigurationManager singleton object.
- *
- * Chip applications should use this to access features of the ConfigurationManager object
- * that are common to all platforms.
- */
-inline ConfigurationManager & ConfigurationMgr(void)
-{
-    return ConfigurationManagerImpl::sInstance;
-}
-
-/**
- * Returns the platform-specific implementation of the ConfigurationManager singleton object.
- *
- * Chip applications can use this to gain access to features of the ConfigurationManager
- * that are specific to the Ameba platform.
- */
-inline ConfigurationManagerImpl & ConfigurationMgrImpl(void)
-{
-    return ConfigurationManagerImpl::sInstance;
-}
-
 } // namespace DeviceLayer
 } // namespace chip

src/platform/Ameba/PlatformManagerImpl.cpp

@@ -57,6 +57,8 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack(void)
 
     CHIP_ERROR err;
 
+    SetConfigurationMgr(&ConfigurationManagerImpl::GetDefaultInstance());
+
     // Make sure the LwIP core lock has been initialized
     err = Internal::InitLwIPCoreLock();
 
@@ -86,9 +88,9 @@ CHIP_ERROR PlatformManagerImpl::_Shutdown()
     {
         uint32_t totalOperationalHours = 0;
 
-        if (ConfigurationMgrImpl().GetTotalOperationalHours(totalOperationalHours) == CHIP_NO_ERROR)
+        if (ConfigurationMgr().GetTotalOperationalHours(totalOperationalHours) == CHIP_NO_ERROR)
         {
-            ConfigurationMgrImpl().StoreTotalOperationalHours(totalOperationalHours + static_cast<uint32_t>(upTime / 3600));
+            ConfigurationMgr().StoreTotalOperationalHours(totalOperationalHours + static_cast<uint32_t>(upTime / 3600));
         }
         else
         {
@@ -124,7 +126,7 @@ CHIP_ERROR PlatformManagerImpl::_GetRebootCount(uint16_t & rebootCount)
 {
     uint32_t count = 0;
 
-    CHIP_ERROR err = ConfigurationMgrImpl().GetRebootCount(count);
+    CHIP_ERROR err = ConfigurationMgr().GetRebootCount(count);
 
     if (err == CHIP_NO_ERROR)
     {
@@ -155,7 +157,7 @@ CHIP_ERROR PlatformManagerImpl::_GetTotalOperationalHours(uint32_t & totalOperat
     if (_GetUpTime(upTime) == CHIP_NO_ERROR)
     {
         uint32_t totalHours = 0;
-        if (ConfigurationMgrImpl().GetTotalOperationalHours(totalHours) == CHIP_NO_ERROR)
+        if (ConfigurationMgr().GetTotalOperationalHours(totalHours) == CHIP_NO_ERROR)
         {
             VerifyOrReturnError(upTime / 3600 <= UINT32_MAX, CHIP_ERROR_INVALID_INTEGER_VALUE);
             totalOperationalHours = totalHours + static_cast<uint32_t>(upTime / 3600);
@@ -170,7 +172,7 @@ CHIP_ERROR PlatformManagerImpl::_GetBootReasons(uint8_t & bootReasons)
 {
     uint32_t reason = 0;
 
-    CHIP_ERROR err = ConfigurationMgrImpl().GetBootReasons(reason);
+    CHIP_ERROR err = ConfigurationMgr().GetBootReasons(reason);
 
     if (err == CHIP_NO_ERROR)
     {

src/platform/Darwin/BUILD.gn

@@ -34,6 +34,7 @@ static_library("Darwin") {
   sources = [
     "../DeviceSafeQueue.cpp",
     "../DeviceSafeQueue.h",
+    "../SingletonConfigurationManager.cpp",
     "BLEManagerImpl.cpp",
     "BLEManagerImpl.h",
     "BlePlatformConfig.h",

src/platform/Darwin/ConfigurationManagerImpl.cpp

@@ -128,9 +128,11 @@ CHIP_ERROR GetMACAddressFromInterfaces(io_iterator_t primaryInterfaceIterator, u
 }
 #endif // TARGET_OS_OSX
 
-/** Singleton instance of the ConfigurationManager implementation object.
- */
-ConfigurationManagerImpl ConfigurationManagerImpl::sInstance;
+ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
+{
+    static ConfigurationManagerImpl sInstance;
+    return sInstance;
+}
 
 CHIP_ERROR ConfigurationManagerImpl::Init()
 {

src/platform/Darwin/ConfigurationManagerImpl.h

@@ -33,15 +33,19 @@ namespace DeviceLayer {
 /**
  * Concrete implementation of the ConfigurationManager singleton object for the Darwin platform.
  */
-class ConfigurationManagerImpl final : public Internal::GenericConfigurationManagerImpl<ConfigurationManagerImpl>,
-                                       private Internal::PosixConfig
+class ConfigurationManagerImpl : public Internal::GenericConfigurationManagerImpl<ConfigurationManagerImpl>,
+                                 private Internal::PosixConfig
 {
     // Allow the GenericConfigurationManagerImpl base class to access helper methods and types
     // defined on this class.
 #ifndef DOXYGEN_SHOULD_SKIP_THIS
     friend class Internal::GenericConfigurationManagerImpl<ConfigurationManagerImpl>;
 #endif
 
+public:
+    // This returns an instance of this class.
+    static ConfigurationManagerImpl & GetDefaultInstance();
+
 private:
     // ===== Members that implement the ConfigurationManager public interface.
 
@@ -53,36 +57,7 @@ class ConfigurationManagerImpl final : public Internal::GenericConfigurationMana
     CHIP_ERROR WritePersistedStorageValue(::chip::Platform::PersistedStorage::Key key, uint32_t value) override;
 
     // NOTE: Other public interface methods are implemented by GenericConfigurationManagerImpl<>.
-
-    // ===== Members for internal use by the following friends.
-
-    friend ConfigurationManager & ConfigurationMgr(void);
-    friend ConfigurationManagerImpl & ConfigurationMgrImpl(void);
-
-    static ConfigurationManagerImpl sInstance;
 };
 
-/**
- * Returns the public interface of the ConfigurationManager singleton object.
- *
- * chip applications should use this to access features of the ConfigurationManager object
- * that are common to all platforms.
- */
-inline ConfigurationManager & ConfigurationMgr(void)
-{
-    return ConfigurationManagerImpl::sInstance;
-}
-
-/**
- * Returns the platform-specific implementation of the ConfigurationManager singleton object.
- *
- * chip applications can use this to gain access to features of the ConfigurationManager
- * that are specific to the ESP32 platform.
- */
-inline ConfigurationManagerImpl & ConfigurationMgrImpl(void)
-{
-    return ConfigurationManagerImpl::sInstance;
-}
-
 } // namespace DeviceLayer
 } // namespace chip

src/platform/Darwin/PlatformManagerImpl.cpp

@@ -43,6 +43,7 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack()
     // Initialize the configuration system.
     err = Internal::PosixConfig::Init();
     SuccessOrExit(err);
+    SetConfigurationMgr(&ConfigurationManagerImpl::GetDefaultInstance());
 
     mRunLoopSem = dispatch_semaphore_create(0);
 

src/platform/EFR32/BUILD.gn

@@ -26,6 +26,7 @@ if (chip_enable_openthread) {
 static_library("EFR32") {
   sources = [
     "../FreeRTOS/SystemTimeSupport.cpp",
+    "../SingletonConfigurationManager.cpp",
     "BLEManagerImpl.cpp",
     "BLEManagerImpl.h",
     "BlePlatformConfig.h",

src/platform/EFR32/ConfigurationManagerImpl.cpp

@@ -34,9 +34,11 @@ namespace DeviceLayer {
 
 using namespace ::chip::DeviceLayer::Internal;
 
-/** Singleton instance of the ConfigurationManager implementation object.
- */
-ConfigurationManagerImpl ConfigurationManagerImpl::sInstance;
+ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
+{
+    static ConfigurationManagerImpl sInstance;
+    return sInstance;
+}
 
 CHIP_ERROR ConfigurationManagerImpl::Init()
 {