Skip to content

Commit 1070836

Browse files
jtung-applepull[bot]
authored andcommitted
[ICD] Subscription resumption after timeout (#27956)
* 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
1 parent 6dd694b commit 1070836

File tree

7 files changed

+190
-2
lines changed

7 files changed

+190
-2
lines changed

src/app/BUILD.gn

+1
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ buildconfig_header("app_buildconfig") {
5555
"CHIP_CONFIG_ENABLE_SESSION_RESUMPTION=${chip_enable_session_resumption}",
5656
"CHIP_CONFIG_ACCESS_CONTROL_POLICY_LOGGING_VERBOSITY=${chip_access_control_policy_logging_verbosity}",
5757
"CHIP_CONFIG_PERSIST_SUBSCRIPTIONS=${chip_persist_subscriptions}",
58+
"CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION=${chip_subscription_timeout_resumption}",
5859
"CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE=${enable_eventlist_attribute}",
5960
"CHIP_CONFIG_ENABLE_ICD_SERVER=${chip_enable_icd_server}",
6061
]

src/app/InteractionModelEngine.cpp

+79
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include <app/util/endpoint-config-api.h>
3535
#include <lib/core/TLVUtilities.h>
3636
#include <lib/support/CodeUtils.h>
37+
#include <lib/support/FibonacciUtils.h>
3738

3839
namespace chip {
3940
namespace app {
@@ -324,6 +325,17 @@ void InteractionModelEngine::OnDone(ReadHandler & apReadObj)
324325
mReportingEngine.ResetReadHandlerTracker(&apReadObj);
325326

326327
mReadHandlers.ReleaseObject(&apReadObj);
328+
329+
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
330+
if (!mSubscriptionResumptionScheduled && HasSubscriptionsToResume())
331+
{
332+
mSubscriptionResumptionScheduled = true;
333+
auto timeTillNextResubscriptionSecs = ComputeTimeSecondsTillNextSubscriptionResumption();
334+
mpExchangeMgr->GetSessionManager()->SystemLayer()->StartTimer(System::Clock::Seconds32(timeTillNextResubscriptionSecs),
335+
ResumeSubscriptionsTimerCallback, this);
336+
mNumSubscriptionResumptionRetries++;
337+
}
338+
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
327339
}
328340

329341
Status InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext,
@@ -1752,6 +1764,9 @@ CHIP_ERROR InteractionModelEngine::ResumeSubscriptions()
17521764
{
17531765
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
17541766
ReturnErrorCodeIf(!mpSubscriptionResumptionStorage, CHIP_NO_ERROR);
1767+
#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
1768+
ReturnErrorCodeIf(mSubscriptionResumptionScheduled, CHIP_NO_ERROR);
1769+
#endif
17551770

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

17771792
if (subscriptionsToResume)
17781793
{
1794+
#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
1795+
mSubscriptionResumptionScheduled = true;
1796+
#endif
17791797
ChipLogProgress(InteractionModel, "Resuming %d subscriptions in %u seconds", subscriptionsToResume, minInterval);
17801798
ReturnErrorOnFailure(mpExchangeMgr->GetSessionManager()->SystemLayer()->StartTimer(System::Clock::Seconds16(minInterval),
17811799
ResumeSubscriptionsTimerCallback, this));
@@ -1794,6 +1812,10 @@ void InteractionModelEngine::ResumeSubscriptionsTimerCallback(System::Layer * ap
17941812
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
17951813
VerifyOrReturn(apAppState != nullptr);
17961814
InteractionModelEngine * imEngine = static_cast<InteractionModelEngine *>(apAppState);
1815+
#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
1816+
imEngine->mSubscriptionResumptionScheduled = false;
1817+
bool resumedSubscriptions = false;
1818+
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
17971819
SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
17981820
auto * iterator = imEngine->mpSubscriptionResumptionStorage->IterateSubscriptions();
17991821
while (iterator->Next(subscriptionInfo))
@@ -1833,10 +1855,67 @@ void InteractionModelEngine::ResumeSubscriptionsTimerCallback(System::Layer * ap
18331855

18341856
ChipLogProgress(InteractionModel, "Resuming subscriptionId %" PRIu32, subscriptionInfo.mSubscriptionId);
18351857
handler->ResumeSubscription(*imEngine->mpCASESessionMgr, subscriptionInfo);
1858+
#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
1859+
resumedSubscriptions = true;
1860+
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
18361861
}
18371862
iterator->Release();
1863+
1864+
#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
1865+
// If no persisted subscriptions needed resumption then all resumption retries are done
1866+
if (!resumedSubscriptions)
1867+
{
1868+
imEngine->mNumSubscriptionResumptionRetries = 0;
1869+
}
1870+
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
1871+
18381872
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
18391873
}
18401874

1875+
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
1876+
uint32_t InteractionModelEngine::ComputeTimeSecondsTillNextSubscriptionResumption()
1877+
{
1878+
if (mNumSubscriptionResumptionRetries > CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX)
1879+
{
1880+
return CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS;
1881+
}
1882+
1883+
return CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS +
1884+
GetFibonacciForIndex(mNumSubscriptionResumptionRetries) *
1885+
CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS;
1886+
}
1887+
1888+
bool InteractionModelEngine::HasSubscriptionsToResume()
1889+
{
1890+
VerifyOrReturnValue(mpSubscriptionResumptionStorage != nullptr, false);
1891+
1892+
// Look through persisted subscriptions and see if any aren't already in mReadHandlers pool
1893+
SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
1894+
auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
1895+
bool foundSubscriptionToResume = false;
1896+
while (iterator->Next(subscriptionInfo))
1897+
{
1898+
if (Loop::Break == mReadHandlers.ForEachActiveObject([&](ReadHandler * handler) {
1899+
SubscriptionId subscriptionId;
1900+
handler->GetSubscriptionId(subscriptionId);
1901+
if (subscriptionId == subscriptionInfo.mSubscriptionId)
1902+
{
1903+
return Loop::Break;
1904+
}
1905+
return Loop::Continue;
1906+
}))
1907+
{
1908+
continue;
1909+
}
1910+
1911+
foundSubscriptionToResume = true;
1912+
break;
1913+
}
1914+
iterator->Release();
1915+
1916+
return foundSubscriptionToResume;
1917+
}
1918+
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
1919+
18411920
} // namespace app
18421921
} // namespace chip

src/app/InteractionModelEngine.h

+8
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
366366
private:
367367
friend class reporting::Engine;
368368
friend class TestCommandInteraction;
369+
friend class TestInteractionModelEngine;
369370
using Status = Protocols::InteractionModel::Status;
370371

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

617+
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
618+
bool HasSubscriptionsToResume();
619+
uint32_t ComputeTimeSecondsTillNextSubscriptionResumption();
620+
uint32_t mNumSubscriptionResumptionRetries = 0;
621+
bool mSubscriptionResumptionScheduled = false;
622+
#endif
623+
616624
FabricTable * mpFabricTable;
617625

618626
CASESessionManager * mpCASESessionMgr = nullptr;

src/app/ReadHandler.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,6 @@ void ReadHandler::OnResponseTimeout(Messaging::ExchangeContext * apExchangeConte
425425
ChipLogError(DataManagement, "Time out! failed to receive status response from Exchange: " ChipLogFormatExchange,
426426
ChipLogValueExchange(apExchangeContext));
427427
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
428-
// TODO: Have a retry mechanism tied to wake interval for IC devices
429428
Close(CloseOptions::kKeepPersistedSubscription);
430429
#else
431430
Close();

src/app/tests/TestInteractionModelEngine.cpp

+37
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ class TestInteractionModelEngine
4848
public:
4949
static void TestAttributePathParamsPushRelease(nlTestSuite * apSuite, void * apContext);
5050
static void TestRemoveDuplicateConcreteAttribute(nlTestSuite * apSuite, void * apContext);
51+
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
52+
static void TestSubscriptionResumptionTimer(nlTestSuite * apSuite, void * apContext);
53+
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
5154
static int GetAttributePathListLength(ObjectList<AttributePathParams> * apattributePathParamsList);
5255
};
5356

@@ -223,6 +226,37 @@ void TestInteractionModelEngine::TestRemoveDuplicateConcreteAttribute(nlTestSuit
223226
InteractionModelEngine::GetInstance()->ReleaseAttributePathList(attributePathParamsList);
224227
}
225228

229+
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
230+
void TestInteractionModelEngine::TestSubscriptionResumptionTimer(nlTestSuite * apSuite, void * apContext)
231+
{
232+
TestContext & ctx = *static_cast<TestContext *>(apContext);
233+
CHIP_ERROR err = CHIP_NO_ERROR;
234+
err = InteractionModelEngine::GetInstance()->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable());
235+
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
236+
237+
uint32_t timeTillNextResubscriptionMs;
238+
InteractionModelEngine::GetInstance()->mNumSubscriptionResumptionRetries = 0;
239+
timeTillNextResubscriptionMs = InteractionModelEngine::GetInstance()->ComputeTimeSecondsTillNextSubscriptionResumption();
240+
NL_TEST_ASSERT(apSuite, timeTillNextResubscriptionMs == CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS);
241+
242+
uint32_t lastTimeTillNextResubscriptionMs = timeTillNextResubscriptionMs;
243+
for (InteractionModelEngine::GetInstance()->mNumSubscriptionResumptionRetries = 1;
244+
InteractionModelEngine::GetInstance()->mNumSubscriptionResumptionRetries <=
245+
CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX;
246+
InteractionModelEngine::GetInstance()->mNumSubscriptionResumptionRetries++)
247+
{
248+
timeTillNextResubscriptionMs = InteractionModelEngine::GetInstance()->ComputeTimeSecondsTillNextSubscriptionResumption();
249+
NL_TEST_ASSERT(apSuite, timeTillNextResubscriptionMs >= lastTimeTillNextResubscriptionMs);
250+
NL_TEST_ASSERT(apSuite, timeTillNextResubscriptionMs < CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS);
251+
lastTimeTillNextResubscriptionMs = timeTillNextResubscriptionMs;
252+
}
253+
254+
InteractionModelEngine::GetInstance()->mNumSubscriptionResumptionRetries = 2000;
255+
timeTillNextResubscriptionMs = InteractionModelEngine::GetInstance()->ComputeTimeSecondsTillNextSubscriptionResumption();
256+
NL_TEST_ASSERT(apSuite, timeTillNextResubscriptionMs == CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS);
257+
}
258+
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
259+
226260
} // namespace app
227261
} // namespace chip
228262

@@ -233,6 +267,9 @@ const nlTest sTests[] =
233267
{
234268
NL_TEST_DEF("TestAttributePathParamsPushRelease", chip::app::TestInteractionModelEngine::TestAttributePathParamsPushRelease),
235269
NL_TEST_DEF("TestRemoveDuplicateConcreteAttribute", chip::app::TestInteractionModelEngine::TestRemoveDuplicateConcreteAttribute),
270+
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
271+
NL_TEST_DEF("TestSubscriptionResumptionTimer", chip::app::TestInteractionModelEngine::TestSubscriptionResumptionTimer),
272+
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
236273
NL_TEST_SENTINEL()
237274
};
238275
// clang-format on

src/lib/core/CHIPConfig.h

+60-1
Original file line numberDiff line numberDiff line change
@@ -902,6 +902,10 @@ extern const char CHIP_NON_PRODUCTION_MARKER[];
902902
#define CHIP_IM_MAX_NUM_TIMED_HANDLER 8
903903
#endif
904904

905+
/**
906+
* @}
907+
*/
908+
905909
/**
906910
* @def CONFIG_BUILD_FOR_HOST_UNIT_TEST
907911
*
@@ -1497,5 +1501,60 @@ extern const char CHIP_NON_PRODUCTION_MARKER[];
14971501
#endif
14981502

14991503
/**
1500-
* @}
1504+
* @name Configuation for resuming subscriptions that timed out
1505+
*
1506+
* @brief
1507+
* The following definitions sets the parameters for subscription resumption in the case of subscription timeout.
1508+
* * #CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX
1509+
* * #CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS
1510+
* * #CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS
1511+
* * #CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS
1512+
*
1513+
* @{
1514+
*/
1515+
1516+
/**
1517+
* @def CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX
1518+
*
1519+
* @brief
1520+
* If subscription timeout resumption is enabled, specify the max fibonacci step index.
1521+
*
1522+
* This index must satisfy below conditions (for readability "CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_" prefix is omitted):
1523+
* * MIN_RETRY_INTERVAL_SECS + Fibonacci(MAX_FIBONACCI_STEP_INDEX + 1) * WAIT_TIME_MULTIPLIER_SECS > MAX_RETRY_INTERVAL_SECS
1524+
* * MIN_RETRY_INTERVAL_SECS + Fibonacci(MAX_FIBONACCI_STEP_INDEX) * WAIT_TIME_MULTIPLIER_SECS < MAX_RETRY_INTERVAL_SECS
1525+
*
1526+
*/
1527+
#ifndef CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX
1528+
#define CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX 10
1529+
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX
1530+
1531+
/**
1532+
* @def CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS
1533+
*
1534+
* @brief The minimum interval before resuming a subsciption that timed out.
1535+
*/
1536+
#ifndef CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS
1537+
#define CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS 300
1538+
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MIN_RETRY_INTERVAL_SECS
1539+
1540+
/**
1541+
* @def CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS
1542+
*
1543+
* @brief The multiplier per step in the calculation of retry interval.
1544+
*/
1545+
#ifndef CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS
1546+
#define CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS 300
1547+
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_WAIT_TIME_MULTIPLIER_SECS
1548+
1549+
/**
1550+
* @def CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS
1551+
*
1552+
* @brief The maximum interval before resuming a subsciption that timed out.
1553+
*/
1554+
#ifndef CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS
1555+
#define CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS (3600 * 6)
1556+
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS
1557+
1558+
/**
1559+
* @}
15011560
*/

src/platform/device.gni

+5
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,11 @@ declare_args() {
111111
}
112112
}
113113

114+
declare_args() {
115+
# Enable subscription resumption after timeout - separate configuration for power use measurement
116+
chip_subscription_timeout_resumption = chip_persist_subscriptions
117+
}
118+
114119
if (chip_device_platform == "bl702" || chip_device_platform == "bl702l") {
115120
if (chip_enable_openthread) {
116121
chip_mdns = "platform"

0 commit comments

Comments
 (0)