Skip to content

Commit 96329ae

Browse files
[ICD] Add Check-In message at boot logic and persistent subscription checks (#32613)
* wip * Add unit tests to validate NodeID subscriptions * Add CATS unit test * Clean up conditionnal checks * finish PR clean up * Fix conditionnals * update comment * fix typo * Fix build issue rename gn source-set to match header * fix unit test #ifdef to #if * Add project config to ESP all-clusters-app to enable the unit test defines * force define to 1 * add CHIPCore.h include * add include after ChipCore * restyle * fix include ordering * restyle * move include * add unit test define to qemu build * fix esp all-clusters-minimal build proble * Apply suggestions from code review Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Fix comment * Finish refactor of the Sub logic when interacting with the ICDManager * Rename verifier function * Clean up comments & code * Apply suggestions from code review Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Fix comment application * Add / update comments * update comment with more details --------- Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
1 parent 2ca30ac commit 96329ae

21 files changed

+981
-63
lines changed

examples/all-clusters-app/esp32/main/Kconfig.projbuild

+4
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ menu "Demo"
5959
depends on IDF_TARGET_ESP32H2
6060
endchoice
6161

62+
config CHIP_PROJECT_CONFIG
63+
string "CHIP Project Configuration file"
64+
default "main/include/CHIPProjectConfig.h"
65+
6266
choice
6367
prompt "Rendezvous Mode"
6468
default RENDEZVOUS_MODE_BLE if BT_ENABLED
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
*
3+
* Copyright (c) 2023 Project CHIP Authors
4+
* All rights reserved.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
/**
20+
* @file
21+
* Example project configuration file for CHIP.
22+
*
23+
* This is a place to put application or project-specific overrides
24+
* to the default configuration values for general CHIP features.
25+
*
26+
*/
27+
28+
#pragma once
29+
30+
/**
31+
* @def CONFIG_BUILD_FOR_HOST_UNIT_TEST
32+
*
33+
* @brief Defines whether we're currently building for unit testing, which enables a set of features
34+
* that are only utilized in those tests. This flag should not be enabled on devices. If you have a test
35+
* that uses this flag, either appropriately conditionalize the entire test on this flag, or to exclude
36+
* the compliation of that test source file entirely.
37+
*/
38+
#define CONFIG_BUILD_FOR_HOST_UNIT_TEST 1

examples/all-clusters-minimal-app/esp32/main/Kconfig.projbuild

+4
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ menu "Demo"
4848
depends on IDF_TARGET_ESP32C2
4949
endchoice
5050

51+
config CHIP_PROJECT_CONFIG
52+
string "CHIP Project Configuration file"
53+
default "main/include/CHIPProjectConfig.h"
54+
5155
choice
5256
prompt "Rendezvous Mode"
5357
default RENDEZVOUS_MODE_BLE if BT_ENABLED
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
*
3+
* Copyright (c) 2023 Project CHIP Authors
4+
* All rights reserved.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
/**
20+
* @file
21+
* Example project configuration file for CHIP.
22+
*
23+
* This is a place to put application or project-specific overrides
24+
* to the default configuration values for general CHIP features.
25+
*
26+
*/
27+
28+
#pragma once
29+
30+
/**
31+
* @def CONFIG_BUILD_FOR_HOST_UNIT_TEST
32+
*
33+
* @brief Defines whether we're currently building for unit testing, which enables a set of features
34+
* that are only utilized in those tests. This flag should not be enabled on devices. If you have a test
35+
* that uses this flag, either appropriately conditionalize the entire test on this flag, or to exclude
36+
* the compliation of that test source file entirely.
37+
*/
38+
#define CONFIG_BUILD_FOR_HOST_UNIT_TEST 1

src/app/BUILD.gn

+3-2
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ source_set("global-attributes") {
122122
]
123123
}
124124

125-
source_set("subscription-manager") {
125+
source_set("subscription-info-provider") {
126126
sources = [ "SubscriptionsInfoProvider.h" ]
127127

128128
public_deps = [ "${chip_root}/src/lib/core" ]
@@ -207,9 +207,10 @@ static_library("interaction-model") {
207207
":app_config",
208208
":constants",
209209
":paths",
210-
":subscription-manager",
210+
":subscription-info-provider",
211211
"${chip_root}/src/app/MessageDef",
212212
"${chip_root}/src/app/icd/server:icd-server-config",
213+
"${chip_root}/src/app/icd/server:manager",
213214
"${chip_root}/src/app/icd/server:observer",
214215
"${chip_root}/src/app/util:af-types",
215216
"${chip_root}/src/app/util:callbacks",

src/app/InteractionModelEngine.cpp

+54-23
Original file line numberDiff line numberDiff line change
@@ -336,30 +336,20 @@ bool InteractionModelEngine::SubjectHasActiveSubscription(FabricIndex aFabricInd
336336
{
337337
bool isActive = false;
338338
mReadHandlers.ForEachActiveObject([aFabricIndex, subjectID, &isActive](ReadHandler * handler) {
339-
if (!handler->IsType(ReadHandler::InteractionType::Subscribe))
340-
{
341-
return Loop::Continue;
342-
}
339+
VerifyOrReturnValue(handler->IsType(ReadHandler::InteractionType::Subscribe), Loop::Continue);
343340

344341
Access::SubjectDescriptor subject = handler->GetSubjectDescriptor();
345-
if (subject.fabricIndex != aFabricIndex)
346-
{
347-
return Loop::Continue;
348-
}
342+
VerifyOrReturnValue(subject.fabricIndex == aFabricIndex, Loop::Continue);
349343

350344
if (subject.authMode == Access::AuthMode::kCase)
351345
{
352346
if (subject.cats.CheckSubjectAgainstCATs(subjectID) || subjectID == subject.subject)
353347
{
354348
isActive = handler->IsActiveSubscription();
355349

356-
// Exit loop only if isActive is set to true
357-
// Otherwise keep looking for another subscription that could
358-
// match the subject
359-
if (isActive)
360-
{
361-
return Loop::Break;
362-
}
350+
// Exit loop only if isActive is set to true.
351+
// Otherwise keep looking for another subscription that could match the subject.
352+
VerifyOrReturnValue(!isActive, Loop::Break);
363353
}
364354
}
365355

@@ -371,8 +361,30 @@ bool InteractionModelEngine::SubjectHasActiveSubscription(FabricIndex aFabricInd
371361

372362
bool InteractionModelEngine::SubjectHasPersistedSubscription(FabricIndex aFabricIndex, NodeId subjectID)
373363
{
374-
// TODO(#30281) : Implement persisted sub check and verify how persistent subscriptions affects this at ICDManager::Init
375-
return false;
364+
bool persistedSubMatches = false;
365+
366+
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
367+
auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
368+
// Verify that we were able to allocate an iterator. If not, we are probably currently trying to resubscribe to our persisted
369+
// subscriptions. As such, we assume we have a persisted subscription and return true.
370+
// If we don't have a persisted subscription for the given fabric index and subjectID, we will send a Check-In message next time
371+
// we transition to ActiveMode.
372+
VerifyOrReturnValue(iterator, true);
373+
374+
SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
375+
while (iterator->Next(subscriptionInfo))
376+
{
377+
// TODO(#31873): Persistent subscription only stores the NodeID for now. We cannot check if the CAT matches
378+
if (subscriptionInfo.mFabricIndex == aFabricIndex && subscriptionInfo.mNodeId == subjectID)
379+
{
380+
persistedSubMatches = true;
381+
break;
382+
}
383+
}
384+
iterator->Release();
385+
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
386+
387+
return persistedSubMatches;
376388
}
377389

378390
void InteractionModelEngine::OnDone(CommandResponseSender & apResponderObj)
@@ -1917,22 +1929,22 @@ CHIP_ERROR InteractionModelEngine::ResumeSubscriptions()
19171929
// future improvements: https://github.com/project-chip/connectedhomeip/issues/25439
19181930

19191931
SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
1920-
auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
1921-
int subscriptionsToResume = 0;
1922-
uint16_t minInterval = 0;
1932+
auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
1933+
mNumOfSubscriptionsToResume = 0;
1934+
uint16_t minInterval = 0;
19231935
while (iterator->Next(subscriptionInfo))
19241936
{
1925-
subscriptionsToResume++;
1937+
mNumOfSubscriptionsToResume++;
19261938
minInterval = std::max(minInterval, subscriptionInfo.mMinInterval);
19271939
}
19281940
iterator->Release();
19291941

1930-
if (subscriptionsToResume)
1942+
if (mNumOfSubscriptionsToResume)
19311943
{
19321944
#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
19331945
mSubscriptionResumptionScheduled = true;
19341946
#endif
1935-
ChipLogProgress(InteractionModel, "Resuming %d subscriptions in %u seconds", subscriptionsToResume, minInterval);
1947+
ChipLogProgress(InteractionModel, "Resuming %d subscriptions in %u seconds", mNumOfSubscriptionsToResume, minInterval);
19361948
ReturnErrorOnFailure(mpExchangeMgr->GetSessionManager()->SystemLayer()->StartTimer(System::Clock::Seconds16(minInterval),
19371949
ResumeSubscriptionsTimerCallback, this));
19381950
}
@@ -2054,5 +2066,24 @@ bool InteractionModelEngine::HasSubscriptionsToResume()
20542066
}
20552067
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
20562068

2069+
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
2070+
void InteractionModelEngine::DecrementNumSubscriptionsToResume()
2071+
{
2072+
VerifyOrReturn(mNumOfSubscriptionsToResume > 0);
2073+
#if CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
2074+
VerifyOrDie(mICDManager);
2075+
#endif // CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
2076+
2077+
mNumOfSubscriptionsToResume--;
2078+
2079+
#if CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
2080+
if (!mNumOfSubscriptionsToResume)
2081+
{
2082+
mICDManager->SetBootUpResumeSubscriptionExecuted();
2083+
}
2084+
#endif // CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
2085+
}
2086+
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
2087+
20572088
} // namespace app
20582089
} // namespace chip

src/app/InteractionModelEngine.h

+36-2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525

2626
#pragma once
2727

28+
// TODO(#32628): Remove the CHIPCore.h header when the esp32 build is correctly fixed
29+
#include <lib/core/CHIPCore.h>
30+
2831
#include <access/AccessControl.h>
2932
#include <app/AppConfig.h>
3033
#include <app/AttributePathParams.h>
@@ -47,6 +50,7 @@
4750
#include <app/TimedHandler.h>
4851
#include <app/WriteClient.h>
4952
#include <app/WriteHandler.h>
53+
#include <app/icd/server/ICDServerConfig.h>
5054
#include <app/reporting/Engine.h>
5155
#include <app/reporting/ReportScheduler.h>
5256
#include <app/util/attribute-metadata.h>
@@ -66,6 +70,10 @@
6670

6771
#include <app/CASESessionManager.h>
6872

73+
#if CHIP_CONFIG_ENABLE_ICD_SERVER
74+
#include <app/icd/server/ICDManager.h> // nogncheck
75+
#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
76+
6977
namespace chip {
7078
namespace app {
7179

@@ -127,6 +135,10 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
127135

128136
void Shutdown();
129137

138+
#if CHIP_CONFIG_ENABLE_ICD_SERVER
139+
void SetICDManager(ICDManager * manager) { mICDManager = manager; };
140+
#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
141+
130142
Messaging::ExchangeManager * GetExchangeManager(void) const { return mpExchangeMgr; }
131143

132144
/**
@@ -317,6 +329,15 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
317329

318330
bool SubjectHasPersistedSubscription(FabricIndex aFabricIndex, NodeId subjectID) override;
319331

332+
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
333+
/**
334+
* @brief Function decrements the number of subscriptions to resume counter - mNumOfSubscriptionsToResume.
335+
* This should be called after we have completed a re-subscribe attempt on a persisted subscription wether the attempt
336+
* was succesful or not.
337+
*/
338+
void DecrementNumSubscriptionsToResume();
339+
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
340+
320341
#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
321342
//
322343
// Get direct access to the underlying read handler pool
@@ -602,6 +623,10 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
602623

603624
CommandHandlerInterface * mCommandHandlerList = nullptr;
604625

626+
#if CHIP_CONFIG_ENABLE_ICD_SERVER
627+
ICDManager * mICDManager = nullptr;
628+
#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
629+
605630
ObjectPool<CommandResponseSender, CHIP_IM_MAX_NUM_COMMAND_HANDLER> mCommandResponderObjs;
606631
ObjectPool<TimedHandler, CHIP_IM_MAX_NUM_TIMED_HANDLER> mTimedHandlers;
607632
WriteHandler mWriteHandlers[CHIP_IM_MAX_NUM_WRITE_HANDLER];
@@ -660,12 +685,21 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
660685
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
661686
#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST
662687

663-
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
688+
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
689+
/**
690+
* mNumOfSubscriptionsToResume tracks the number of subscriptions that the device will try to resume at its next resumption
691+
* attempt. At boot up, the attempt will be at the highest min interval of all the subscriptions to resume.
692+
* When the subscription timeout resumption feature is present, after the boot up attempt, the next attempt will be determined
693+
* by ComputeTimeSecondsTillNextSubscriptionResumption.
694+
*/
695+
int8_t mNumOfSubscriptionsToResume = 0;
696+
#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
664697
bool HasSubscriptionsToResume();
665698
uint32_t ComputeTimeSecondsTillNextSubscriptionResumption();
666699
uint32_t mNumSubscriptionResumptionRetries = 0;
667700
bool mSubscriptionResumptionScheduled = false;
668-
#endif
701+
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
702+
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
669703

670704
FabricTable * mpFabricTable;
671705

src/app/ReadHandler.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -821,6 +821,7 @@ void ReadHandler::PersistSubscription()
821821
auto * subscriptionResumptionStorage = mManagementCallback.GetInteractionModelEngine()->GetSubscriptionResumptionStorage();
822822
VerifyOrReturn(subscriptionResumptionStorage != nullptr);
823823

824+
// TODO(#31873): We need to store the CAT information to enable better interactions with ICDs
824825
SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo = { .mNodeId = GetInitiatorNodeId(),
825826
.mFabricIndex = GetAccessingFabricIndex(),
826827
.mSubscriptionId = mSubscriptionId,

src/app/ReadHandler.h

+2
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ class TestReportScheduler;
7070
} // namespace reporting
7171

7272
class InteractionModelEngine;
73+
class TestInteractionModelEngine;
7374

7475
/**
7576
* @class ReadHandler
@@ -433,6 +434,7 @@ class ReadHandler : public Messaging::ExchangeDelegate
433434
//
434435
friend class chip::app::reporting::Engine;
435436
friend class chip::app::InteractionModelEngine;
437+
friend class TestInteractionModelEngine;
436438

437439
// The report scheduler needs to be able to access StateFlag private functions ShouldStartReporting(), CanStartReporting(),
438440
// ForceDirtyState() and IsDirty() to know when to schedule a run so it is declared as a friend class.

0 commit comments

Comments
 (0)