From 61cf395a307cf34b809417e6ea31509b1a725a36 Mon Sep 17 00:00:00 2001 From: Damian Krolik Date: Thu, 27 Oct 2022 14:16:15 +0200 Subject: [PATCH 1/3] [app] Implement deferred attribute persister Fast-changing attributes with NVM storage, such as CurrentLevel of the LevelControl cluster, may result in rapid flash wearout using the default attribute persister which stores all values immediately. Implement a helper adapter class for the attribute persistence interface to defer writes of selected attributes. Use the new class in nRF Connect lighting-app for verification. --- .../lighting-app/nrfconnect/main/AppTask.cpp | 8 +++ src/app/BUILD.gn | 1 + .../DeferredAttributePersistenceProvider.cpp | 70 ++++++++++++++++++ .../DeferredAttributePersistenceProvider.h | 72 +++++++++++++++++++ src/app/server/Server.h | 2 + 5 files changed, 153 insertions(+) create mode 100644 src/app/DeferredAttributePersistenceProvider.cpp create mode 100644 src/app/DeferredAttributePersistenceProvider.h diff --git a/examples/lighting-app/nrfconnect/main/AppTask.cpp b/examples/lighting-app/nrfconnect/main/AppTask.cpp index bd2375e3d02001..92a9a97af43fb3 100644 --- a/examples/lighting-app/nrfconnect/main/AppTask.cpp +++ b/examples/lighting-app/nrfconnect/main/AppTask.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -89,6 +90,12 @@ bool sHaveBLEConnections = false; chip::DeviceLayer::DeviceInfoProviderImpl gExampleDeviceInfoProvider; +DeferredAttribute gCurrentLevelPersister(ConcreteAttributePath(kLightEndpointId, Clusters::LevelControl::Id, + Clusters::LevelControl::Attributes::CurrentLevel::Id)); +DeferredAttributePersistenceProvider gDeferredAttributePersister(Server::GetInstance().GetAttributePersister(), + Span(&gCurrentLevelPersister, 1), + System::Clock::Milliseconds32(5000)); + } // namespace AppTask AppTask::sAppTask; @@ -195,6 +202,7 @@ CHIP_ERROR AppTask::Init() gExampleDeviceInfoProvider.SetStorageDelegate(&Server::GetInstance().GetPersistentStorage()); chip::DeviceLayer::SetDeviceInfoProvider(&gExampleDeviceInfoProvider); + app::SetAttributePersistenceProvider(&gDeferredAttributePersister); ConfigurationMgr().LogDeviceConfig(); PrintOnboardingCodes(chip::RendezvousInformationFlags(chip::RendezvousInformationFlag::kBLE)); diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index fb3e6524881636..2d3c45228ab296 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -82,6 +82,7 @@ static_library("app") { "CommandResponseHelper.h", "CommandSender.cpp", "DefaultAttributePersistenceProvider.cpp", + "DeferredAttributePersistenceProvider.cpp", "DeviceProxy.cpp", "DeviceProxy.h", "EventManagement.cpp", diff --git a/src/app/DeferredAttributePersistenceProvider.cpp b/src/app/DeferredAttributePersistenceProvider.cpp new file mode 100644 index 00000000000000..e1e2207c6ac618 --- /dev/null +++ b/src/app/DeferredAttributePersistenceProvider.cpp @@ -0,0 +1,70 @@ +/* + * Copyright (c) 2022 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +namespace chip { +namespace app { + +CHIP_ERROR DeferredAttribute::PrepareWrite(AttributePersistenceProvider & persister, const EmberAfAttributeMetadata * metadata, + const ByteSpan & value) +{ + mPersister = &persister; + mMetadata = metadata; + + if (mValue.AllocatedSize() != value.size()) + { + mValue.Alloc(value.size()); + ReturnErrorCodeIf(!mValue, CHIP_ERROR_NO_MEMORY); + } + + memcpy(mValue.Get(), value.data(), value.size()); + return CHIP_NO_ERROR; +} + +void DeferredAttribute::Flush() +{ + VerifyOrReturn(mValue); + mPersister->WriteValue(mPath, mMetadata, ByteSpan(mValue.Get(), mValue.AllocatedSize())); + mValue.Release(); +} + +CHIP_ERROR DeferredAttributePersistenceProvider::WriteValue(const ConcreteAttributePath & path, + const EmberAfAttributeMetadata * metadata, const ByteSpan & value) +{ + for (DeferredAttribute & da : mDeferredAttributes) + { + if (da.Matches(path)) + { + ReturnErrorOnFailure(da.PrepareWrite(mPersister, metadata, value)); + DeviceLayer::SystemLayer().StartTimer(System::Clock::Milliseconds32(mWriteDelay), Flush, &da); + return CHIP_NO_ERROR; + } + } + + return mPersister.WriteValue(path, metadata, value); +} + +CHIP_ERROR DeferredAttributePersistenceProvider::ReadValue(const ConcreteAttributePath & path, + const EmberAfAttributeMetadata * metadata, MutableByteSpan & value) +{ + return mPersister.ReadValue(path, metadata, value); +} + +} // namespace app +} // namespace chip diff --git a/src/app/DeferredAttributePersistenceProvider.h b/src/app/DeferredAttributePersistenceProvider.h new file mode 100644 index 00000000000000..d3a9519570f6cf --- /dev/null +++ b/src/app/DeferredAttributePersistenceProvider.h @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2022 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +#include +#include +#include + +namespace chip { +namespace app { + +class DeferredAttribute +{ +public: + explicit DeferredAttribute(const ConcreteAttributePath & path) : mPath(path) {} + + bool Matches(const ConcreteAttributePath & path) const { return mPath == path; } + CHIP_ERROR PrepareWrite(AttributePersistenceProvider & persister, const EmberAfAttributeMetadata * metadata, + const ByteSpan & value); + void Flush(); + +private: + const ConcreteAttributePath mPath; + AttributePersistenceProvider * mPersister; + const EmberAfAttributeMetadata * mMetadata; + Platform::ScopedMemoryBufferWithSize mValue; +}; + +/** + * Decorator class for the AttributePersistenceProvider implementation that + * defers writes of selected attributes. + * + * This class is useful to increase the flash lifetime by reducing the number + * of writes of fast-changing attributes, such as CurrentLevel attribute of the + * LevelControl cluster. + */ +class DeferredAttributePersistenceProvider : public AttributePersistenceProvider +{ +public: + DeferredAttributePersistenceProvider(AttributePersistenceProvider & persister, + const Span & deferredAttributes, + System::Clock::Milliseconds32 writeDelay) : + mPersister(persister), + mDeferredAttributes(deferredAttributes), mWriteDelay(writeDelay) + {} + + CHIP_ERROR WriteValue(const ConcreteAttributePath & path, const EmberAfAttributeMetadata * metadata, const ByteSpan & value); + CHIP_ERROR ReadValue(const ConcreteAttributePath & path, const EmberAfAttributeMetadata * metadata, MutableByteSpan & value); + +private: + static void Flush(System::Layer *, void * deferredAttr) { static_cast(deferredAttr)->Flush(); } + + AttributePersistenceProvider & mPersister; + const Span mDeferredAttributes; + const System::Clock::Milliseconds32 mWriteDelay; +}; + +} // namespace app +} // namespace chip diff --git a/src/app/server/Server.h b/src/app/server/Server.h index 25b093e192e4f5..eb7eda58b7741a 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -340,6 +340,8 @@ class Server Credentials::OperationalCertificateStore * GetOpCertStore() { return mOpCertStore; } + app::DefaultAttributePersistenceProvider & GetAttributePersister() { return mAttributePersister; } + /** * This function send the ShutDown event before stopping * the event loop. From 3eb2537095f6c95dbe345a1fae3c54d0e8d23288 Mon Sep 17 00:00:00 2001 From: Damian Krolik Date: Fri, 28 Oct 2022 09:10:44 +0200 Subject: [PATCH 2/3] Code review --- .../lighting-app/nrfconnect/main/AppTask.cpp | 2 +- src/app/AttributePersistenceProvider.h | 4 +- .../DefaultAttributePersistenceProvider.cpp | 3 +- src/app/DefaultAttributePersistenceProvider.h | 3 +- .../DeferredAttributePersistenceProvider.cpp | 52 ++++++++++++++----- .../DeferredAttributePersistenceProvider.h | 26 +++++++--- src/app/server/Server.h | 2 +- src/app/util/attribute-storage.cpp | 3 +- 8 files changed, 64 insertions(+), 31 deletions(-) diff --git a/examples/lighting-app/nrfconnect/main/AppTask.cpp b/examples/lighting-app/nrfconnect/main/AppTask.cpp index 92a9a97af43fb3..7a0e91b0cf8aff 100644 --- a/examples/lighting-app/nrfconnect/main/AppTask.cpp +++ b/examples/lighting-app/nrfconnect/main/AppTask.cpp @@ -92,7 +92,7 @@ chip::DeviceLayer::DeviceInfoProviderImpl gExampleDeviceInfoProvider; DeferredAttribute gCurrentLevelPersister(ConcreteAttributePath(kLightEndpointId, Clusters::LevelControl::Id, Clusters::LevelControl::Attributes::CurrentLevel::Id)); -DeferredAttributePersistenceProvider gDeferredAttributePersister(Server::GetInstance().GetAttributePersister(), +DeferredAttributePersistenceProvider gDeferredAttributePersister(Server::GetInstance().GetDefaultAttributePersister(), Span(&gCurrentLevelPersister, 1), System::Clock::Milliseconds32(5000)); diff --git a/src/app/AttributePersistenceProvider.h b/src/app/AttributePersistenceProvider.h index 95da7e96221733..2f4403570ff459 100644 --- a/src/app/AttributePersistenceProvider.h +++ b/src/app/AttributePersistenceProvider.h @@ -37,7 +37,6 @@ class AttributePersistenceProvider * list) to non-volatile memory. * * @param [in] aPath the attribute path for the data being written. - * @param [in] aMetadata the attribute metadata, as a convenience. * @param [in] aValue the data to write. Integers and floats are * represented in native endianness. Strings are represented * as Pascal-style strings, as in ZCL, with a length prefix @@ -51,8 +50,7 @@ class AttributePersistenceProvider * of the data in the string (including the length prefix), * which is no larger than the `size` member of aMetadata. */ - virtual CHIP_ERROR WriteValue(const ConcreteAttributePath & aPath, const EmberAfAttributeMetadata * aMetadata, - const ByteSpan & aValue) = 0; + virtual CHIP_ERROR WriteValue(const ConcreteAttributePath & aPath, const ByteSpan & aValue) = 0; /** * Read an attribute value from non-volatile memory. diff --git a/src/app/DefaultAttributePersistenceProvider.cpp b/src/app/DefaultAttributePersistenceProvider.cpp index 40e973f58b218d..a45bd02bb5c908 100644 --- a/src/app/DefaultAttributePersistenceProvider.cpp +++ b/src/app/DefaultAttributePersistenceProvider.cpp @@ -22,8 +22,7 @@ namespace chip { namespace app { -CHIP_ERROR DefaultAttributePersistenceProvider::WriteValue(const ConcreteAttributePath & aPath, - const EmberAfAttributeMetadata * aMetadata, const ByteSpan & aValue) +CHIP_ERROR DefaultAttributePersistenceProvider::WriteValue(const ConcreteAttributePath & aPath, const ByteSpan & aValue) { VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE); diff --git a/src/app/DefaultAttributePersistenceProvider.h b/src/app/DefaultAttributePersistenceProvider.h index 41bb19b7a4f9a4..e6397b9d883ed2 100644 --- a/src/app/DefaultAttributePersistenceProvider.h +++ b/src/app/DefaultAttributePersistenceProvider.h @@ -49,8 +49,7 @@ class DefaultAttributePersistenceProvider : public AttributePersistenceProvider void Shutdown() {} // AttributePersistenceProvider implementation. - CHIP_ERROR WriteValue(const ConcreteAttributePath & aPath, const EmberAfAttributeMetadata * aMetadata, - const ByteSpan & aValue) override; + CHIP_ERROR WriteValue(const ConcreteAttributePath & aPath, const ByteSpan & aValue) override; CHIP_ERROR ReadValue(const ConcreteAttributePath & aPath, const EmberAfAttributeMetadata * aMetadata, MutableByteSpan & aValue) override; diff --git a/src/app/DeferredAttributePersistenceProvider.cpp b/src/app/DeferredAttributePersistenceProvider.cpp index e1e2207c6ac618..37374e15284689 100644 --- a/src/app/DeferredAttributePersistenceProvider.cpp +++ b/src/app/DeferredAttributePersistenceProvider.cpp @@ -21,11 +21,9 @@ namespace chip { namespace app { -CHIP_ERROR DeferredAttribute::PrepareWrite(AttributePersistenceProvider & persister, const EmberAfAttributeMetadata * metadata, - const ByteSpan & value) +CHIP_ERROR DeferredAttribute::PrepareWrite(System::Clock::Timestamp flushTime, const ByteSpan & value) { - mPersister = &persister; - mMetadata = metadata; + mFlushTime = flushTime; if (mValue.AllocatedSize() != value.size()) { @@ -37,27 +35,26 @@ CHIP_ERROR DeferredAttribute::PrepareWrite(AttributePersistenceProvider & persis return CHIP_NO_ERROR; } -void DeferredAttribute::Flush() +void DeferredAttribute::Flush(AttributePersistenceProvider & persister) { - VerifyOrReturn(mValue); - mPersister->WriteValue(mPath, mMetadata, ByteSpan(mValue.Get(), mValue.AllocatedSize())); + VerifyOrReturn(IsArmed()); + persister.WriteValue(mPath, ByteSpan(mValue.Get(), mValue.AllocatedSize())); mValue.Release(); } -CHIP_ERROR DeferredAttributePersistenceProvider::WriteValue(const ConcreteAttributePath & path, - const EmberAfAttributeMetadata * metadata, const ByteSpan & value) +CHIP_ERROR DeferredAttributePersistenceProvider::WriteValue(const ConcreteAttributePath & path, const ByteSpan & value) { for (DeferredAttribute & da : mDeferredAttributes) { if (da.Matches(path)) { - ReturnErrorOnFailure(da.PrepareWrite(mPersister, metadata, value)); - DeviceLayer::SystemLayer().StartTimer(System::Clock::Milliseconds32(mWriteDelay), Flush, &da); + ReturnErrorOnFailure(da.PrepareWrite(System::SystemClock().GetMonotonicTimestamp() + mWriteDelay, value)); + FlushAndScheduleNext(); return CHIP_NO_ERROR; } } - return mPersister.WriteValue(path, metadata, value); + return mPersister.WriteValue(path, value); } CHIP_ERROR DeferredAttributePersistenceProvider::ReadValue(const ConcreteAttributePath & path, @@ -66,5 +63,36 @@ CHIP_ERROR DeferredAttributePersistenceProvider::ReadValue(const ConcreteAttribu return mPersister.ReadValue(path, metadata, value); } +void DeferredAttributePersistenceProvider::FlushAndScheduleNext() +{ + const System::Clock::Timestamp now = System::SystemClock().GetMonotonicTimestamp(); + System::Clock::Timestamp nextFlushTime = System::Clock::Timestamp::max(); + + for (DeferredAttribute & da : mDeferredAttributes) + { + if (!da.IsArmed()) + { + continue; + } + + if (da.GetFlushTime() <= now) + { + da.Flush(mPersister); + } + else + { + nextFlushTime = chip::min(nextFlushTime, da.GetFlushTime()); + } + } + + if (nextFlushTime != System::Clock::Timestamp::max()) + { + DeviceLayer::SystemLayer().StartTimer( + nextFlushTime - now, + [](System::Layer *, void * me) { static_cast(me)->FlushAndScheduleNext(); }, + this); + } +} + } // namespace app } // namespace chip diff --git a/src/app/DeferredAttributePersistenceProvider.h b/src/app/DeferredAttributePersistenceProvider.h index d3a9519570f6cf..2482d5c9076b33 100644 --- a/src/app/DeferredAttributePersistenceProvider.h +++ b/src/app/DeferredAttributePersistenceProvider.h @@ -28,14 +28,15 @@ class DeferredAttribute explicit DeferredAttribute(const ConcreteAttributePath & path) : mPath(path) {} bool Matches(const ConcreteAttributePath & path) const { return mPath == path; } - CHIP_ERROR PrepareWrite(AttributePersistenceProvider & persister, const EmberAfAttributeMetadata * metadata, - const ByteSpan & value); - void Flush(); + bool IsArmed() const { return static_cast(mValue); } + System::Clock::Timestamp GetFlushTime() const { return mFlushTime; } + + CHIP_ERROR PrepareWrite(System::Clock::Timestamp flushTime, const ByteSpan & value); + void Flush(AttributePersistenceProvider & persister); private: const ConcreteAttributePath mPath; - AttributePersistenceProvider * mPersister; - const EmberAfAttributeMetadata * mMetadata; + System::Clock::Timestamp mFlushTime; Platform::ScopedMemoryBufferWithSize mValue; }; @@ -57,11 +58,20 @@ class DeferredAttributePersistenceProvider : public AttributePersistenceProvider mDeferredAttributes(deferredAttributes), mWriteDelay(writeDelay) {} - CHIP_ERROR WriteValue(const ConcreteAttributePath & path, const EmberAfAttributeMetadata * metadata, const ByteSpan & value); - CHIP_ERROR ReadValue(const ConcreteAttributePath & path, const EmberAfAttributeMetadata * metadata, MutableByteSpan & value); + /* + * If the written attribute is one of the deferred attributes specified in the constructor, + * postpone the write operation by the configured delay. If this attribute changes within the + * delay period, further postpone the operation so that the actual write happens once the + * attribute has remained constant for the write delay period. + * + * For other attributes, immediately pass the write operation to the decorated persister. + */ + CHIP_ERROR WriteValue(const ConcreteAttributePath & path, const ByteSpan & value) override; + CHIP_ERROR ReadValue(const ConcreteAttributePath & path, const EmberAfAttributeMetadata * metadata, + MutableByteSpan & value) override; private: - static void Flush(System::Layer *, void * deferredAttr) { static_cast(deferredAttr)->Flush(); } + void FlushAndScheduleNext(); AttributePersistenceProvider & mPersister; const Span mDeferredAttributes; diff --git a/src/app/server/Server.h b/src/app/server/Server.h index eb7eda58b7741a..25fe77664278b6 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -340,7 +340,7 @@ class Server Credentials::OperationalCertificateStore * GetOpCertStore() { return mOpCertStore; } - app::DefaultAttributePersistenceProvider & GetAttributePersister() { return mAttributePersister; } + app::DefaultAttributePersistenceProvider & GetDefaultAttributePersister() { return mAttributePersister; } /** * This function send the ShutDown event before stopping diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp index 359ef83a26a9a2..38708a52dbac0a 100644 --- a/src/app/util/attribute-storage.cpp +++ b/src/app/util/attribute-storage.cpp @@ -1300,8 +1300,7 @@ void emAfSaveAttributeToStorageIfNeeded(uint8_t * data, EndpointId endpoint, Clu auto * attrStorage = app::GetAttributePersistenceProvider(); if (attrStorage) { - attrStorage->WriteValue(app::ConcreteAttributePath(endpoint, clusterId, metadata->attributeId), metadata, - ByteSpan(data, dataSize)); + attrStorage->WriteValue(app::ConcreteAttributePath(endpoint, clusterId, metadata->attributeId), ByteSpan(data, dataSize)); } else { From 7d71e0356b3cdcbf2178cae62e6c22c3f32b3bd1 Mon Sep 17 00:00:00 2001 From: Damian Krolik Date: Fri, 4 Nov 2022 12:15:41 +0100 Subject: [PATCH 3/3] Add a comment --- examples/lighting-app/nrfconnect/main/AppTask.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/examples/lighting-app/nrfconnect/main/AppTask.cpp b/examples/lighting-app/nrfconnect/main/AppTask.cpp index 7a0e91b0cf8aff..c43a7c16334382 100644 --- a/examples/lighting-app/nrfconnect/main/AppTask.cpp +++ b/examples/lighting-app/nrfconnect/main/AppTask.cpp @@ -90,6 +90,11 @@ bool sHaveBLEConnections = false; chip::DeviceLayer::DeviceInfoProviderImpl gExampleDeviceInfoProvider; +// Define a custom attribute persister which makes actual write of the CurrentLevel attribute value +// to the non-volatile storage only when it has remained constant for 5 seconds. This is to reduce +// the flash wearout when the attribute changes frequently as a result of MoveToLevel command. +// DeferredAttribute object describes a deferred attribute, but also holds a buffer with a value to +// be written, so it must live so long as the DeferredAttributePersistenceProvider object. DeferredAttribute gCurrentLevelPersister(ConcreteAttributePath(kLightEndpointId, Clusters::LevelControl::Id, Clusters::LevelControl::Attributes::CurrentLevel::Id)); DeferredAttributePersistenceProvider gDeferredAttributePersister(Server::GetInstance().GetDefaultAttributePersister(),