Skip to content

Commit

Permalink
Add injectable ICD Check-In BackOff strategy
Browse files Browse the repository at this point in the history
  • Loading branch information
mkardous-silabs committed Jul 25, 2024
1 parent 6fbe28b commit 1459034
Show file tree
Hide file tree
Showing 11 changed files with 267 additions and 27 deletions.
17 changes: 17 additions & 0 deletions src/app/icd/server/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,22 @@ source_set("notifier") {
]
}

source_set("check-in-back-off") {
sources = [ "ICDCheckInBackOffStrategy.h" ]

public_deps = [
":monitoring-table",
"${chip_root}/src/lib/core",
"${chip_root}/src/lib/support",
]
}

source_set("default-check-in-back-off") {
sources = [ "DefaultICDCheckInBackOffStrategy.h" ]

public_deps = [ ":check-in-back-off" ]
}

# ICD Manager source-set is broken out of the main source-set to enable unit tests
# All sources and configurations used by the ICDManager need to go in this source-set
source_set("manager") {
Expand All @@ -89,6 +105,7 @@ source_set("manager") {

if (chip_enable_icd_checkin) {
public_deps += [
":check-in-back-off",
":monitoring-table",
":sender",
"${chip_root}/src/app:app_config",
Expand Down
63 changes: 63 additions & 0 deletions src/app/icd/server/DefaultICDCheckInBackOffStrategy.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
*
* Copyright (c) 2024 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 <app/icd/server/ICDCheckInBackOffStrategy.h>
#include <lib/core/ClusterEnums.h>

namespace chip {
namespace app {

/**
* @brief Default ICD Check-In BackOff Strategy.
* The default strategy is based on the two types of controllers
* - kPermanent : Always send a Check-In message
* - kEphemeral : Never send a Check-In message
*
* This implementation represents a no back off strategy.
*/
class DefaultICDCheckInBackOffStrategy : public ICDCheckInBackOffStrategy
{
public:
DefaultICDCheckInBackOffStrategy() = default;
~DefaultICDCheckInBackOffStrategy() = default;

/**
* @brief Function checks if the entry is a permanent or ephemeral client.
* If the client is permanent, we should send a Check-In message.
* If the cliet is ephemerakl, we should not send a Check-In message.
*
* @param entry Entry for which we are deciding if we need to send a Check-In message or not.
* @return true If the client is permanent, return true.
* @return false If the client is not permanent, ephemeral or invalid, return false.
*/
bool ShouldSendCheckInMessage(const ICDMonitoringEntry & entry)
{
return (entry.clientType == Clusters::IcdManagement::ClientTypeEnum::kPermanent);
}

/**
* @brief The default Check-In BackOff fundamentally implements a no back off strategy.
* As such, we don't need to execute anything to force the maximum Check-In BackOff.
*
*/
CHIP_ERROR ForceMaximumCheckInBackoff() { return CHIP_NO_ERROR; }
};

} // namespace app
} // namespace chip
62 changes: 62 additions & 0 deletions src/app/icd/server/ICDCheckInBackOffStrategy.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
*
* Copyright (c) 2024 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 <app/icd/server/ICDMonitoringTable.h>
#include <lib/core/CHIPError.h>

namespace chip {
namespace app {

/**
* @brief This class defines the necessary interface a ICD Check-In BackOff strategy needs to implment to be consummed by the
* ICDManager class. The strategy is injected with the init server params when initializing the device Server class.
*/
class ICDCheckInBackOffStrategy
{
public:
virtual ~ICDCheckInBackOffStrategy() = default;

/**
* @brief Function is used by the ICDManager to determine if a Check-In message should be sent to the given entry based on the
* Check-In BackOff strategy.
*
* There are no requirements on how the Check-In BackOff strategy should behave.
* The only specified requirement is the maximum time between to Check-In message, MaximumCheckInBackOff.
* All strategies must respect this requirement.
*
* @param entry ICDMonitoringEntry for which we are about to send a Check-In message to.
*
* @return true ICDCheckInBackOffStrategy determines that we SHOULD send a Check-In message to the given entry
* @return falseI CDCheckInBackOffStrategy determines that we SHOULD NOT send a Check-In message to the given entry
*/
virtual bool ShouldSendCheckInMessage(const ICDMonitoringEntry & entry) = 0;

/**
* @brief Function is used within the test event trigger to force the maximum BackOff state of the ICD Check-In BackOff
* strategy. This enables to validate the strategy and to certify it respects the MaximumCheckInBackOff interval during
* certification.
*
* Function sets the maxmimum BackOff state for all clients registered with the ICD
*
* @return CHIP_ERROR Any error returned during the forcing of the maximum BackOff state
*/
virtual CHIP_ERROR ForceMaximumCheckInBackoff() = 0;
};

} // namespace app
} // namespace chip
34 changes: 20 additions & 14 deletions src/app/icd/server/ICDManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,11 @@
namespace {
enum class ICDTestEventTriggerEvent : uint64_t
{
kAddActiveModeReq = 0x0046'0000'00000001,
kRemoveActiveModeReq = 0x0046'0000'00000002,
kInvalidateHalfCounterValues = 0x0046'0000'00000003,
kInvalidateAllCounterValues = 0x0046'0000'00000004,
kAddActiveModeReq = 0x0046'0000'00000001,
kRemoveActiveModeReq = 0x0046'0000'00000002,
kInvalidateHalfCounterValues = 0x0046'0000'00000003,
kInvalidateAllCounterValues = 0x0046'0000'00000004,
kForceMaximumCheckInBackOffState = 0x0046'0000'00000005,
};
} // namespace

Expand All @@ -52,14 +53,16 @@ static_assert(UINT8_MAX >= CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS,
"ICDManager::mOpenExchangeContextCount cannot hold count for the max exchange count");

void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricTable, Crypto::SymmetricKeystore * symmetricKeystore,
Messaging::ExchangeManager * exchangeManager, SubscriptionsInfoProvider * subInfoProvider)
Messaging::ExchangeManager * exchangeManager, SubscriptionsInfoProvider * subInfoProvider,
ICDCheckInBackOffStrategy * strategy)
{
#if CHIP_CONFIG_ENABLE_ICD_CIP
VerifyOrDie(storage != nullptr);
VerifyOrDie(fabricTable != nullptr);
VerifyOrDie(symmetricKeystore != nullptr);
VerifyOrDie(exchangeManager != nullptr);
VerifyOrDie(subInfoProvider != nullptr);
VerifyOrDie(strategy != nullptr);
#endif // CHIP_CONFIG_ENABLE_ICD_CIP

#if CHIP_CONFIG_ENABLE_ICD_LIT
Expand All @@ -82,11 +85,12 @@ void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricT
VerifyOrDie(ICDNotifier::GetInstance().Subscribe(this) == CHIP_NO_ERROR);

#if CHIP_CONFIG_ENABLE_ICD_CIP
mStorage = storage;
mFabricTable = fabricTable;
mSymmetricKeystore = symmetricKeystore;
mExchangeManager = exchangeManager;
mSubInfoProvider = subInfoProvider;
mStorage = storage;
mFabricTable = fabricTable;
mSymmetricKeystore = symmetricKeystore;
mExchangeManager = exchangeManager;
mSubInfoProvider = subInfoProvider;
mICDCheckInBackOffStrategy = strategy;

VerifyOrDie(ICDConfigurationData::GetInstance().GetICDCounter().Init(mStorage, DefaultStorageKeyAllocator::ICDCheckInCounter(),
ICDConfigurationData::kICDCounterPersistenceIncrement) ==
Expand Down Expand Up @@ -188,15 +192,15 @@ void ICDManager::SendCheckInMsgs()
continue;
}

if (entry.clientType == ClientTypeEnum::kEphemeral)
if (!ShouldCheckInMsgsBeSentAtActiveModeFunction(entry.fabricIndex, entry.monitoredSubject))
{
// If the registered client is ephemeral, do not send a Check-In message
// continue to next entry
continue;
}

if (!ShouldCheckInMsgsBeSentAtActiveModeFunction(entry.fabricIndex, entry.monitoredSubject))
// Validate Check-In BackOff strategy if we should send a Check-In message.
if (!mICDCheckInBackOffStrategy->ShouldSendCheckInMessage(entry))
{
// continue to next entry
continue;
}

Expand Down Expand Up @@ -689,6 +693,8 @@ CHIP_ERROR ICDManager::HandleEventTrigger(uint64_t eventTrigger)
case ICDTestEventTriggerEvent::kInvalidateAllCounterValues:
err = ICDConfigurationData::GetInstance().GetICDCounter().InvalidateAllCheckInCounterValues();
break;
case ICDTestEventTriggerEvent::kForceMaximumCheckInBackOffState:
err = mICDCheckInBackOffStrategy->ForceMaximumCheckInBackoff();
#endif // CHIP_CONFIG_ENABLE_ICD_CIP
default:
err = CHIP_ERROR_INVALID_ARGUMENT;
Expand Down
21 changes: 12 additions & 9 deletions src/app/icd/server/ICDManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@
#include <system/SystemClock.h>

#if CHIP_CONFIG_ENABLE_ICD_CIP
#include <app/icd/server/ICDCheckInSender.h> // nogncheck
#include <app/icd/server/ICDMonitoringTable.h> // nogncheck
#endif // CHIP_CONFIG_ENABLE_ICD_CIP
#include <app/icd/server/ICDCheckInBackOffStrategy.h> // nogncheck
#include <app/icd/server/ICDCheckInSender.h> // nogncheck
#include <app/icd/server/ICDMonitoringTable.h> // nogncheck
#endif // CHIP_CONFIG_ENABLE_ICD_CIP

namespace chip {
namespace Crypto {
Expand Down Expand Up @@ -115,7 +116,8 @@ class ICDManager : public ICDListener, public TestEventTriggerHandler
~ICDManager() = default;

void Init(PersistentStorageDelegate * storage, FabricTable * fabricTable, Crypto::SymmetricKeystore * symmetricKeyStore,
Messaging::ExchangeManager * exchangeManager, SubscriptionsInfoProvider * subInfoProvider);
Messaging::ExchangeManager * exchangeManager, SubscriptionsInfoProvider * subInfoProvider,
ICDCheckInBackOffStrategy * strategy);
void Shutdown();

/**
Expand Down Expand Up @@ -318,11 +320,12 @@ class ICDManager : public ICDListener, public TestEventTriggerHandler
bool mIsBootUpResumeSubscriptionExecuted = false;
#endif // !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS

PersistentStorageDelegate * mStorage = nullptr;
FabricTable * mFabricTable = nullptr;
Messaging::ExchangeManager * mExchangeManager = nullptr;
Crypto::SymmetricKeystore * mSymmetricKeystore = nullptr;
SubscriptionsInfoProvider * mSubInfoProvider = nullptr;
PersistentStorageDelegate * mStorage = nullptr;
FabricTable * mFabricTable = nullptr;
Messaging::ExchangeManager * mExchangeManager = nullptr;
Crypto::SymmetricKeystore * mSymmetricKeystore = nullptr;
SubscriptionsInfoProvider * mSubInfoProvider = nullptr;
ICDCheckInBackOffStrategy * mICDCheckInBackOffStrategy = nullptr;
ObjectPool<ICDCheckInSender, (CHIP_CONFIG_ICD_CLIENTS_SUPPORTED_PER_FABRIC * CHIP_CONFIG_MAX_FABRICS)> mICDSenderPool;
#endif // CHIP_CONFIG_ENABLE_ICD_CIP

Expand Down
2 changes: 2 additions & 0 deletions src/app/icd/server/tests/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ chip_test_suite("tests") {
output_name = "libICDServerTests"

test_sources = [
"TestDefaultICDCheckInBackOffStrategy.cpp",
"TestICDManager.cpp",
"TestICDMonitoringTable.cpp",
]

sources = [ "ICDConfigurationDataTestAccess.h" ]

public_deps = [
"${chip_root}/src/app/icd/server:default-check-in-back-off",
"${chip_root}/src/app/icd/server:manager",
"${chip_root}/src/app/icd/server:monitoring-table",
"${chip_root}/src/lib/core:string-builder-adapters",
Expand Down
57 changes: 57 additions & 0 deletions src/app/icd/server/tests/TestDefaultICDCheckInBackOffStrategy.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
*
* Copyright (c) 2024 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 <pw_unit_test/framework.h>

#include <app/icd/server/DefaultICDCheckInBackOffStrategy.h>
#include <app/icd/server/ICDMonitoringTable.h>
#include <crypto/DefaultSessionKeystore.h>
#include <lib/core/ClusterEnums.h>
#include <lib/core/DataModelTypes.h>
#include <lib/core/NodeId.h>

using namespace chip;
using namespace chip::app;
using namespace chip::app::Clusters::IcdManagement;

using TestSessionKeystoreImpl = Crypto::DefaultSessionKeystore;

namespace {

TEST(TestDefaultICDCheckInBackOffStrategy, TestShouldSendCheckInMessagePermanentClient)
{
TestSessionKeystoreImpl keystore;
ICDMonitoringEntry entry(&keystore);

entry.clientType = ClientTypeEnum::kPermanent;

DefaultICDCheckInBackOffStrategy strategy;
EXPECT_TRUE(strategy.ShouldSendCheckInMessage(entry));
}

TEST(TestDefaultICDCheckInBackOffStrategy, TestShouldSendCheckInMessageEphemeralClient)
{
TestSessionKeystoreImpl keystore;
ICDMonitoringEntry entry(&keystore);

entry.clientType = ClientTypeEnum::kEphemeral;

DefaultICDCheckInBackOffStrategy strategy;
EXPECT_FALSE(strategy.ShouldSendCheckInMessage(entry));
}

} // namespace
8 changes: 5 additions & 3 deletions src/app/icd/server/tests/TestICDManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include <app/SubscriptionsInfoProvider.h>
#include <app/TestEventTriggerDelegate.h>
#include <app/icd/server/DefaultICDCheckInBackOffStrategy.h>
#include <app/icd/server/ICDConfigurationData.h>
#include <app/icd/server/ICDManager.h>
#include <app/icd/server/ICDMonitoringTable.h>
Expand Down Expand Up @@ -197,7 +198,7 @@ class TestICDManager : public Test::LoopbackMessagingContext

mICDStateObserver.ResetAll();
mICDManager.RegisterObserver(&mICDStateObserver);
mICDManager.Init(&testStorage, &GetFabricTable(), &mKeystore, &GetExchangeManager(), &mSubInfoProvider);
mICDManager.Init(&testStorage, &GetFabricTable(), &mKeystore, &GetExchangeManager(), &mSubInfoProvider, &mStrategy);
}

// Performs teardown for each individual test in the test suite
Expand All @@ -212,6 +213,7 @@ class TestICDManager : public Test::LoopbackMessagingContext
TestSubscriptionsInfoProvider mSubInfoProvider;
TestPersistentStorageDelegate testStorage;
TestICDStateObserver mICDStateObserver;
DefaultICDCheckInBackOffStrategy mStrategy;
};

TEST_F(TestICDManager, TestICDModeDurations)
Expand Down Expand Up @@ -568,7 +570,7 @@ TEST_F(TestICDManager, TestICDCounter)

// Shut down and reinit ICDManager to increment counter
mICDManager.Shutdown();
mICDManager.Init(&(testStorage), &GetFabricTable(), &(mKeystore), &GetExchangeManager(), &(mSubInfoProvider));
mICDManager.Init(&(testStorage), &GetFabricTable(), &(mKeystore), &GetExchangeManager(), &(mSubInfoProvider), &mStrategy);
mICDManager.RegisterObserver(&(mICDStateObserver));

EXPECT_EQ(counter + ICDConfigurationData::kICDCounterPersistenceIncrement,
Expand Down Expand Up @@ -973,7 +975,7 @@ TEST_F(TestICDManager, TestICDStateObserverOnICDModeChangeOnInit)
// Shut down and reinit ICDManager - We should go to LIT mode since we have a registration
mICDManager.Shutdown();
mICDManager.RegisterObserver(&(mICDStateObserver));
mICDManager.Init(&testStorage, &GetFabricTable(), &mKeystore, &GetExchangeManager(), &mSubInfoProvider);
mICDManager.Init(&testStorage, &GetFabricTable(), &mKeystore, &GetExchangeManager(), &mSubInfoProvider, &mStrategy);

// We have a registration, transition to LIT mode
EXPECT_TRUE(mICDStateObserver.mOnICDModeChangeCalled);
Expand Down
Loading

0 comments on commit 1459034

Please sign in to comment.