Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ICD] Add Check-In message at boot logic and persistent subscription checks #32613

Merged
merged 29 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6612770
wip
mkardous-silabs Mar 4, 2024
271a628
Add unit tests to validate NodeID subscriptions
mkardous-silabs Mar 15, 2024
bfef690
Add CATS unit test
mkardous-silabs Mar 15, 2024
7641227
Clean up conditionnal checks
mkardous-silabs Mar 15, 2024
742ec95
finish PR clean up
mkardous-silabs Mar 17, 2024
bb0cfd8
Fix conditionnals
mkardous-silabs Mar 18, 2024
60f996c
update comment
mkardous-silabs Mar 18, 2024
a088243
fix typo
mkardous-silabs Mar 18, 2024
ec25e96
Fix build issue
mkardous-silabs Mar 18, 2024
bc676fb
fix unit test #ifdef to #if
mkardous-silabs Mar 19, 2024
b12eb7a
Add project config to ESP all-clusters-app to enable the unit test de…
mkardous-silabs Mar 19, 2024
be088b4
force define to 1
mkardous-silabs Mar 19, 2024
77abb2c
add CHIPCore.h include
mkardous-silabs Mar 19, 2024
57c0522
add include after ChipCore
mkardous-silabs Mar 19, 2024
e31e5c4
restyle
mkardous-silabs Mar 19, 2024
a90901b
fix include ordering
mkardous-silabs Mar 19, 2024
4d1081c
restyle
mkardous-silabs Mar 19, 2024
2f42fd0
move include
mkardous-silabs Mar 19, 2024
d86b554
add unit test define to qemu build
mkardous-silabs Mar 19, 2024
cd8efa6
fix esp all-clusters-minimal build proble
mkardous-silabs Mar 19, 2024
58ae2e1
Apply suggestions from code review
mkardous-silabs Mar 20, 2024
07b7a42
Fix comment
mkardous-silabs Mar 24, 2024
5fe51f3
Finish refactor of the Sub logic when interacting with the ICDManager
mkardous-silabs Mar 26, 2024
7105c88
Rename verifier function
mkardous-silabs Mar 26, 2024
50b128b
Clean up comments & code
mkardous-silabs Mar 26, 2024
d30312b
Apply suggestions from code review
mkardous-silabs Apr 2, 2024
c45315b
Fix comment application
mkardous-silabs Apr 2, 2024
59a9862
Add / update comments
mkardous-silabs Apr 2, 2024
39b23f4
update comment with more details
mkardous-silabs Apr 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions examples/all-clusters-app/esp32/main/Kconfig.projbuild
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ menu "Demo"
depends on IDF_TARGET_ESP32H2
endchoice

config CHIP_PROJECT_CONFIG
string "CHIP Project Configuration file"
default "main/include/CHIPProjectConfig.h"

choice
prompt "Rendezvous Mode"
default RENDEZVOUS_MODE_BLE if BT_ENABLED
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
*
* Copyright (c) 2023 Project CHIP Authors
* All rights reserved.
*
* 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.
*/

/**
* @file
* Example project configuration file for CHIP.
*
* This is a place to put application or project-specific overrides
* to the default configuration values for general CHIP features.
*
*/

#pragma once

/**
* @def CONFIG_BUILD_FOR_HOST_UNIT_TEST
*
* @brief Defines whether we're currently building for unit testing, which enables a set of features
* that are only utilized in those tests. This flag should not be enabled on devices. If you have a test
* that uses this flag, either appropriately conditionalize the entire test on this flag, or to exclude
* the compliation of that test source file entirely.
*/
#define CONFIG_BUILD_FOR_HOST_UNIT_TEST 1
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ menu "Demo"
depends on IDF_TARGET_ESP32C2
endchoice

config CHIP_PROJECT_CONFIG
string "CHIP Project Configuration file"
default "main/include/CHIPProjectConfig.h"

choice
prompt "Rendezvous Mode"
default RENDEZVOUS_MODE_BLE if BT_ENABLED
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
*
* Copyright (c) 2023 Project CHIP Authors
* All rights reserved.
*
* 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.
*/

/**
* @file
* Example project configuration file for CHIP.
*
* This is a place to put application or project-specific overrides
* to the default configuration values for general CHIP features.
*
*/

#pragma once

/**
* @def CONFIG_BUILD_FOR_HOST_UNIT_TEST
*
* @brief Defines whether we're currently building for unit testing, which enables a set of features
* that are only utilized in those tests. This flag should not be enabled on devices. If you have a test
* that uses this flag, either appropriately conditionalize the entire test on this flag, or to exclude
* the compliation of that test source file entirely.
*/
#define CONFIG_BUILD_FOR_HOST_UNIT_TEST 1
5 changes: 3 additions & 2 deletions src/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ source_set("global-attributes") {
]
}

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

public_deps = [ "${chip_root}/src/lib/core" ]
Expand Down Expand Up @@ -207,9 +207,10 @@ static_library("interaction-model") {
":app_config",
":constants",
":paths",
":subscription-manager",
":subscription-info-provider",
"${chip_root}/src/app/MessageDef",
"${chip_root}/src/app/icd/server:icd-server-config",
"${chip_root}/src/app/icd/server:manager",
"${chip_root}/src/app/icd/server:observer",
"${chip_root}/src/app/util:af-types",
"${chip_root}/src/app/util:callbacks",
Expand Down
77 changes: 54 additions & 23 deletions src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,30 +336,20 @@ bool InteractionModelEngine::SubjectHasActiveSubscription(FabricIndex aFabricInd
{
bool isActive = false;
mReadHandlers.ForEachActiveObject([aFabricIndex, subjectID, &isActive](ReadHandler * handler) {
if (!handler->IsType(ReadHandler::InteractionType::Subscribe))
{
return Loop::Continue;
}
VerifyOrReturnValue(handler->IsType(ReadHandler::InteractionType::Subscribe), Loop::Continue);

Access::SubjectDescriptor subject = handler->GetSubjectDescriptor();
if (subject.fabricIndex != aFabricIndex)
{
return Loop::Continue;
}
VerifyOrReturnValue(subject.fabricIndex == aFabricIndex, Loop::Continue);

if (subject.authMode == Access::AuthMode::kCase)
{
if (subject.cats.CheckSubjectAgainstCATs(subjectID) || subjectID == subject.subject)
{
isActive = handler->IsActiveSubscription();

// Exit loop only if isActive is set to true
// Otherwise keep looking for another subscription that could
// match the subject
if (isActive)
{
return Loop::Break;
}
// Exit loop only if isActive is set to true.
// Otherwise keep looking for another subscription that could match the subject.
VerifyOrReturnValue(!isActive, Loop::Break);
}
}

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

bool InteractionModelEngine::SubjectHasPersistedSubscription(FabricIndex aFabricIndex, NodeId subjectID)
{
// TODO(#30281) : Implement persisted sub check and verify how persistent subscriptions affects this at ICDManager::Init
return false;
bool persistedSubMatches = false;

#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
mkardous-silabs marked this conversation as resolved.
Show resolved Hide resolved
// Verify that we were able to allocate an iterator. If not, we are probably currently trying to resubscribe to our persisted
// subscriptions. As such, we assume we have a persisted subscription and return true.
// If we don't have a persisted subscription for the given fabric index and subjectID, we will send a Check-In message next time
// we transition to ActiveMode.
VerifyOrReturnValue(iterator, true);

SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
while (iterator->Next(subscriptionInfo))
{
// TODO(#31873): Persistent subscription only stores the NodeID for now. We cannot check if the CAT matches
if (subscriptionInfo.mFabricIndex == aFabricIndex && subscriptionInfo.mNodeId == subjectID)
{
persistedSubMatches = true;
break;
}
}
iterator->Release();
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS

return persistedSubMatches;
}

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

SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
int subscriptionsToResume = 0;
uint16_t minInterval = 0;
auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
mNumOfSubscriptionsToResume = 0;
uint16_t minInterval = 0;
while (iterator->Next(subscriptionInfo))
{
subscriptionsToResume++;
mNumOfSubscriptionsToResume++;
minInterval = std::max(minInterval, subscriptionInfo.mMinInterval);
}
iterator->Release();

if (subscriptionsToResume)
if (mNumOfSubscriptionsToResume)
{
#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
mSubscriptionResumptionScheduled = true;
#endif
ChipLogProgress(InteractionModel, "Resuming %d subscriptions in %u seconds", subscriptionsToResume, minInterval);
ChipLogProgress(InteractionModel, "Resuming %d subscriptions in %u seconds", mNumOfSubscriptionsToResume, minInterval);
ReturnErrorOnFailure(mpExchangeMgr->GetSessionManager()->SystemLayer()->StartTimer(System::Clock::Seconds16(minInterval),
ResumeSubscriptionsTimerCallback, this));
}
Expand Down Expand Up @@ -2054,5 +2066,24 @@ bool InteractionModelEngine::HasSubscriptionsToResume()
}
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION

#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
void InteractionModelEngine::DecrementNumSubscriptionsToResume()
{
VerifyOrReturn(mNumOfSubscriptionsToResume > 0);
#if CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
VerifyOrDie(mICDManager);
#endif // CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION

mNumOfSubscriptionsToResume--;

#if CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
if (!mNumOfSubscriptionsToResume)
{
mICDManager->SetBootUpResumeSubscriptionExecuted();
}
#endif // CHIP_CONFIG_ENABLE_ICD_CIP && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
}
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS

} // namespace app
} // namespace chip
38 changes: 36 additions & 2 deletions src/app/InteractionModelEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@

#pragma once

// TODO(#32628): Remove the CHIPCore.h header when the esp32 build is correctly fixed
#include <lib/core/CHIPCore.h>

#include <access/AccessControl.h>
#include <app/AppConfig.h>
#include <app/AttributePathParams.h>
Expand All @@ -47,6 +50,7 @@
#include <app/TimedHandler.h>
#include <app/WriteClient.h>
#include <app/WriteHandler.h>
#include <app/icd/server/ICDServerConfig.h>
#include <app/reporting/Engine.h>
#include <app/reporting/ReportScheduler.h>
#include <app/util/attribute-metadata.h>
Expand All @@ -66,6 +70,10 @@

#include <app/CASESessionManager.h>

#if CHIP_CONFIG_ENABLE_ICD_SERVER
#include <app/icd/server/ICDManager.h> // nogncheck
#endif // CHIP_CONFIG_ENABLE_ICD_SERVER

namespace chip {
namespace app {

Expand Down Expand Up @@ -127,6 +135,10 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,

void Shutdown();

#if CHIP_CONFIG_ENABLE_ICD_SERVER
void SetICDManager(ICDManager * manager) { mICDManager = manager; };
#endif // CHIP_CONFIG_ENABLE_ICD_SERVER

Messaging::ExchangeManager * GetExchangeManager(void) const { return mpExchangeMgr; }

/**
Expand Down Expand Up @@ -317,6 +329,15 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,

bool SubjectHasPersistedSubscription(FabricIndex aFabricIndex, NodeId subjectID) override;

#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
/**
* @brief Function decrements the number of subscriptions to resume counter - mNumOfSubscriptionsToResume.
* This should be called after we have completed a re-subscribe attempt on a persisted subscription wether the attempt
* was succesful or not.
*/
void DecrementNumSubscriptionsToResume();
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS

#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
//
// Get direct access to the underlying read handler pool
Expand Down Expand Up @@ -602,6 +623,10 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,

CommandHandlerInterface * mCommandHandlerList = nullptr;

#if CHIP_CONFIG_ENABLE_ICD_SERVER
ICDManager * mICDManager = nullptr;
#endif // CHIP_CONFIG_ENABLE_ICD_SERVER

ObjectPool<CommandResponseSender, CHIP_IM_MAX_NUM_COMMAND_HANDLER> mCommandResponderObjs;
ObjectPool<TimedHandler, CHIP_IM_MAX_NUM_TIMED_HANDLER> mTimedHandlers;
WriteHandler mWriteHandlers[CHIP_IM_MAX_NUM_WRITE_HANDLER];
Expand Down Expand Up @@ -660,12 +685,21 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST

#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
/**
* mNumOfSubscriptionsToResume tracks the number of subscriptions that the device will try to resume at its next resumption
* attempt. At boot up, the attempt will be at the highest min interval of all the subscriptions to resume.
* When the subscription timeout resumption feature is present, after the boot up attempt, the next attempt will be determined
* by ComputeTimeSecondsTillNextSubscriptionResumption.
*/
int8_t mNumOfSubscriptionsToResume = 0;
#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
bool HasSubscriptionsToResume();
uint32_t ComputeTimeSecondsTillNextSubscriptionResumption();
uint32_t mNumSubscriptionResumptionRetries = 0;
bool mSubscriptionResumptionScheduled = false;
#endif
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS

FabricTable * mpFabricTable;

Expand Down
1 change: 1 addition & 0 deletions src/app/ReadHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,7 @@ void ReadHandler::PersistSubscription()
auto * subscriptionResumptionStorage = mManagementCallback.GetInteractionModelEngine()->GetSubscriptionResumptionStorage();
VerifyOrReturn(subscriptionResumptionStorage != nullptr);

// TODO(#31873): We need to store the CAT information to enable better interactions with ICDs
SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo = { .mNodeId = GetInitiatorNodeId(),
.mFabricIndex = GetAccessingFabricIndex(),
.mSubscriptionId = mSubscriptionId,
Expand Down
2 changes: 2 additions & 0 deletions src/app/ReadHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class TestReportScheduler;
} // namespace reporting

class InteractionModelEngine;
class TestInteractionModelEngine;

/**
* @class ReadHandler
Expand Down Expand Up @@ -433,6 +434,7 @@ class ReadHandler : public Messaging::ExchangeDelegate
//
friend class chip::app::reporting::Engine;
friend class chip::app::InteractionModelEngine;
friend class TestInteractionModelEngine;

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