Merge branch 'master' into feature/fix-cluster-revisions

Author: lazarkov

src/app/CommandSender.cpp

@@ -224,6 +224,7 @@ CHIP_ERROR CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExcha
     {
         err = ProcessInvokeResponse(std::move(aPayload), moreChunkedMessages);
         SuccessOrExit(err);
+        mInvokeResponseMessageCount++;
         if (moreChunkedMessages)
         {
             StatusResponse::Send(Status::Success, apExchangeContext, /*aExpectResponse = */ true);
@@ -534,6 +535,11 @@ CHIP_ERROR CommandSender::FinishCommand(const Optional<uint16_t> & aTimedInvokeT
     return CHIP_NO_ERROR;
 }
 
+size_t CommandSender::GetInvokeResponseMessageCount()
+{
+    return static_cast<size_t>(mInvokeResponseMessageCount);
+}
+
 CHIP_ERROR CommandSender::Finalize(System::PacketBufferHandle & commandPacket)
 {
     VerifyOrReturnError(mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE);

src/app/CommandSender.h

@@ -318,6 +318,14 @@ class CommandSender final : public Messaging::ExchangeDelegate
         return FinishCommand(aTimedInvokeTimeoutMs, optionalArgs);
     }
 
+    /**
+     * @brief Returns the number of InvokeResponseMessages received.
+     *
+     * Responses to multiple requests might be split across several InvokeResponseMessages.
+     * This function helps track the total count. Primarily for test validation purposes.
+     */
+    size_t GetInvokeResponseMessageCount();
+
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST
     /**
      * Version of AddRequestData that allows sending a message that is
@@ -507,8 +515,10 @@ class CommandSender final : public Messaging::ExchangeDelegate
     TLV::TLVType mDataElementContainerType = TLV::kTLVType_NotSpecified;
 
     chip::System::PacketBufferTLVWriter mCommandMessageWriter;
-    uint16_t mFinishedCommandCount    = 0;
-    uint16_t mRemoteMaxPathsPerInvoke = 1;
+
+    uint16_t mInvokeResponseMessageCount = 0;
+    uint16_t mFinishedCommandCount       = 0;
+    uint16_t mRemoteMaxPathsPerInvoke    = 1;
 
     State mState                = State::Idle;
     bool mSuppressResponse      = false;

src/app/clusters/icd-management-server/icd-management-server.cpp

@@ -149,7 +149,7 @@ CHIP_ERROR IcdManagementAttributeAccess::ReadRegisteredClients(EndpointId endpoi
 
 CHIP_ERROR IcdManagementAttributeAccess::ReadICDCounter(EndpointId endpoint, AttributeValueEncoder & encoder)
 {
-    return encoder.Encode(mICDConfigurationData->GetICDCounter());
+    return encoder.Encode(mICDConfigurationData->GetICDCounter().GetValue());
 }
 
 CHIP_ERROR IcdManagementAttributeAccess::ReadClientsSupportedPerFabric(EndpointId endpoint, AttributeValueEncoder & encoder)
@@ -292,7 +292,7 @@ Status ICDManagementServer::RegisterClient(CommandHandler * commandObj, const Co
         TriggerICDMTableUpdatedEvent();
     }
 
-    icdCounter = mICDConfigurationData->GetICDCounter();
+    icdCounter = mICDConfigurationData->GetICDCounter().GetValue();
     return InteractionModel::Status::Success;
 }
 

src/app/icd/server/BUILD.gn

@@ -115,5 +115,6 @@ source_set("configuration-data") {
     ":icd-server-config",
     "${chip_root}/src/lib/core",
     "${chip_root}/src/lib/support",
+    "${chip_root}/src/protocols/secure_channel:check-in-counter",
   ]
 }

src/app/icd/server/ICDConfigurationData.h

@@ -20,6 +20,7 @@
 #include <lib/core/Optional.h>
 #include <lib/support/TimeUtils.h>
 #include <platform/CHIPDeviceConfig.h>
+#include <protocols/secure_channel/CheckInCounter.h>
 #include <system/SystemClock.h>
 
 namespace chip {
@@ -44,7 +45,7 @@ class TestICDManager;
 class ICDConfigurationData
 {
 public:
-    static constexpr uint32_t ICD_CHECK_IN_COUNTER_MIN_INCREMENT = 100;
+    static constexpr uint32_t kICDCounterPersistenceIncrement = 100;
 
     enum class ICDMode : uint8_t
     {
@@ -60,7 +61,7 @@ class ICDConfigurationData
 
     System::Clock::Milliseconds16 GetActiveModeThreshold() { return mActiveThreshold; }
 
-    uint32_t GetICDCounter() { return mICDCounter; }
+    Protocols::SecureChannel::CheckInCounter & GetICDCounter() { return mICDCounter; }
 
     uint16_t GetClientsSupportedPerFabric() { return mFabricClientsSupported; }
 
@@ -99,7 +100,6 @@ class ICDConfigurationData
     friend class chip::app::TestICDManager;
 
     void SetICDMode(ICDMode mode) { mICDMode = mode; };
-    void SetICDCounter(uint32_t count) { mICDCounter = count; }
     void SetSlowPollingInterval(System::Clock::Milliseconds32 slowPollInterval) { mSlowPollingInterval = slowPollInterval; };
     void SetFastPollingInterval(System::Clock::Milliseconds32 fastPollInterval) { mFastPollingInterval = fastPollInterval; };
 
@@ -137,7 +137,7 @@ class ICDConfigurationData
 
     System::Clock::Milliseconds16 mActiveThreshold = System::Clock::Milliseconds16(CHIP_CONFIG_ICD_ACTIVE_MODE_THRESHOLD_MS);
 
-    uint32_t mICDCounter = 0;
+    Protocols::SecureChannel::CheckInCounter mICDCounter;
 
     static_assert((CHIP_CONFIG_ICD_CLIENTS_SUPPORTED_PER_FABRIC) >= 1,
                   "Spec requires the minimum of supported clients per fabric be equal or greater to 1.");

src/app/icd/server/ICDManager.cpp

@@ -72,7 +72,9 @@ void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricT
     mExchangeManager   = exchangeManager;
     mSubManager        = manager;
 
-    VerifyOrDie(InitCounter() == CHIP_NO_ERROR);
+    VerifyOrDie(ICDConfigurationData::GetInstance().GetICDCounter().Init(mStorage, DefaultStorageKeyAllocator::ICDCheckInCounter(),
+                                                                         ICDConfigurationData::kICDCounterPersistenceIncrement) ==
+                CHIP_NO_ERROR);
 
     UpdateICDMode();
     UpdateOperationState(OperationalState::IdleMode);
@@ -115,7 +117,8 @@ void ICDManager::SendCheckInMsgs()
 #if !CONFIG_BUILD_FOR_HOST_UNIT_TEST
     VerifyOrDie(mStorage != nullptr);
     VerifyOrDie(mFabricTable != nullptr);
-    uint32_t counter        = ICDConfigurationData::GetInstance().GetICDCounter();
+
+    uint32_t counterValue   = ICDConfigurationData::GetInstance().GetICDCounter().GetNextCheckInCounterValue();
     bool counterIncremented = false;
 
     for (const auto & fabricInfo : *mFabricTable)
@@ -156,7 +159,7 @@ void ICDManager::SendCheckInMsgs()
             {
                 counterIncremented = true;
 
-                if (CHIP_NO_ERROR != IncrementCounter())
+                if (CHIP_NO_ERROR != ICDConfigurationData::GetInstance().GetICDCounter().Advance())
                 {
                     ChipLogError(AppServer, "Incremented ICDCounter but failed to access/save to Persistent storage");
                 }
@@ -167,7 +170,7 @@ void ICDManager::SendCheckInMsgs()
             ICDCheckInSender * sender = mICDSenderPool.CreateObject(mExchangeManager);
             VerifyOrReturn(sender != nullptr, ChipLogError(AppServer, "Failed to allocate ICDCheckinSender"));
 
-            if (CHIP_NO_ERROR != sender->RequestResolve(entry, mFabricTable, counter))
+            if (CHIP_NO_ERROR != sender->RequestResolve(entry, mFabricTable, counterValue))
             {
                 ChipLogError(AppServer, "Failed to send ICD Check-In");
             }
@@ -176,54 +179,6 @@ void ICDManager::SendCheckInMsgs()
 #endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST
 }
 
-CHIP_ERROR ICDManager::InitCounter()
-{
-    CHIP_ERROR err;
-    uint32_t temp;
-    uint16_t size = static_cast<uint16_t>(sizeof(uint32_t));
-
-    err = mStorage->SyncGetKeyValue(DefaultStorageKeyAllocator::ICDCheckInCounter().KeyName(), &temp, size);
-    if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND)
-    {
-        // First time retrieving the counter
-        temp = chip::Crypto::GetRandU32();
-    }
-    else if (err != CHIP_NO_ERROR)
-    {
-        return err;
-    }
-
-    ICDConfigurationData::GetInstance().SetICDCounter(temp);
-    temp += ICDConfigurationData::ICD_CHECK_IN_COUNTER_MIN_INCREMENT;
-
-    // Increment the count directly to minimize flash write.
-    return mStorage->SyncSetKeyValue(DefaultStorageKeyAllocator::ICDCheckInCounter().KeyName(), &temp, size);
-}
-
-CHIP_ERROR ICDManager::IncrementCounter()
-{
-    uint32_t temp      = 0;
-    StorageKeyName key = DefaultStorageKeyAllocator::ICDCheckInCounter();
-    uint16_t size      = static_cast<uint16_t>(sizeof(uint32_t));
-
-    ICDConfigurationData::GetInstance().mICDCounter++;
-
-    if (mStorage == nullptr)
-    {
-        return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
-    }
-
-    ReturnErrorOnFailure(mStorage->SyncGetKeyValue(key.KeyName(), &temp, size));
-
-    if (temp == ICDConfigurationData::GetInstance().mICDCounter)
-    {
-        temp = ICDConfigurationData::GetInstance().mICDCounter + ICDConfigurationData::ICD_CHECK_IN_COUNTER_MIN_INCREMENT;
-        return mStorage->SyncSetKeyValue(key.KeyName(), &temp, size);
-    }
-
-    return CHIP_NO_ERROR;
-}
-
 void ICDManager::UpdateICDMode()
 {
     assertChipStackLockedByCurrentThread();

src/app/icd/server/ICDManager.h

@@ -122,10 +122,6 @@ class ICDManager : public ICDListener
      */
     static void OnTransitionToIdle(System::Layer * aLayer, void * appState);
 
-    // ICD Counter
-    CHIP_ERROR IncrementCounter();
-    CHIP_ERROR InitCounter();
-
     uint8_t mOpenExchangeContextCount = 0;
     uint8_t mCheckInRequestCount      = 0;
 

src/app/tests/TestCommandInteraction.cpp

@@ -883,6 +883,7 @@ void TestCommandInteraction::TestCommandInvalidMessage1(nlTestSuite * apSuite, v
     NL_TEST_ASSERT(apSuite,
                    mockCommandSenderDelegate.onResponseCalledTimes == 0 && mockCommandSenderDelegate.onFinalCalledTimes == 1 &&
                        mockCommandSenderDelegate.onErrorCalledTimes == 1);
+    NL_TEST_ASSERT(apSuite, commandSender.GetInvokeResponseMessageCount() == 0);
 
     ctx.DrainAndServiceIO();
 
@@ -1338,12 +1339,14 @@ void TestCommandInteraction::TestCommandSenderCommandSuccessResponseFlow(nlTestS
     err = commandSender.SendCommandRequest(ctx.GetSessionBobToAlice());
 
     NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
+    NL_TEST_ASSERT(apSuite, commandSender.GetInvokeResponseMessageCount() == 0);
 
     ctx.DrainAndServiceIO();
 
     NL_TEST_ASSERT(apSuite,
                    mockCommandSenderDelegate.onResponseCalledTimes == 1 && mockCommandSenderDelegate.onFinalCalledTimes == 1 &&
                        mockCommandSenderDelegate.onErrorCalledTimes == 0);
+    NL_TEST_ASSERT(apSuite, commandSender.GetInvokeResponseMessageCount() == 1);
 
     NL_TEST_ASSERT(apSuite, GetNumActiveHandlerObjects() == 0);
     NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);

src/app/tests/TestICDManager.cpp

@@ -27,6 +27,7 @@
 #include <lib/support/TestPersistentStorageDelegate.h>
 #include <lib/support/TimeUtils.h>
 #include <lib/support/UnitTestContext.h>
+#include <lib/support/UnitTestExtendedAssertions.h>
 #include <lib/support/UnitTestRegistration.h>
 #include <nlunit-test.h>
 #include <system/SystemLayerImpl.h>
@@ -113,7 +114,7 @@ class TestContext : public chip::Test::AppContext
     CHIP_ERROR SetUp() override
     {
         ReturnErrorOnFailure(chip::Test::AppContext::SetUp());
-        mICDManager.Init(&testStorage, &GetFabricTable(), &mKeystore, &GetExchangeManager(), &mSubManager);
+        mICDManager.Init(&testStorage, &GetFabricTable(), &mKeystore, &GetExchangeManager(), &mSubInfoProvider);
         mICDManager.RegisterObserver(&mICDStateObserver);
         return CHIP_NO_ERROR;
     }
@@ -128,12 +129,12 @@ class TestContext : public chip::Test::AppContext
     System::Clock::Internal::MockClock mMockClock;
     TestSessionKeystoreImpl mKeystore;
     app::ICDManager mICDManager;
-    TestSubscriptionsInfoProvider mSubManager;
+    TestSubscriptionsInfoProvider mSubInfoProvider;
     TestPersistentStorageDelegate testStorage;
+    TestICDStateObserver mICDStateObserver;
 
 private:
     System::Clock::ClockBase * mRealClock;
-    TestICDStateObserver mICDStateObserver;
 };
 
 } // namespace
@@ -196,7 +197,7 @@ class TestICDManager
         ctx->mICDManager.SetTestFeatureMapValue(0x07);
 
         // Set that there are no matching subscriptions
-        ctx->mSubManager.SetReturnValue(false);
+        ctx->mSubInfoProvider.SetReturnValue(false);
 
         // Set New durations for test case
         Milliseconds32 oldActiveModeDuration = icdConfigData.GetActiveModeDuration();
@@ -275,7 +276,7 @@ class TestICDManager
         ctx->mICDManager.SetTestFeatureMapValue(0x07);
 
         // Set that there are not matching subscriptions
-        ctx->mSubManager.SetReturnValue(true);
+        ctx->mSubInfoProvider.SetReturnValue(true);
 
         // Set New durations for test case
         Milliseconds32 oldActiveModeDuration = icdConfigData.GetActiveModeDuration();
@@ -506,10 +507,16 @@ class TestICDManager
     static void TestICDCounter(nlTestSuite * aSuite, void * aContext)
     {
         TestContext * ctx = static_cast<TestContext *>(aContext);
-        uint32_t counter  = ICDConfigurationData::GetInstance().GetICDCounter();
-        ctx->mICDManager.IncrementCounter();
-        uint32_t counter2 = ICDConfigurationData::GetInstance().GetICDCounter();
-        NL_TEST_ASSERT(aSuite, (counter + 1) == counter2);
+        uint32_t counter  = ICDConfigurationData::GetInstance().GetICDCounter().GetValue();
+
+        // Shut down and reinit ICDManager to increment counter
+        ctx->mICDManager.Shutdown();
+        ctx->mICDManager.Init(&(ctx->testStorage), &(ctx->GetFabricTable()), &(ctx->mKeystore), &(ctx->GetExchangeManager()),
+                              &(ctx->mSubInfoProvider));
+        ctx->mICDManager.RegisterObserver(&(ctx->mICDStateObserver));
+
+        NL_TEST_ASSERT_EQUALS(aSuite, counter + ICDConfigurationData::kICDCounterPersistenceIncrement,
+                              ICDConfigurationData::GetInstance().GetICDCounter().GetValue());
     }
 
     static void TestOnSubscriptionReport(nlTestSuite * aSuite, void * aContext)