Skip to content

Commit

Permalink
[ICD] Subscription resumption after timeout (#27956)
Browse files Browse the repository at this point in the history
* Subscription timeout resumption

* Added unit test for ComputeTimeTillNextSubscriptionResumption() and addressed review comments

* Added ifdef conditional around new unit test and fixed up new code ifdefs

* Corrected chip_subscription_timeout_resumption = chip_persist_subscriptions by default

* Address review comments
  • Loading branch information
jtung-apple authored and pull[bot] committed Feb 20, 2024
1 parent 0c17ebf commit 1978784
Show file tree
Hide file tree
Showing 7 changed files with 190 additions and 2 deletions.
1 change: 1 addition & 0 deletions src/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ buildconfig_header("app_buildconfig") {
"CHIP_CONFIG_ENABLE_SESSION_RESUMPTION=${chip_enable_session_resumption}",
"CHIP_CONFIG_ACCESS_CONTROL_POLICY_LOGGING_VERBOSITY=${chip_access_control_policy_logging_verbosity}",
"CHIP_CONFIG_PERSIST_SUBSCRIPTIONS=${chip_persist_subscriptions}",
"CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION=${chip_subscription_timeout_resumption}",
"CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE=${enable_eventlist_attribute}",
"CHIP_CONFIG_ENABLE_ICD_SERVER=${chip_enable_icd_server}",
]
Expand Down
79 changes: 79 additions & 0 deletions src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <app/util/endpoint-config-api.h>
#include <lib/core/TLVUtilities.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/FibonacciUtils.h>

namespace chip {
namespace app {
Expand Down Expand Up @@ -324,6 +325,17 @@ void InteractionModelEngine::OnDone(ReadHandler & apReadObj)
mReportingEngine.ResetReadHandlerTracker(&apReadObj);

mReadHandlers.ReleaseObject(&apReadObj);

#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
if (!mSubscriptionResumptionScheduled && HasSubscriptionsToResume())
{
mSubscriptionResumptionScheduled = true;
auto timeTillNextResubscriptionSecs = ComputeTimeSecondsTillNextSubscriptionResumption();
mpExchangeMgr->GetSessionManager()->SystemLayer()->StartTimer(System::Clock::Seconds32(timeTillNextResubscriptionSecs),
ResumeSubscriptionsTimerCallback, this);
mNumSubscriptionResumptionRetries++;
}
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
}

Status InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext,
Expand Down Expand Up @@ -1752,6 +1764,9 @@ CHIP_ERROR InteractionModelEngine::ResumeSubscriptions()
{
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
ReturnErrorCodeIf(!mpSubscriptionResumptionStorage, CHIP_NO_ERROR);
#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
ReturnErrorCodeIf(mSubscriptionResumptionScheduled, CHIP_NO_ERROR);
#endif

// To avoid the case of a reboot loop causing rapid traffic generation / power consumption, subscription resumption should make
// use of the persisted min-interval values, and wait before resumption. Ideally, each persisted subscription should wait their
Expand All @@ -1776,6 +1791,9 @@ CHIP_ERROR InteractionModelEngine::ResumeSubscriptions()

if (subscriptionsToResume)
{
#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
mSubscriptionResumptionScheduled = true;
#endif
ChipLogProgress(InteractionModel, "Resuming %d subscriptions in %u seconds", subscriptionsToResume, minInterval);
ReturnErrorOnFailure(mpExchangeMgr->GetSessionManager()->SystemLayer()->StartTimer(System::Clock::Seconds16(minInterval),
ResumeSubscriptionsTimerCallback, this));
Expand All @@ -1794,6 +1812,10 @@ void InteractionModelEngine::ResumeSubscriptionsTimerCallback(System::Layer * ap
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
VerifyOrReturn(apAppState != nullptr);
InteractionModelEngine * imEngine = static_cast<InteractionModelEngine *>(apAppState);
#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
imEngine->mSubscriptionResumptionScheduled = false;
bool resumedSubscriptions = false;
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
auto * iterator = imEngine->mpSubscriptionResumptionStorage->IterateSubscriptions();
while (iterator->Next(subscriptionInfo))
Expand Down Expand Up @@ -1833,10 +1855,67 @@ void InteractionModelEngine::ResumeSubscriptionsTimerCallback(System::Layer * ap

ChipLogProgress(InteractionModel, "Resuming subscriptionId %" PRIu32, subscriptionInfo.mSubscriptionId);
handler->ResumeSubscription(*imEngine->mpCASESessionMgr, subscriptionInfo);
#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
resumedSubscriptions = true;
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
}
iterator->Release();

#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
// If no persisted subscriptions needed resumption then all resumption retries are done
if (!resumedSubscriptions)
{
imEngine->mNumSubscriptionResumptionRetries = 0;
}
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION

#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
}

#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
uint32_t InteractionModelEngine::ComputeTimeSecondsTillNextSubscriptionResumption()
{
if (mNumSubscriptionResumptionRetries > CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX)
{
return CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS;
}

return CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS +
GetFibonacciForIndex(mNumSubscriptionResumptionRetries) *
CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS;
}

bool InteractionModelEngine::HasSubscriptionsToResume()
{
VerifyOrReturnValue(mpSubscriptionResumptionStorage != nullptr, false);

// Look through persisted subscriptions and see if any aren't already in mReadHandlers pool
SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
bool foundSubscriptionToResume = false;
while (iterator->Next(subscriptionInfo))
{
if (Loop::Break == mReadHandlers.ForEachActiveObject([&](ReadHandler * handler) {
SubscriptionId subscriptionId;
handler->GetSubscriptionId(subscriptionId);
if (subscriptionId == subscriptionInfo.mSubscriptionId)
{
return Loop::Break;
}
return Loop::Continue;
}))
{
continue;
}

foundSubscriptionToResume = true;
break;
}
iterator->Release();

return foundSubscriptionToResume;
}
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION

} // namespace app
} // namespace chip
8 changes: 8 additions & 0 deletions src/app/InteractionModelEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
private:
friend class reporting::Engine;
friend class TestCommandInteraction;
friend class TestInteractionModelEngine;
using Status = Protocols::InteractionModel::Status;

void OnDone(CommandHandler & apCommandObj) override;
Expand Down Expand Up @@ -613,6 +614,13 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
bool mForceHandlerQuota = false;
#endif

#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
bool HasSubscriptionsToResume();
uint32_t ComputeTimeSecondsTillNextSubscriptionResumption();
uint32_t mNumSubscriptionResumptionRetries = 0;
bool mSubscriptionResumptionScheduled = false;
#endif

FabricTable * mpFabricTable;

CASESessionManager * mpCASESessionMgr = nullptr;
Expand Down
1 change: 0 additions & 1 deletion src/app/ReadHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,6 @@ void ReadHandler::OnResponseTimeout(Messaging::ExchangeContext * apExchangeConte
ChipLogError(DataManagement, "Time out! failed to receive status response from Exchange: " ChipLogFormatExchange,
ChipLogValueExchange(apExchangeContext));
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
// TODO: Have a retry mechanism tied to wake interval for IC devices
Close(CloseOptions::kKeepPersistedSubscription);
#else
Close();
Expand Down
37 changes: 37 additions & 0 deletions src/app/tests/TestInteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ class TestInteractionModelEngine
public:
static void TestAttributePathParamsPushRelease(nlTestSuite * apSuite, void * apContext);
static void TestRemoveDuplicateConcreteAttribute(nlTestSuite * apSuite, void * apContext);
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
static void TestSubscriptionResumptionTimer(nlTestSuite * apSuite, void * apContext);
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
static int GetAttributePathListLength(ObjectList<AttributePathParams> * apattributePathParamsList);
};

Expand Down Expand Up @@ -223,6 +226,37 @@ void TestInteractionModelEngine::TestRemoveDuplicateConcreteAttribute(nlTestSuit
InteractionModelEngine::GetInstance()->ReleaseAttributePathList(attributePathParamsList);
}

#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
void TestInteractionModelEngine::TestSubscriptionResumptionTimer(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
CHIP_ERROR err = CHIP_NO_ERROR;
err = InteractionModelEngine::GetInstance()->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable());
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

uint32_t timeTillNextResubscriptionMs;
InteractionModelEngine::GetInstance()->mNumSubscriptionResumptionRetries = 0;
timeTillNextResubscriptionMs = InteractionModelEngine::GetInstance()->ComputeTimeSecondsTillNextSubscriptionResumption();
NL_TEST_ASSERT(apSuite, timeTillNextResubscriptionMs == CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS);

uint32_t lastTimeTillNextResubscriptionMs = timeTillNextResubscriptionMs;
for (InteractionModelEngine::GetInstance()->mNumSubscriptionResumptionRetries = 1;
InteractionModelEngine::GetInstance()->mNumSubscriptionResumptionRetries <=
CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX;
InteractionModelEngine::GetInstance()->mNumSubscriptionResumptionRetries++)
{
timeTillNextResubscriptionMs = InteractionModelEngine::GetInstance()->ComputeTimeSecondsTillNextSubscriptionResumption();
NL_TEST_ASSERT(apSuite, timeTillNextResubscriptionMs >= lastTimeTillNextResubscriptionMs);
NL_TEST_ASSERT(apSuite, timeTillNextResubscriptionMs < CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS);
lastTimeTillNextResubscriptionMs = timeTillNextResubscriptionMs;
}

InteractionModelEngine::GetInstance()->mNumSubscriptionResumptionRetries = 2000;
timeTillNextResubscriptionMs = InteractionModelEngine::GetInstance()->ComputeTimeSecondsTillNextSubscriptionResumption();
NL_TEST_ASSERT(apSuite, timeTillNextResubscriptionMs == CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS);
}
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION

} // namespace app
} // namespace chip

Expand All @@ -233,6 +267,9 @@ const nlTest sTests[] =
{
NL_TEST_DEF("TestAttributePathParamsPushRelease", chip::app::TestInteractionModelEngine::TestAttributePathParamsPushRelease),
NL_TEST_DEF("TestRemoveDuplicateConcreteAttribute", chip::app::TestInteractionModelEngine::TestRemoveDuplicateConcreteAttribute),
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
NL_TEST_DEF("TestSubscriptionResumptionTimer", chip::app::TestInteractionModelEngine::TestSubscriptionResumptionTimer),
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
NL_TEST_SENTINEL()
};
// clang-format on
Expand Down
61 changes: 60 additions & 1 deletion src/lib/core/CHIPConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,10 @@ extern const char CHIP_NON_PRODUCTION_MARKER[];
#define CHIP_IM_MAX_NUM_TIMED_HANDLER 8
#endif

/**
* @}
*/

/**
* @def CONFIG_BUILD_FOR_HOST_UNIT_TEST
*
Expand Down Expand Up @@ -1497,5 +1501,60 @@ extern const char CHIP_NON_PRODUCTION_MARKER[];
#endif

/**
* @}
* @name Configuation for resuming subscriptions that timed out
*
* @brief
* The following definitions sets the parameters for subscription resumption in the case of subscription timeout.
* * #CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX
* * #CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS
* * #CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS
* * #CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS
*
* @{
*/

/**
* @def CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX
*
* @brief
* If subscription timeout resumption is enabled, specify the max fibonacci step index.
*
* This index must satisfy below conditions (for readability "CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_" prefix is omitted):
* * MIN_RETRY_INTERVAL_SECS + Fibonacci(MAX_FIBONACCI_STEP_INDEX + 1) * WAIT_TIME_MULTIPLIER_SECS > MAX_RETRY_INTERVAL_SECS
* * MIN_RETRY_INTERVAL_SECS + Fibonacci(MAX_FIBONACCI_STEP_INDEX) * WAIT_TIME_MULTIPLIER_SECS < MAX_RETRY_INTERVAL_SECS
*
*/
#ifndef CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX
#define CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX 10
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX

/**
* @def CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS
*
* @brief The minimum interval before resuming a subsciption that timed out.
*/
#ifndef CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS
#define CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS 300
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS

/**
* @def CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS
*
* @brief The multiplier per step in the calculation of retry interval.
*/
#ifndef CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS
#define CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS 300
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS

/**
* @def CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS
*
* @brief The maximum interval before resuming a subsciption that timed out.
*/
#ifndef CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS
#define CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS (3600 * 6)
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS

/**
* @}
*/
5 changes: 5 additions & 0 deletions src/platform/device.gni
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ declare_args() {
}
}

declare_args() {
# Enable subscription resumption after timeout - separate configuration for power use measurement
chip_subscription_timeout_resumption = chip_persist_subscriptions
}

if (chip_device_platform == "bl702" || chip_device_platform == "bl702l") {
if (chip_enable_openthread) {
chip_mdns = "platform"
Expand Down

0 comments on commit 1978784

Please sign in to comment.