diff --git a/src/app/icd/ICDCheckInSender.cpp b/src/app/icd/ICDCheckInSender.cpp index 8557967eceba1a..9781069b2c743d 100644 --- a/src/app/icd/ICDCheckInSender.cpp +++ b/src/app/icd/ICDCheckInSender.cpp @@ -61,10 +61,7 @@ CHIP_ERROR ICDCheckInSender::SendCheckInMsg(const Transport::PeerAddress & addr) VerifyOrReturnError(!buffer.IsNull(), CHIP_ERROR_NO_MEMORY); MutableByteSpan output{ buffer->Start(), buffer->MaxDataLength() }; - // TODO retrieve Check-in counter - CounterType counter = 0; - - ReturnErrorOnFailure(CheckinMessage::GenerateCheckinMessagePayload(mKey, counter, ByteSpan(), output)); + ReturnErrorOnFailure(CheckinMessage::GenerateCheckinMessagePayload(mKey, mICDCounter, ByteSpan(), output)); buffer->SetDataLength(static_cast(output.size())); VerifyOrReturnError(mExchangeManager->GetSessionManager() != nullptr, CHIP_ERROR_INTERNAL); @@ -81,13 +78,15 @@ CHIP_ERROR ICDCheckInSender::SendCheckInMsg(const Transport::PeerAddress & addr) return exchangeContext->SendMessage(MsgType::ICD_CheckIn, std::move(buffer), Messaging::SendMessageFlags::kNoAutoRequestAck); } -CHIP_ERROR ICDCheckInSender::RequestResolve(ICDMonitoringEntry & entry, FabricTable * fabricTable) +CHIP_ERROR ICDCheckInSender::RequestResolve(ICDMonitoringEntry & entry, FabricTable * fabricTable, uint32_t counter) { VerifyOrReturnError(entry.IsValid(), CHIP_ERROR_INTERNAL); VerifyOrReturnError(fabricTable != nullptr, CHIP_ERROR_INTERNAL); const FabricInfo * fabricInfo = fabricTable->FindFabricWithIndex(entry.fabricIndex); PeerId peerId(fabricInfo->GetCompressedFabricId(), entry.checkInNodeID); + mICDCounter = counter; + AddressResolve::NodeLookupRequest request(peerId); memcpy(mKey.AsMutable(), entry.key.As(), diff --git a/src/app/icd/ICDCheckInSender.h b/src/app/icd/ICDCheckInSender.h index a06efeee419287..0055d66804d0fb 100644 --- a/src/app/icd/ICDCheckInSender.h +++ b/src/app/icd/ICDCheckInSender.h @@ -34,7 +34,7 @@ class ICDCheckInSender : public AddressResolve::NodeListener ICDCheckInSender(Messaging::ExchangeManager * exchangeManager); ~ICDCheckInSender(){}; - CHIP_ERROR RequestResolve(ICDMonitoringEntry & entry, FabricTable * fabricTable); + CHIP_ERROR RequestResolve(ICDMonitoringEntry & entry, FabricTable * fabricTable, uint32_t counter); // AddressResolve::NodeListener - notifications when dnssd finds a node IP address void OnNodeAddressResolved(const PeerId & peerId, const AddressResolve::ResolveResult & result) override; @@ -51,6 +51,8 @@ class ICDCheckInSender : public AddressResolve::NodeListener Messaging::ExchangeManager * mExchangeManager = nullptr; Crypto::Aes128KeyHandle mKey = Crypto::Aes128KeyHandle(); + + uint32_t mICDCounter = 0; }; } // namespace app diff --git a/src/app/icd/ICDConfigurationData.h b/src/app/icd/ICDConfigurationData.h index efb4e7e73867ca..4fd16c92c1a7b1 100644 --- a/src/app/icd/ICDConfigurationData.h +++ b/src/app/icd/ICDConfigurationData.h @@ -42,6 +42,8 @@ class ICDManager; class ICDConfigurationData { public: + static constexpr uint32_t ICD_CHECK_IN_COUNTER_MIN_INCREMENT = 100; + enum class ICDMode : uint8_t { SIT, // Short Interval Time ICD @@ -108,8 +110,6 @@ class ICDConfigurationData uint16_t mActiveThreshold_ms = CHIP_CONFIG_ICD_ACTIVE_MODE_THRESHOLD_MS; - // TODO : Implement ICD counter - // https://github.com/project-chip/connectedhomeip/issues/29184 uint32_t mICDCounter = 0; static_assert((CHIP_CONFIG_ICD_CLIENTS_SUPPORTED_PER_FABRIC) >= 1, diff --git a/src/app/icd/ICDManager.cpp b/src/app/icd/ICDManager.cpp index eac574ca20ab2d..daf312058ee693 100644 --- a/src/app/icd/ICDManager.cpp +++ b/src/app/icd/ICDManager.cpp @@ -64,6 +64,8 @@ void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricT mSymmetricKeystore = symmetricKeystore; mExchangeManager = exchangeManager; + VerifyOrDie(InitCounter() == CHIP_NO_ERROR); + // Removing the check for now since it is possible for the Fast polling // to be larger than the ActiveModeDuration for now // uint32_t activeModeDuration = ICDConfigurationData::GetInstance().GetActiveModeDurationMs(); @@ -105,6 +107,9 @@ void ICDManager::SendCheckInMsgs() #if !CONFIG_BUILD_FOR_HOST_UNIT_TEST VerifyOrDie(mStorage != nullptr); VerifyOrDie(mFabricTable != nullptr); + uint32_t counter = ICDConfigurationData::GetInstance().GetICDCounter(); + bool counterIncremented = false; + for (const auto & fabricInfo : *mFabricTable) { uint16_t supported_clients = ICDConfigurationData::GetInstance().GetClientsSupportedPerFabric(); @@ -139,12 +144,23 @@ void ICDManager::SendCheckInMsgs() continue; } + // Increment counter only once to prevent depletion of the available range. + if (!counterIncremented) + { + counterIncremented = true; + + if (CHIP_NO_ERROR != IncrementCounter()) + { + ChipLogError(AppServer, "Incremented ICDCounter but failed to access/save to Persistent storage"); + } + } + // SenderPool will be released upon transition from active to idle state // This will happen when all ICD Check-In messages are sent on the network ICDCheckInSender * sender = mICDSenderPool.CreateObject(mExchangeManager); VerifyOrReturn(sender != nullptr, ChipLogError(AppServer, "Failed to allocate ICDCheckinSender")); - if (CHIP_NO_ERROR != sender->RequestResolve(entry, mFabricTable)) + if (CHIP_NO_ERROR != sender->RequestResolve(entry, mFabricTable, counter)) { ChipLogError(AppServer, "Failed to send ICD Check-In"); } @@ -153,6 +169,54 @@ 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(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(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(); diff --git a/src/app/icd/ICDManager.h b/src/app/icd/ICDManager.h index 7958adada0de63..4034f1520b34df 100644 --- a/src/app/icd/ICDManager.h +++ b/src/app/icd/ICDManager.h @@ -120,6 +120,10 @@ 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; diff --git a/src/app/tests/TestICDManager.cpp b/src/app/tests/TestICDManager.cpp index e3ca6eccbea768..03f3b7c2a76901 100644 --- a/src/app/tests/TestICDManager.cpp +++ b/src/app/tests/TestICDManager.cpp @@ -314,6 +314,15 @@ class TestICDManager // Check ICDManager is still in the LIT operating mode NL_TEST_ASSERT(aSuite, ICDConfigurationData::GetInstance().GetICDMode() == ICDConfigurationData::ICDMode::SIT); } + + static void TestICDCounter(nlTestSuite * aSuite, void * aContext) + { + TestContext * ctx = static_cast(aContext); + uint32_t counter = ICDConfigurationData::GetInstance().GetICDCounter(); + ctx->mICDManager.IncrementCounter(); + uint32_t counter2 = ICDConfigurationData::GetInstance().GetICDCounter(); + NL_TEST_ASSERT(aSuite, (counter + 1) == counter2); + } }; } // namespace app @@ -329,6 +338,7 @@ static const nlTest sTests[] = NL_TEST_DEF("TestICDModeDurations", TestICDManager::TestICDModeDurations), NL_TEST_DEF("TestKeepActivemodeRequests", TestICDManager::TestKeepActivemodeRequests), NL_TEST_DEF("TestICDMRegisterUnregisterEvents", TestICDManager::TestICDMRegisterUnregisterEvents), + NL_TEST_DEF("TestICDCounter", TestICDManager::TestICDCounter), NL_TEST_SENTINEL() }; // clang-format on diff --git a/src/app/tests/suites/TestIcdManagementCluster.yaml b/src/app/tests/suites/TestIcdManagementCluster.yaml index e07b707e190858..d23c06e96ed208 100644 --- a/src/app/tests/suites/TestIcdManagementCluster.yaml +++ b/src/app/tests/suites/TestIcdManagementCluster.yaml @@ -56,7 +56,10 @@ tests: command: "readAttribute" attribute: "ICDCounter" response: - value: 0 + constraints: + type: int32u + minValue: 0x0 + maxValue: 0xFFFFFFFF - label: "Read ClientsSupportedPerFabric" command: "readAttribute" @@ -136,7 +139,10 @@ tests: response: values: - name: "ICDCounter" - value: 0 + constraints: + type: int32u + minValue: 0x0 + maxValue: 0xFFFFFFFF - label: "Register 2.1" command: "RegisterClient" @@ -152,7 +158,10 @@ tests: response: values: - name: "ICDCounter" - value: 0 + constraints: + type: int32u + minValue: 0x0 + maxValue: 0xFFFFFFFF - label: "Register 3.1" command: "RegisterClient" @@ -190,7 +199,10 @@ tests: response: values: - name: "ICDCounter" - value: 0 + constraints: + type: int32u + minValue: 0x0 + maxValue: 0xFFFFFFFF - label: "Read RegisteredClients" command: "readAttribute" @@ -218,7 +230,10 @@ tests: response: values: - name: "ICDCounter" - value: 0 + constraints: + type: int32u + minValue: 0x0 + maxValue: 0xFFFFFFFF - label: "Read RegisteredClients" command: "readAttribute" diff --git a/src/lib/support/DefaultStorageKeyAllocator.h b/src/lib/support/DefaultStorageKeyAllocator.h index b5193c089eda65..21c0948a98493c 100644 --- a/src/lib/support/DefaultStorageKeyAllocator.h +++ b/src/lib/support/DefaultStorageKeyAllocator.h @@ -134,6 +134,9 @@ class DefaultStorageKeyAllocator static StorageKeyName GroupDataCounter() { return StorageKeyName::FromConst("g/gdc"); } static StorageKeyName GroupControlCounter() { return StorageKeyName::FromConst("g/gcc"); } + // ICD Check-In Counter + static StorageKeyName ICDCheckInCounter() { return StorageKeyName::FromConst("icd/cic"); } + // Device Information Provider static StorageKeyName UserLabelLengthKey(EndpointId endpoint) { return StorageKeyName::Formatted("g/userlbl/%x", endpoint); } static StorageKeyName UserLabelIndexKey(EndpointId endpoint, uint32_t index) diff --git a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h index 1c9892f1a4c767..3f8c5a6b3730fa 100644 --- a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h +++ b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h @@ -55905,10 +55905,9 @@ class TestIcdManagementCluster : public TestCommandBridge { VerifyOrReturn(CheckValue("status", err ? err.code : 0, 0)); - { - id actualValue = value; - VerifyOrReturn(CheckValue("ICDCounter", actualValue, 0UL)); - } + VerifyOrReturn(CheckConstraintType("ICDCounter", "int32u", "int32u")); + VerifyOrReturn(CheckConstraintMinValue("ICDCounter", [value unsignedIntValue], 0UL)); + VerifyOrReturn(CheckConstraintMaxValue("ICDCounter", [value unsignedIntValue], 4294967295UL)); NextTest(); }]; @@ -56129,10 +56128,9 @@ class TestIcdManagementCluster : public TestCommandBridge { VerifyOrReturn(CheckValue("status", err ? err.code : 0, 0)); - { - id actualValue = values.icdCounter; - VerifyOrReturn(CheckValue("ICDCounter", actualValue, 0UL)); - } + VerifyOrReturn(CheckConstraintType("ICDCounter", "int32u", "int32u")); + VerifyOrReturn(CheckConstraintMinValue("ICDCounter", [values.icdCounter unsignedIntValue], 0UL)); + VerifyOrReturn(CheckConstraintMaxValue("ICDCounter", [values.icdCounter unsignedIntValue], 4294967295UL)); NextTest(); }]; @@ -56164,10 +56162,9 @@ class TestIcdManagementCluster : public TestCommandBridge { VerifyOrReturn(CheckValue("status", err ? err.code : 0, 0)); - { - id actualValue = values.icdCounter; - VerifyOrReturn(CheckValue("ICDCounter", actualValue, 0UL)); - } + VerifyOrReturn(CheckConstraintType("ICDCounter", "int32u", "int32u")); + VerifyOrReturn(CheckConstraintMinValue("ICDCounter", [values.icdCounter unsignedIntValue], 0UL)); + VerifyOrReturn(CheckConstraintMaxValue("ICDCounter", [values.icdCounter unsignedIntValue], 4294967295UL)); NextTest(); }]; @@ -56261,10 +56258,9 @@ class TestIcdManagementCluster : public TestCommandBridge { VerifyOrReturn(CheckValue("status", err ? err.code : 0, 0)); - { - id actualValue = values.icdCounter; - VerifyOrReturn(CheckValue("ICDCounter", actualValue, 0UL)); - } + VerifyOrReturn(CheckConstraintType("ICDCounter", "int32u", "int32u")); + VerifyOrReturn(CheckConstraintMinValue("ICDCounter", [values.icdCounter unsignedIntValue], 0UL)); + VerifyOrReturn(CheckConstraintMaxValue("ICDCounter", [values.icdCounter unsignedIntValue], 4294967295UL)); NextTest(); }]; @@ -56331,10 +56327,9 @@ class TestIcdManagementCluster : public TestCommandBridge { VerifyOrReturn(CheckValue("status", err ? err.code : 0, 0)); - { - id actualValue = values.icdCounter; - VerifyOrReturn(CheckValue("ICDCounter", actualValue, 0UL)); - } + VerifyOrReturn(CheckConstraintType("ICDCounter", "int32u", "int32u")); + VerifyOrReturn(CheckConstraintMinValue("ICDCounter", [values.icdCounter unsignedIntValue], 0UL)); + VerifyOrReturn(CheckConstraintMaxValue("ICDCounter", [values.icdCounter unsignedIntValue], 4294967295UL)); NextTest(); }];