Skip to content

Commit

Permalink
[ICD] Implement ICD Check-In Counter (#30705)
Browse files Browse the repository at this point in the history
* Implement ICD Counter

* Fix Integration tests

* fix yaml

* fix darwin test gen
  • Loading branch information
jepenven-silabs authored and pull[bot] committed Feb 5, 2024
1 parent 4576e6f commit 1a084cb
Show file tree
Hide file tree
Showing 9 changed files with 126 additions and 34 deletions.
9 changes: 4 additions & 5 deletions src/app/icd/ICDCheckInSender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint16_t>(output.size()));

VerifyOrReturnError(mExchangeManager->GetSessionManager() != nullptr, CHIP_ERROR_INTERNAL);
Expand All @@ -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<Crypto::Aes128KeyByteArray>(), entry.key.As<Crypto::Aes128KeyByteArray>(),
Expand Down
4 changes: 3 additions & 1 deletion src/app/icd/ICDCheckInSender.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -51,6 +51,8 @@ class ICDCheckInSender : public AddressResolve::NodeListener
Messaging::ExchangeManager * mExchangeManager = nullptr;

Crypto::Aes128KeyHandle mKey = Crypto::Aes128KeyHandle();

uint32_t mICDCounter = 0;
};

} // namespace app
Expand Down
4 changes: 2 additions & 2 deletions src/app/icd/ICDConfigurationData.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
66 changes: 65 additions & 1 deletion src/app/icd/ICDManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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");
}
Expand All @@ -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<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();
Expand Down
4 changes: 4 additions & 0 deletions src/app/icd/ICDManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
10 changes: 10 additions & 0 deletions src/app/tests/TestICDManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<TestContext *>(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
Expand All @@ -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
Expand Down
25 changes: 20 additions & 5 deletions src/app/tests/suites/TestIcdManagementCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ tests:
command: "readAttribute"
attribute: "ICDCounter"
response:
value: 0
constraints:
type: int32u
minValue: 0x0
maxValue: 0xFFFFFFFF

- label: "Read ClientsSupportedPerFabric"
command: "readAttribute"
Expand Down Expand Up @@ -136,7 +139,10 @@ tests:
response:
values:
- name: "ICDCounter"
value: 0
constraints:
type: int32u
minValue: 0x0
maxValue: 0xFFFFFFFF

- label: "Register 2.1"
command: "RegisterClient"
Expand All @@ -152,7 +158,10 @@ tests:
response:
values:
- name: "ICDCounter"
value: 0
constraints:
type: int32u
minValue: 0x0
maxValue: 0xFFFFFFFF

- label: "Register 3.1"
command: "RegisterClient"
Expand Down Expand Up @@ -190,7 +199,10 @@ tests:
response:
values:
- name: "ICDCounter"
value: 0
constraints:
type: int32u
minValue: 0x0
maxValue: 0xFFFFFFFF

- label: "Read RegisteredClients"
command: "readAttribute"
Expand Down Expand Up @@ -218,7 +230,10 @@ tests:
response:
values:
- name: "ICDCounter"
value: 0
constraints:
type: int32u
minValue: 0x0
maxValue: 0xFFFFFFFF

- label: "Read RegisteredClients"
command: "readAttribute"
Expand Down
3 changes: 3 additions & 0 deletions src/lib/support/DefaultStorageKeyAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
35 changes: 15 additions & 20 deletions zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 1a084cb

Please sign in to comment.