From d8b003bf198d17d5abf6356db0182968a443ce00 Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Thu, 10 Mar 2022 15:29:42 -0500 Subject: [PATCH] Allow runtime init of some `Server` members (1/2) (#16065) * Allow runtime init of some `Server` members (1/2) This PR is on the path towards having Server::Server no longer statically initialize its members with storage, and instead relying on Server::Init(). This will simplify organization of unit tests and also the convergence of Controller/Server storage to address issue #16028 Issue #16028 * Restyled by clang-format * Move a comment to the correct place Co-authored-by: Restyled.io --- .../DefaultAttributePersistenceProvider.cpp | 8 ++++++-- src/app/DefaultAttributePersistenceProvider.h | 20 +++++++++++++++---- src/app/server/CommissioningWindowManager.h | 12 ++++++++++- src/app/server/Server.cpp | 10 ++++++++-- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/app/DefaultAttributePersistenceProvider.cpp b/src/app/DefaultAttributePersistenceProvider.cpp index c589c1d36c14c9..9b7c3a4538912b 100644 --- a/src/app/DefaultAttributePersistenceProvider.cpp +++ b/src/app/DefaultAttributePersistenceProvider.cpp @@ -25,6 +25,8 @@ namespace app { CHIP_ERROR DefaultAttributePersistenceProvider::WriteValue(const ConcreteAttributePath & aPath, const EmberAfAttributeMetadata * aMetadata, const ByteSpan & aValue) { + VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE); + // TODO: we may want to have a small cache for values that change a lot, so // we only write them once a bunch of changes happen or on timer or // shutdown. @@ -33,15 +35,17 @@ CHIP_ERROR DefaultAttributePersistenceProvider::WriteValue(const ConcreteAttribu { return CHIP_ERROR_BUFFER_TOO_SMALL; } - return mStorage.SyncSetKeyValue(key.AttributeValue(aPath), aValue.data(), static_cast(aValue.size())); + return mStorage->SyncSetKeyValue(key.AttributeValue(aPath), aValue.data(), static_cast(aValue.size())); } CHIP_ERROR DefaultAttributePersistenceProvider::ReadValue(const ConcreteAttributePath & aPath, const EmberAfAttributeMetadata * aMetadata, MutableByteSpan & aValue) { + VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE); + DefaultStorageKeyAllocator key; uint16_t size = static_cast(min(aValue.size(), static_cast(UINT16_MAX))); - ReturnErrorOnFailure(mStorage.SyncGetKeyValue(key.AttributeValue(aPath), aValue.data(), size)); + ReturnErrorOnFailure(mStorage->SyncGetKeyValue(key.AttributeValue(aPath), aValue.data(), size)); EmberAfAttributeType type = aMetadata->attributeType; if (emberAfIsStringAttributeType(type)) { diff --git a/src/app/DefaultAttributePersistenceProvider.h b/src/app/DefaultAttributePersistenceProvider.h index bc538ad6130261..41bb19b7a4f9a4 100644 --- a/src/app/DefaultAttributePersistenceProvider.h +++ b/src/app/DefaultAttributePersistenceProvider.h @@ -33,8 +33,20 @@ namespace app { class DefaultAttributePersistenceProvider : public AttributePersistenceProvider { public: - // aStorage must outlive this object. - DefaultAttributePersistenceProvider(PersistentStorageDelegate & aStorage) : mStorage(aStorage) {} + DefaultAttributePersistenceProvider() {} + + // Passed-in storage must outlive this object. + CHIP_ERROR Init(PersistentStorageDelegate * storage) + { + if (storage == nullptr) + { + return CHIP_ERROR_INVALID_ARGUMENT; + } + mStorage = storage; + return CHIP_NO_ERROR; + } + + void Shutdown() {} // AttributePersistenceProvider implementation. CHIP_ERROR WriteValue(const ConcreteAttributePath & aPath, const EmberAfAttributeMetadata * aMetadata, @@ -42,8 +54,8 @@ class DefaultAttributePersistenceProvider : public AttributePersistenceProvider CHIP_ERROR ReadValue(const ConcreteAttributePath & aPath, const EmberAfAttributeMetadata * aMetadata, MutableByteSpan & aValue) override; -private: - PersistentStorageDelegate & mStorage; +protected: + PersistentStorageDelegate * mStorage; }; } // namespace app diff --git a/src/app/server/CommissioningWindowManager.h b/src/app/server/CommissioningWindowManager.h index 02dde6edeb77c9..86b360c38fd276 100644 --- a/src/app/server/CommissioningWindowManager.h +++ b/src/app/server/CommissioningWindowManager.h @@ -39,7 +39,17 @@ class Server; class CommissioningWindowManager : public SessionEstablishmentDelegate, public app::CommissioningModeProvider { public: - CommissioningWindowManager(Server * server) : mAppDelegate(nullptr), mServer(server) {} + CommissioningWindowManager() {} + + CHIP_ERROR Init(Server * server) + { + if (server == nullptr) + { + return CHIP_ERROR_INVALID_ARGUMENT; + } + mServer = server; + return CHIP_NO_ERROR; + } void SetAppDelegate(AppDelegate * delegate) { mAppDelegate = delegate; } diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 137eb2cf974de2..c1d5da11a95653 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -107,8 +107,7 @@ Server::Server() : .devicePool = &mDevicePool, .dnsResolver = nullptr, }), - mCommissioningWindowManager(this), mGroupsProvider(mDeviceStorage), mAttributePersister(mDeviceStorage), - mAccessControl(Access::Examples::GetAccessControlDelegate(&mDeviceStorage)) + mGroupsProvider(mDeviceStorage), mAccessControl(Access::Examples::GetAccessControlDelegate(&mDeviceStorage)) {} CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint16_t unsecureServicePort, @@ -120,13 +119,16 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint CHIP_ERROR err = CHIP_NO_ERROR; + // TODO: Remove chip::Platform::MemoryInit() call from Server class, it belongs to outer code chip::Platform::MemoryInit(); + SuccessOrExit(err = mCommissioningWindowManager.Init(this)); mCommissioningWindowManager.SetAppDelegate(delegate); mCommissioningWindowManager.SetSessionIDAllocator(&mSessionIDAllocator); // Set up attribute persistence before we try to bring up the data model // handler. + SuccessOrExit(mAttributePersister.Init(&mDeviceStorage)); SetAttributePersistenceProvider(&mAttributePersister); InitDataModelHandler(&mExchangeMgr); @@ -337,8 +339,12 @@ void Server::Shutdown() } mSessions.Shutdown(); mTransports.Close(); + + mAttributePersister.Shutdown(); mCommissioningWindowManager.Shutdown(); mCASESessionManager.Shutdown(); + + // TODO: Remove chip::Platform::MemoryInit() call from Server class, it belongs to outer code chip::Platform::MemoryShutdown(); }