diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp index af29020986837a..3fc21e01d03192 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -84,7 +84,7 @@ struct CopyAndAdjustDeltaTimeContext void EventManagement::Init(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer, const LogStorageResources * const apLogStorageResources, - MonotonicallyIncreasingCounter * apEventNumberCounter) + MonotonicallyIncreasingCounter * apEventNumberCounter) { CircularEventBuffer * current = nullptr; CircularEventBuffer * prev = nullptr; @@ -335,7 +335,7 @@ CHIP_ERROR EventManagement::ConstructEvent(EventLoadOutContext * apContext, Even void EventManagement::CreateEventManagement(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer, const LogStorageResources * const apLogStorageResources, - MonotonicallyIncreasingCounter * apEventNumberCounter) + MonotonicallyIncreasingCounter * apEventNumberCounter) { sInstance.Init(apExchangeManager, aNumBuffers, apCircularEventBuffer, apLogStorageResources, apEventNumberCounter); @@ -401,7 +401,7 @@ void EventManagement::VendEventNumber() } // Assign event Number to the buffer's counter's value. - mLastEventNumber = static_cast(mpEventNumberCounter->GetValue()); + mLastEventNumber = mpEventNumberCounter->GetValue(); } CHIP_ERROR EventManagement::LogEvent(EventLoggingDelegate * apDelegate, const EventOptions & aEventOptions, diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h index 8c9130e6765823..8afd26c903aaeb 100644 --- a/src/app/EventManagement.h +++ b/src/app/EventManagement.h @@ -195,7 +195,8 @@ class EventManagement * */ void Init(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer, - const LogStorageResources * const apLogStorageResources, MonotonicallyIncreasingCounter * apEventNumberCounter); + const LogStorageResources * const apLogStorageResources, + MonotonicallyIncreasingCounter * apEventNumberCounter); static EventManagement & GetInstance(); @@ -224,7 +225,7 @@ class EventManagement static void CreateEventManagement(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer, const LogStorageResources * const apLogStorageResources, - MonotonicallyIncreasingCounter * apEventNumberCounter); + MonotonicallyIncreasingCounter * apEventNumberCounter); static void DestroyEventManagement(); @@ -512,7 +513,7 @@ class EventManagement #endif // !CHIP_SYSTEM_CONFIG_NO_LOCKING // The counter we're going to use for event numbers. - MonotonicallyIncreasingCounter * mpEventNumberCounter = nullptr; + MonotonicallyIncreasingCounter * mpEventNumberCounter = nullptr; EventNumber mLastEventNumber = 0; ///< Last event Number vended Timestamp mLastEventTimestamp; ///< The timestamp of the last event in this buffer diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index be6828e73347ff..c6eaa1a709e33e 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -93,7 +93,7 @@ Server Server::sServer; static uint8_t sInfoEventBuffer[CHIP_DEVICE_CONFIG_EVENT_LOGGING_INFO_BUFFER_SIZE]; static uint8_t sDebugEventBuffer[CHIP_DEVICE_CONFIG_EVENT_LOGGING_DEBUG_BUFFER_SIZE]; static uint8_t sCritEventBuffer[CHIP_DEVICE_CONFIG_EVENT_LOGGING_CRIT_BUFFER_SIZE]; -static ::chip::PersistedCounter sGlobalEventIdCounter; +static ::chip::PersistedCounter sGlobalEventIdCounter; static ::chip::app::CircularEventBuffer sLoggingBuffer[CHIP_NUM_EVENT_LOGGING_BUFFERS]; #endif // CHIP_CONFIG_ENABLE_SERVER_IM_EVENT diff --git a/src/app/tests/TestEventLogging.cpp b/src/app/tests/TestEventLogging.cpp index e0f390728be788..a700475aa91f29 100644 --- a/src/app/tests/TestEventLogging.cpp +++ b/src/app/tests/TestEventLogging.cpp @@ -97,7 +97,7 @@ class TestContext : public chip::Test::AppContext } private: - chip::MonotonicallyIncreasingCounter mEventCounter; + chip::MonotonicallyIncreasingCounter mEventCounter; }; void ENFORCE_FORMAT(1, 2) SimpleDumpWriter(const char * aFormat, ...) diff --git a/src/app/tests/TestEventOverflow.cpp b/src/app/tests/TestEventOverflow.cpp index 66e13176b53f0b..aa7e2da475919c 100644 --- a/src/app/tests/TestEventOverflow.cpp +++ b/src/app/tests/TestEventOverflow.cpp @@ -89,7 +89,7 @@ class TestContext : public chip::Test::AppContext } private: - chip::MonotonicallyIncreasingCounter mEventCounter; + chip::MonotonicallyIncreasingCounter mEventCounter; }; class TestEventGenerator : public chip::app::EventLoggingDelegate diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index ad0c3c7a2343a3..53be39ca2bee9a 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -100,7 +100,7 @@ class TestContext : public chip::Test::AppContext } private: - chip::MonotonicallyIncreasingCounter mEventCounter; + chip::MonotonicallyIncreasingCounter mEventCounter; }; TestContext sContext; diff --git a/src/app/tests/integration/chip_im_responder.cpp b/src/app/tests/integration/chip_im_responder.cpp index 6fa6b93bb6bc3c..e360e3137c7612 100644 --- a/src/app/tests/integration/chip_im_responder.cpp +++ b/src/app/tests/integration/chip_im_responder.cpp @@ -160,7 +160,7 @@ uint8_t gInfoEventBuffer[2048]; uint8_t gCritEventBuffer[2048]; chip::app::CircularEventBuffer gCircularEventBuffer[3]; -chip::MonotonicallyIncreasingCounter gEventCounter; +chip::MonotonicallyIncreasingCounter gEventCounter; CHIP_ERROR InitializeEventLogging(chip::Messaging::ExchangeManager * apMgr) { diff --git a/src/controller/tests/TestEventCaching.cpp b/src/controller/tests/TestEventCaching.cpp index cc00ea81bec51d..db71284ead893b 100644 --- a/src/controller/tests/TestEventCaching.cpp +++ b/src/controller/tests/TestEventCaching.cpp @@ -90,7 +90,7 @@ class TestContext : public chip::Test::AppContext } private: - MonotonicallyIncreasingCounter mEventCounter; + MonotonicallyIncreasingCounter mEventCounter; }; nlTestSuite * gSuite = nullptr; diff --git a/src/controller/tests/TestEventChunking.cpp b/src/controller/tests/TestEventChunking.cpp index 98c431426c34c4..bc742348b54349 100644 --- a/src/controller/tests/TestEventChunking.cpp +++ b/src/controller/tests/TestEventChunking.cpp @@ -90,7 +90,7 @@ class TestContext : public chip::Test::AppContext } private: - MonotonicallyIncreasingCounter mEventCounter; + MonotonicallyIncreasingCounter mEventCounter; }; uint32_t gIterationCount = 0; diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.h b/src/include/platform/internal/GenericConfigurationManagerImpl.h index 65d8281f70b1c8..27d9a8d918bbc7 100644 --- a/src/include/platform/internal/GenericConfigurationManagerImpl.h +++ b/src/include/platform/internal/GenericConfigurationManagerImpl.h @@ -122,7 +122,7 @@ class GenericConfigurationManagerImpl : public ConfigurationManager protected: #if CHIP_ENABLE_ROTATING_DEVICE_ID && defined(CHIP_DEVICE_CONFIG_ROTATING_DEVICE_ID_UNIQUE_ID) - chip::LifetimePersistedCounter mLifetimePersistedCounter; + chip::LifetimePersistedCounter mLifetimePersistedCounter; #endif #if CHIP_USE_TRANSITIONAL_COMMISSIONABLE_DATA_PROVIDER diff --git a/src/lib/core/CHIPEncoding.h b/src/lib/core/CHIPEncoding.h index 3210bcea8a5ef8..c1a6783fa1099a 100644 --- a/src/lib/core/CHIPEncoding.h +++ b/src/lib/core/CHIPEncoding.h @@ -207,6 +207,15 @@ inline void Write8(uint8_t *& p, uint8_t v) */ namespace LittleEndian { +/** + * This conditionally performs, as necessary for the target system, a + * byte order swap by value of the specified value, presumed to be in + * little endian byte ordering to the target system (i.e. host) + * byte ordering. + */ +template +inline T HostSwap(T v); + /** * This conditionally performs, as necessary for the target system, a * byte order swap by value of the specified 16-bit value, presumed to @@ -225,6 +234,12 @@ inline uint16_t HostSwap16(uint16_t v) return nl::ByteOrder::Swap16LittleToHost(v); } +template <> +inline uint16_t HostSwap(uint16_t v) +{ + return HostSwap16(v); +} + /** * This conditionally performs, as necessary for the target system, a * byte order swap by value of the specified 32-bit value, presumed to @@ -243,6 +258,12 @@ inline uint32_t HostSwap32(uint32_t v) return nl::ByteOrder::Swap32LittleToHost(v); } +template <> +inline uint32_t HostSwap(uint32_t v) +{ + return HostSwap32(v); +} + /** * This conditionally performs, as necessary for the target system, a * byte order swap by value of the specified 64-bit value, presumed to @@ -261,6 +282,12 @@ inline uint64_t HostSwap64(uint64_t v) return nl::ByteOrder::Swap64LittleToHost(v); } +template <> +inline uint64_t HostSwap(uint64_t v) +{ + return HostSwap64(v); +} + /** * Perform a, potentially unaligned, memory read of the little endian * byte ordered 16-bit value from the specified pointer address, diff --git a/src/lib/support/BUILD.gn b/src/lib/support/BUILD.gn index 9747eec6a67fab..e96066221d5b47 100644 --- a/src/lib/support/BUILD.gn +++ b/src/lib/support/BUILD.gn @@ -70,7 +70,6 @@ static_library("support") { "BytesToHex.cpp", "BytesToHex.h", "CHIPArgParser.cpp", - "CHIPCounter.cpp", "CHIPCounter.h", "CHIPMem.cpp", "CHIPMem.h", @@ -87,10 +86,8 @@ static_library("support") { "FixedBufferAllocator.cpp", "FixedBufferAllocator.h", "Iterators.h", - "LifetimePersistedCounter.cpp", "LifetimePersistedCounter.h", "ObjectLifeCycle.h", - "PersistedCounter.cpp", "PersistedCounter.h", "PersistentStorageMacros.h", "Pool.cpp", diff --git a/src/lib/support/CHIPCounter.cpp b/src/lib/support/CHIPCounter.cpp deleted file mode 100644 index e39929ce5b0edb..00000000000000 --- a/src/lib/support/CHIPCounter.cpp +++ /dev/null @@ -1,53 +0,0 @@ -/* - * - * Copyright (c) 2020 Project CHIP Authors - * Copyright (c) 2016-2017 Nest Labs, Inc. - * All rights reserved. - * - * 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 - -namespace chip { - -MonotonicallyIncreasingCounter::MonotonicallyIncreasingCounter() : mCounterValue(0) {} - -MonotonicallyIncreasingCounter::~MonotonicallyIncreasingCounter() {} - -CHIP_ERROR -MonotonicallyIncreasingCounter::Init(uint32_t aStartValue) -{ - CHIP_ERROR err = CHIP_NO_ERROR; - - mCounterValue = aStartValue; - - return err; -} - -CHIP_ERROR -MonotonicallyIncreasingCounter::Advance() -{ - CHIP_ERROR err = CHIP_NO_ERROR; - - mCounterValue++; - - return err; -} - -uint32_t MonotonicallyIncreasingCounter::GetValue() -{ - return mCounterValue; -} - -} // namespace chip diff --git a/src/lib/support/CHIPCounter.h b/src/lib/support/CHIPCounter.h index 9a55b97c85f7ad..89a7bb751ad645 100644 --- a/src/lib/support/CHIPCounter.h +++ b/src/lib/support/CHIPCounter.h @@ -37,6 +37,7 @@ namespace chip { * An interface for managing a counter as an integer value. */ +template class Counter { public: @@ -57,7 +58,7 @@ class Counter * * @return The current value of the counter. */ - virtual uint32_t GetValue() = 0; + virtual T GetValue() = 0; }; /** @@ -67,11 +68,12 @@ class Counter * A class for managing a monotonically-increasing counter as an integer value. */ -class MonotonicallyIncreasingCounter : public Counter +template +class MonotonicallyIncreasingCounter : public Counter { public: - MonotonicallyIncreasingCounter(); - ~MonotonicallyIncreasingCounter() override; + MonotonicallyIncreasingCounter() : mCounterValue(0) {} + ~MonotonicallyIncreasingCounter() override{}; /** * @brief @@ -81,7 +83,14 @@ class MonotonicallyIncreasingCounter : public Counter * * @return A CHIP error code if something fails, CHIP_NO_ERROR otherwise */ - CHIP_ERROR Init(uint32_t aStartValue); + CHIP_ERROR Init(T aStartValue) + { + CHIP_ERROR err = CHIP_NO_ERROR; + + mCounterValue = aStartValue; + + return err; + } /** * @brief @@ -89,7 +98,14 @@ class MonotonicallyIncreasingCounter : public Counter * * @return A CHIP error code if something fails, CHIP_NO_ERROR otherwise */ - CHIP_ERROR Advance() override; + CHIP_ERROR Advance() override + { + CHIP_ERROR err = CHIP_NO_ERROR; + + mCounterValue++; + + return err; + } /** * @brief @@ -97,10 +113,10 @@ class MonotonicallyIncreasingCounter : public Counter * * @return The current value of the counter. */ - uint32_t GetValue() override; + T GetValue() override { return mCounterValue; } protected: - uint32_t mCounterValue; + T mCounterValue; }; } // namespace chip diff --git a/src/lib/support/LifetimePersistedCounter.cpp b/src/lib/support/LifetimePersistedCounter.cpp deleted file mode 100644 index eeb9be79d4c883..00000000000000 --- a/src/lib/support/LifetimePersistedCounter.cpp +++ /dev/null @@ -1,71 +0,0 @@ -/* - * - * Copyright (c) 2021 Project CHIP Authors - * All rights reserved. - * - * 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 "LifetimePersistedCounter.h" - -#include -#include -#include - -#include -#include - -namespace chip { - -LifetimePersistedCounter::LifetimePersistedCounter() : mId(chip::Platform::PersistedStorage::kEmptyKey) {} - -LifetimePersistedCounter::~LifetimePersistedCounter() {} - -CHIP_ERROR -LifetimePersistedCounter::Init(const chip::Platform::PersistedStorage::Key aId) -{ - mId = aId; - uint32_t startValue; - - // Read our previously-stored starting value. - ReturnErrorOnFailure(ReadStartValue(startValue)); - - // This will set the starting value, after which we're ready. - return MonotonicallyIncreasingCounter::Init(startValue); -} - -CHIP_ERROR -LifetimePersistedCounter::Advance() -{ - VerifyOrReturnError(mId != chip::Platform::PersistedStorage::kEmptyKey, CHIP_ERROR_INCORRECT_STATE); - - ReturnErrorOnFailure(MonotonicallyIncreasingCounter::Advance()); - - return chip::Platform::PersistedStorage::Write(mId, GetValue()); -} - -CHIP_ERROR -LifetimePersistedCounter::ReadStartValue(uint32_t & aStartValue) -{ - aStartValue = 0; - - CHIP_ERROR err = chip::Platform::PersistedStorage::Read(mId, aStartValue); - if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND) - { - // No previously-stored value, no worries, the counter is initialized to zero. - // Suppress the error. - err = CHIP_NO_ERROR; - } - return err; -} - -} // namespace chip diff --git a/src/lib/support/LifetimePersistedCounter.h b/src/lib/support/LifetimePersistedCounter.h index bfde19bc3ca099..7394615aede062 100644 --- a/src/lib/support/LifetimePersistedCounter.h +++ b/src/lib/support/LifetimePersistedCounter.h @@ -49,11 +49,13 @@ namespace chip { * - Output: 0, 1, 2, 3, 4 * */ -class LifetimePersistedCounter : public MonotonicallyIncreasingCounter + +template +class LifetimePersistedCounter : public MonotonicallyIncreasingCounter { public: - LifetimePersistedCounter(); - ~LifetimePersistedCounter() override; + LifetimePersistedCounter() : mId(chip::Platform::PersistedStorage::kEmptyKey) {} + ~LifetimePersistedCounter() override = default; /** * @brief @@ -67,7 +69,17 @@ class LifetimePersistedCounter : public MonotonicallyIncreasingCounter * CHIP_ERROR_INVALID_INTEGER_VALUE if aEpoch is 0. * CHIP_NO_ERROR otherwise */ - CHIP_ERROR Init(chip::Platform::PersistedStorage::Key aId); + CHIP_ERROR Init(const chip::Platform::PersistedStorage::Key aId) + { + mId = aId; + T startValue; + + // Read our previously-stored starting value. + ReturnErrorOnFailure(ReadStartValue(startValue)); + + // This will set the starting value, after which we're ready. + return MonotonicallyIncreasingCounter::Init(startValue); + } /** * @brief @@ -76,19 +88,16 @@ class LifetimePersistedCounter : public MonotonicallyIncreasingCounter * * @return Any error returned by a write to persisted storage. */ - CHIP_ERROR Advance() override; + CHIP_ERROR Advance() override + { + VerifyOrReturnError(mId != chip::Platform::PersistedStorage::kEmptyKey, CHIP_ERROR_INCORRECT_STATE); -private: - /** - * @brief - * Write out the counter value to persistent storage. - * - * @param[in] aStartValue The counter value to write out. - * - * @return Any error returned by a write to persistent storage. - */ - CHIP_ERROR PersistNextEpochStart(uint32_t aStartValue); + ReturnErrorOnFailure(MonotonicallyIncreasingCounter::Advance()); + return chip::Platform::PersistedStorage::Write(mId, MonotonicallyIncreasingCounter::GetValue()); + } + +private: /** * @brief * Read our starting counter value (if we have one) in from persistent storage. @@ -97,7 +106,19 @@ class LifetimePersistedCounter : public MonotonicallyIncreasingCounter * * @return Any error returned by a read from persistent storage. */ - CHIP_ERROR ReadStartValue(uint32_t & aStartValue); + CHIP_ERROR ReadStartValue(T & aStartValue) + { + aStartValue = 0; + + CHIP_ERROR err = chip::Platform::PersistedStorage::Read(mId, aStartValue); + if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND) + { + // No previously-stored value, no worries, the counter is initialized to zero. + // Suppress the error. + err = CHIP_NO_ERROR; + } + return err; + } chip::Platform::PersistedStorage::Key mId; // start value is stored here }; diff --git a/src/lib/support/PersistedCounter.cpp b/src/lib/support/PersistedCounter.cpp deleted file mode 100644 index 38b72c50268131..00000000000000 --- a/src/lib/support/PersistedCounter.cpp +++ /dev/null @@ -1,131 +0,0 @@ -/* - * - * Copyright (c) 2020 Project CHIP Authors - * Copyright (c) 2016-2017 Nest Labs, Inc. - * All rights reserved. - * - * 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 "PersistedCounter.h" - -#include -#include -#include - -#include -#include - -namespace chip { - -PersistedCounter::PersistedCounter() {} - -PersistedCounter::~PersistedCounter() {} - -CHIP_ERROR -PersistedCounter::Init(PersistentStorageDelegate * aStorage, KeyType aKey, uint32_t aEpoch) -{ - VerifyOrReturnError(aStorage != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(aKey != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(aEpoch > 0, CHIP_ERROR_INVALID_INTEGER_VALUE); - - mStorage = aStorage; - mKey = aKey; - mEpoch = aEpoch; - - uint32_t startValue; - - // Read our previously-stored starting value. - ReturnErrorOnFailure(ReadStartValue(startValue)); - -#if CHIP_CONFIG_PERSISTED_COUNTER_DEBUG_LOGGING - ChipLogDetail(EventLogging, "PersistedCounter::Init() aEpoch 0x%x startValue 0x%x", aEpoch, startValue); -#endif - - ReturnErrorOnFailure(PersistNextEpochStart(startValue + aEpoch)); - - // This will set the starting value, after which we're ready. - return MonotonicallyIncreasingCounter::Init(startValue); -} - -CHIP_ERROR -PersistedCounter::Advance() -{ - VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE); - VerifyOrReturnError(mKey != nullptr, CHIP_ERROR_INCORRECT_STATE); - - ReturnErrorOnFailure(MonotonicallyIncreasingCounter::Advance()); - - if (GetValue() >= mNextEpoch) - { - // Value advanced past the previously persisted "start point". - // Ensure that a new starting point is persisted. - ReturnErrorOnFailure(PersistNextEpochStart(mNextEpoch + mEpoch)); - - // Advancing the epoch should have ensured that the current value - // is valid - VerifyOrReturnError(GetValue() < mNextEpoch, CHIP_ERROR_INTERNAL); - } - return CHIP_NO_ERROR; -} - -CHIP_ERROR -PersistedCounter::PersistNextEpochStart(uint32_t aStartValue) -{ - mNextEpoch = aStartValue; -#if CHIP_CONFIG_PERSISTED_COUNTER_DEBUG_LOGGING - ChipLogDetail(EventLogging, "PersistedCounter::WriteStartValue() aStartValue 0x%x", aStartValue); -#endif - - uint32_t valueLE = Encoding::LittleEndian::HostSwap32(aStartValue); - - DefaultStorageKeyAllocator keyAlloc; - return mStorage->SyncSetKeyValue((keyAlloc.*mKey)(), &valueLE, sizeof(valueLE)); -} - -CHIP_ERROR -PersistedCounter::ReadStartValue(uint32_t & aStartValue) -{ - DefaultStorageKeyAllocator keyAlloc; - uint32_t valueLE = 0; - uint16_t size = sizeof(valueLE); - - CHIP_ERROR err = mStorage->SyncGetKeyValue((keyAlloc.*mKey)(), &valueLE, size); - if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND) - { - // No previously-stored value, no worries, the counter is initialized to zero. - // Suppress the error. - err = CHIP_NO_ERROR; - } - else - { - // TODO: Figure out how to avoid a bootloop here. Maybe we should just - // init to 0? Or a random value? - ReturnErrorOnFailure(err); - } - - if (size != sizeof(valueLE)) - { - // TODO: Again, figure out whether this could lead to bootloops. - return CHIP_ERROR_INCORRECT_STATE; - } - - aStartValue = Encoding::LittleEndian::HostSwap32(valueLE); - -#if CHIP_CONFIG_PERSISTED_COUNTER_DEBUG_LOGGING - ChipLogDetail(EventLogging, "PersistedCounter::ReadStartValue() aStartValue 0x%x", aStartValue); -#endif - - return CHIP_NO_ERROR; -} - -} // namespace chip diff --git a/src/lib/support/PersistedCounter.h b/src/lib/support/PersistedCounter.h index 0dccee9bbd5a57..f17434aab40305 100644 --- a/src/lib/support/PersistedCounter.h +++ b/src/lib/support/PersistedCounter.h @@ -27,6 +27,7 @@ #pragma once +#include #include #include #include @@ -54,11 +55,12 @@ namespace chip { * - Output: 400, 401 ... * */ -class PersistedCounter : public MonotonicallyIncreasingCounter +template +class PersistedCounter : public MonotonicallyIncreasingCounter { public: - PersistedCounter(); - ~PersistedCounter() override; + PersistedCounter() {} + ~PersistedCounter() override {} typedef const char * (DefaultStorageKeyAllocator::*KeyType)(); @@ -75,7 +77,39 @@ class PersistedCounter : public MonotonicallyIncreasingCounter * CHIP_ERROR_INVALID_INTEGER_VALUE if aEpoch is 0. * CHIP_NO_ERROR otherwise */ - CHIP_ERROR Init(PersistentStorageDelegate * aStorage, KeyType aKey, uint32_t aEpoch); + CHIP_ERROR Init(PersistentStorageDelegate * aStorage, KeyType aKey, T aEpoch) + { + VerifyOrReturnError(aStorage != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(aKey != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(aEpoch > 0, CHIP_ERROR_INVALID_INTEGER_VALUE); + + mStorage = aStorage; + mKey = aKey; + mEpoch = aEpoch; + + T startValue; + + // Read our previously-stored starting value. + ReturnErrorOnFailure(ReadStartValue(startValue)); + +#if CHIP_CONFIG_PERSISTED_COUNTER_DEBUG_LOGGING + // Compiler should optimize these branches. + if (is_same_v) + { + ChipLogDetail(EventLogging, "PersistedCounter::Init() aEpoch 0x%" PRIx64 " startValue 0x%" PRIx64, aEpoch, startValue); + } + else if (is_same_v) + { + ChipLogDetail(EventLogging, "PersistedCounter::Init() aEpoch 0x%" PRIx32 " startValue 0x%" PRIx32, + static_cast(aEpoch), static_cast(startValue)); + } +#endif + + ReturnErrorOnFailure(PersistNextEpochStart(startValue + aEpoch)); + + // This will set the starting value, after which we're ready. + return MonotonicallyIncreasingCounter::Init(startValue); + } /** * @brief @@ -84,7 +118,25 @@ class PersistedCounter : public MonotonicallyIncreasingCounter * * @return Any error returned by a write to persisted storage. */ - CHIP_ERROR Advance() override; + CHIP_ERROR Advance() override + { + VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mKey != nullptr, CHIP_ERROR_INCORRECT_STATE); + + ReturnErrorOnFailure(MonotonicallyIncreasingCounter::Advance()); + + if (MonotonicallyIncreasingCounter::GetValue() >= mNextEpoch) + { + // Value advanced past the previously persisted "start point". + // Ensure that a new starting point is persisted. + ReturnErrorOnFailure(PersistNextEpochStart(mNextEpoch + mEpoch)); + + // Advancing the epoch should have ensured that the current value + // is valid + VerifyOrReturnError(MonotonicallyIncreasingCounter::GetValue() < mNextEpoch, CHIP_ERROR_INTERNAL); + } + return CHIP_NO_ERROR; + } private: /** @@ -95,7 +147,27 @@ class PersistedCounter : public MonotonicallyIncreasingCounter * * @return Any error returned by a write to persistent storage. */ - CHIP_ERROR PersistNextEpochStart(uint32_t aStartValue); + CHIP_ERROR PersistNextEpochStart(T aStartValue) + { + mNextEpoch = aStartValue; +#if CHIP_CONFIG_PERSISTED_COUNTER_DEBUG_LOGGING + // Compiler should optimize these branches. + if (is_same_v) + { + ChipLogDetail(EventLogging, "PersistedCounter::WriteStartValue() aStartValue 0x%" PRIx64, aStartValue); + } + else + { + ChipLogDetail(EventLogging, "PersistedCounter::WriteStartValue() aStartValue 0x%" PRIx32, + static_cast(aStartValue)); + } +#endif + + T valueLE = Encoding::LittleEndian::HostSwap(aStartValue); + + DefaultStorageKeyAllocator keyAlloc; + return mStorage->SyncSetKeyValue((keyAlloc.*mKey)(), &valueLE, sizeof(valueLE)); + } /** * @brief @@ -105,12 +177,45 @@ class PersistedCounter : public MonotonicallyIncreasingCounter * * @return Any error returned by a read from persistent storage. */ - CHIP_ERROR ReadStartValue(uint32_t & aStartValue); + CHIP_ERROR ReadStartValue(T & aStartValue) + { + DefaultStorageKeyAllocator keyAlloc; + T valueLE = 0; + uint16_t size = sizeof(valueLE); + + CHIP_ERROR err = mStorage->SyncGetKeyValue((keyAlloc.*mKey)(), &valueLE, size); + if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND) + { + // No previously-stored value, no worries, the counter is initialized to zero. + // Suppress the error. + err = CHIP_NO_ERROR; + } + else + { + // TODO: Figure out how to avoid a bootloop here. Maybe we should just + // init to 0? Or a random value? + ReturnErrorOnFailure(err); + } + + if (size != sizeof(valueLE)) + { + // TODO: Again, figure out whether this could lead to bootloops. + return CHIP_ERROR_INCORRECT_STATE; + } + + aStartValue = Encoding::LittleEndian::HostSwap(valueLE); + +#if CHIP_CONFIG_PERSISTED_COUNTER_DEBUG_LOGGING + ChipLogDetail(EventLogging, "PersistedCounter::ReadStartValue() aStartValue 0x%x", aStartValue); +#endif + + return CHIP_NO_ERROR; + } PersistentStorageDelegate * mStorage = nullptr; // start value is stored here KeyType mKey = nullptr; - uint32_t mEpoch = 0; // epoch modulus value - uint32_t mNextEpoch = 0; // next epoch start + T mEpoch = 0; // epoch modulus value + T mNextEpoch = 0; // next epoch start }; } // namespace chip diff --git a/src/lib/support/tests/TestCHIPCounter.cpp b/src/lib/support/tests/TestCHIPCounter.cpp index df9a3deb346cfd..e4b64a36db773a 100644 --- a/src/lib/support/tests/TestCHIPCounter.cpp +++ b/src/lib/support/tests/TestCHIPCounter.cpp @@ -23,13 +23,13 @@ static void CheckStartWithZero(nlTestSuite * inSuite, void * inContext) { - chip::MonotonicallyIncreasingCounter counter; + chip::MonotonicallyIncreasingCounter counter; NL_TEST_ASSERT(inSuite, counter.GetValue() == 0); } static void CheckInitialize(nlTestSuite * inSuite, void * inContext) { - chip::MonotonicallyIncreasingCounter counter; + chip::MonotonicallyIncreasingCounter counter; NL_TEST_ASSERT(inSuite, counter.Init(4321) == CHIP_NO_ERROR); NL_TEST_ASSERT(inSuite, counter.GetValue() == 4321); @@ -37,7 +37,7 @@ static void CheckInitialize(nlTestSuite * inSuite, void * inContext) static void CheckAdvance(nlTestSuite * inSuite, void * inContext) { - chip::MonotonicallyIncreasingCounter counter; + chip::MonotonicallyIncreasingCounter counter; NL_TEST_ASSERT(inSuite, counter.Init(22) == CHIP_NO_ERROR); NL_TEST_ASSERT(inSuite, counter.GetValue() == 22); diff --git a/src/lib/support/tests/TestPersistedCounter.cpp b/src/lib/support/tests/TestPersistedCounter.cpp index 76859d0b840460..98a316f30509d1 100644 --- a/src/lib/support/tests/TestPersistedCounter.cpp +++ b/src/lib/support/tests/TestPersistedCounter.cpp @@ -98,7 +98,7 @@ static void CheckOOB(nlTestSuite * inSuite, void * inContext) // a count of 0 and a value of 0x10000 for the next starting value // in persistent storage. - chip::PersistedCounter counter; + chip::PersistedCounter counter; auto testKey = &chip::DefaultStorageKeyAllocator::IMEventNumber; CHIP_ERROR err = counter.Init(sPersistentStore, testKey, 0x10000); @@ -114,7 +114,7 @@ static void CheckReboot(nlTestSuite * inSuite, void * inContext) InitializePersistedStorage(context); - chip::PersistedCounter counter, counter2; + chip::PersistedCounter counter, counter2; // When initializing the first time out of the box, we should have // a count of 0. @@ -142,7 +142,7 @@ static void CheckWriteNextCounterStart(nlTestSuite * inSuite, void * inContext) InitializePersistedStorage(context); - chip::PersistedCounter counter; + chip::PersistedCounter counter; // When initializing the first time out of the box, we should have // a count of 0.