Skip to content

Commit c2d2834

Browse files
yunhanw-googleyunhanw-google
committed
address comments
1 parent 6a97e2c commit c2d2834

7 files changed

+20
-23
lines changed

‎src/app/InteractionModelEngine.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -1069,14 +1069,16 @@ void InteractionModelEngine::OnResponseTimeout(Messaging::ExchangeContext * ec)
10691069
}
10701070

10711071
#if CHIP_CONFIG_ENABLE_READ_CLIENT
1072-
void InteractionModelEngine::OnActiveModeNotification(ScopedNodeId aPeer)
1072+
void InteractionModelEngine::OnActiveModeNotification(ScopedNodeId aPeer, uint64_t aMonitoredSubject)
10731073
{
10741074
for (ReadClient * pListItem = mpActiveReadClientList; pListItem != nullptr;)
10751075
{
10761076
auto pNextItem = pListItem->GetNextClient();
10771077
// It is possible that pListItem is destroyed by the app in OnActiveModeNotification.
10781078
// Get the next item before invoking `OnActiveModeNotification`.
1079-
if (ScopedNodeId(pListItem->GetPeerNodeId(), pListItem->GetFabricIndex()) == aPeer)
1079+
CATValues cats;
1080+
mpFabricTable->FetchCATs(pListItem->GetFabricIndex(), cats);
1081+
if (ScopedNodeId(pListItem->GetPeerNodeId(), pListItem->GetFabricIndex()) == aPeer && cats.CheckSubjectAgainstCATs(aMonitoredSubject))
10801082
{
10811083
pListItem->OnActiveModeNotification();
10821084
}

‎src/app/InteractionModelEngine.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -238,13 +238,13 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
238238
*
239239
* See ReadClient::OnActiveModeNotification
240240
*/
241-
void OnActiveModeNotification(ScopedNodeId aPeer);
241+
void OnActiveModeNotification(ScopedNodeId aPeer, uint64_t aMonitoredSubject);
242242

243243
/**
244244
* Used to notify when a peer becomes LIT ICD or vice versa.
245245
*
246246
* ReadClient will call this function when it finds any updates of the OperatingMode attribute from ICD management
247-
* cluster. The application doesn't need to call this function, usually.
247+
* cluster. The application doesn't need to call this function, usually.
248248
*/
249249
void OnPeerTypeChange(ScopedNodeId aPeer, ReadClient::PeerType aType);
250250

‎src/app/ReadClient.cpp

+8-7
Original file line numberDiff line numberDiff line change
@@ -490,10 +490,6 @@ void ReadClient::OnActiveModeNotification()
490490
// called, either mEventPathParamsListSize or mAttributePathParamsListSize is not 0.
491491
VerifyOrReturn(mReadPrepareParams.mEventPathParamsListSize != 0 || mReadPrepareParams.mAttributePathParamsListSize != 0);
492492

493-
// If mCatsMatchCheckIn is true, it means cats used in icd registration matches with the one in current subscription,
494-
// OnActiveModeNotification continues to revive the subscription as needed, otherwise, do nothing.
495-
VerifyOrReturn(mReadPrepareParams.mCatsMatchCheckIn);
496-
497493
// When we reach here, the subscription definitely exceeded the liveness timeout. Just continue the unfinished resubscription
498494
// logic in `OnLivenessTimeoutCallback`.
499495
if (IsInactiveICDSubscription())
@@ -502,9 +498,8 @@ void ReadClient::OnActiveModeNotification()
502498
return;
503499
}
504500

505-
// If the server sends out check-in message, and there is a tracked active subscription in client side at the same time, it
506-
// means current client does not realize this tracked subscription has gone, and we should forcibly timeout current
507-
// subscription, and schedule a new one.
501+
// When receiving check-in message, it means all tracked subscriptions for this node has gone, if there is a related active subscription
502+
// and subscription has not yet rescheduled in client side, we should forcibly timeout the current subscription, and schedule a new one.
508503
if (!mIsResubscriptionScheduled)
509504
{
510505
// Closing will ultimately trigger ScheduleResubscription with the aReestablishCASE argument set to true, effectively
@@ -525,6 +520,12 @@ void ReadClient::OnPeerTypeChange(PeerType aType)
525520
mIsPeerLIT = (aType == PeerType::kLITICD);
526521

527522
ChipLogProgress(DataManagement, "Peer is now %s LIT ICD.", mIsPeerLIT ? "a" : "not a");
523+
524+
// If the peer is no longer LIT and in inactive ICD subscription status, try to wake up the subscription and do resubscribe.
525+
if (!mIsPeerLIT && IsInactiveICDSubscription())
526+
{
527+
TriggerResubscriptionForLivenessTimeout(CHIP_ERROR_TIMEOUT);
528+
}
528529
}
529530

530531
CHIP_ERROR ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,

‎src/app/ReadPrepareParams.h

-5
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,6 @@ struct ReadPrepareParams
4949
bool mIsFabricFiltered = true;
5050
bool mIsPeerLIT = false;
5151

52-
// see ReadClient::OnActiveModeNotification
53-
bool mCatsMatchCheckIn = true;
54-
5552
ReadPrepareParams() {}
5653
ReadPrepareParams(const SessionHandle & sessionHandle) { mSessionHolder.Grab(sessionHandle); }
5754
ReadPrepareParams(ReadPrepareParams && other) : mSessionHolder(other.mSessionHolder)
@@ -69,7 +66,6 @@ struct ReadPrepareParams
6966
mTimeout = other.mTimeout;
7067
mIsFabricFiltered = other.mIsFabricFiltered;
7168
mIsPeerLIT = other.mIsPeerLIT;
72-
mCatsMatchCheckIn = other.mCatsMatchCheckIn;
7369
other.mpEventPathParamsList = nullptr;
7470
other.mEventPathParamsListSize = 0;
7571
other.mpAttributePathParamsList = nullptr;
@@ -95,7 +91,6 @@ struct ReadPrepareParams
9591
mTimeout = other.mTimeout;
9692
mIsFabricFiltered = other.mIsFabricFiltered;
9793
mIsPeerLIT = other.mIsPeerLIT;
98-
mCatsMatchCheckIn = other.mCatsMatchCheckIn;
9994
other.mpEventPathParamsList = nullptr;
10095
other.mEventPathParamsListSize = 0;
10196
other.mpAttributePathParamsList = nullptr;

‎src/app/icd/client/CheckInHandler.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ CHIP_ERROR CheckInHandler::OnMessageReceived(Messaging::ExchangeContext * ec, co
138138
mpICDClientStorage->StoreEntry(clientInfo);
139139
mpCheckInDelegate->OnCheckInComplete(clientInfo);
140140
#if CHIP_CONFIG_ENABLE_READ_CLIENT
141-
mpImEngine->OnActiveModeNotification(clientInfo.peer_node);
141+
mpImEngine->OnActiveModeNotification(clientInfo.peer_node, clientInfo.monitored_subject);
142142
#endif // CHIP_CONFIG_ENABLE_READ_CLIENT
143143
}
144144

‎src/app/icd/client/RefreshKeySender.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ CHIP_ERROR RefreshKeySender::RegisterClientWithNewKey(Messaging::ExchangeManager
6969

7070
mpCheckInDelegate->OnCheckInComplete(mICDClientInfo);
7171
#if CHIP_CONFIG_ENABLE_READ_CLIENT
72-
mpImEngine->OnActiveModeNotification(mICDClientInfo.peer_node);
72+
mpImEngine->OnActiveModeNotification(mICDClientInfo.peer_node,mICDClientInfo.monitored_subject);
7373
#endif // CHIP_CONFIG_ENABLE_READ_CLIENT
7474
mpCheckInDelegate->OnKeyRefreshDone(this, CHIP_NO_ERROR);
7575
};

‎src/controller/tests/data_model/TestRead.cpp

+4-5
Original file line numberDiff line numberDiff line change
@@ -2330,7 +2330,7 @@ TEST_F(TestRead, TestSubscribe_OnActiveModeNotification)
23302330
GetLoopback().mNumMessagesToDrop = 0;
23312331
callback.ClearCounters();
23322332
InteractionModelEngine::GetInstance()->OnActiveModeNotification(
2333-
ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()));
2333+
ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()), static_cast<uint64_t>(0));
23342334
EXPECT_EQ(callback.mOnResubscriptionsAttempted, 1);
23352335
EXPECT_EQ(callback.mLastError, CHIP_ERROR_TIMEOUT);
23362336

@@ -2402,7 +2402,7 @@ TEST_F(TestRead, TestSubscribe_SubGoAwayInserverOnActiveModeNotification)
24022402
GetLoopback().mNumMessagesToDrop = 0;
24032403
callback.ClearCounters();
24042404
InteractionModelEngine::GetInstance()->OnActiveModeNotification(
2405-
ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()));
2405+
ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()), static_cast<uint64_t>(0));
24062406
EXPECT_EQ(callback.mOnResubscriptionsAttempted, 1);
24072407
EXPECT_EQ(callback.mLastError, CHIP_ERROR_TIMEOUT);
24082408

@@ -2453,7 +2453,6 @@ TEST_F(TestRead, TestSubscribe_MismatchedSubGoAwayInserverOnActiveModeNotificati
24532453
attributePathParams[0].mEndpointId = kTestEndpointId;
24542454
attributePathParams[0].mClusterId = Clusters::UnitTesting::Id;
24552455
attributePathParams[0].mAttributeId = Clusters::UnitTesting::Attributes::Boolean::Id;
2456-
readPrepareParams.mCatsMatchCheckIn = false;
24572456
constexpr uint16_t maxIntervalCeilingSeconds = 1;
24582457

24592458
readPrepareParams.mMaxIntervalCeilingSeconds = maxIntervalCeilingSeconds;
@@ -2474,7 +2473,7 @@ TEST_F(TestRead, TestSubscribe_MismatchedSubGoAwayInserverOnActiveModeNotificati
24742473
GetLoopback().mNumMessagesToDrop = 0;
24752474
callback.ClearCounters();
24762475
InteractionModelEngine::GetInstance()->OnActiveModeNotification(
2477-
ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()));
2476+
ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()), static_cast<uint64_t>(0));
24782477
EXPECT_EQ(callback.mOnResubscriptionsAttempted, 0);
24792478
EXPECT_EQ(callback.mLastError, CHIP_NO_ERROR);
24802479
EXPECT_EQ(callback.mOnError, 0);
@@ -2526,7 +2525,7 @@ TEST_F(TestRead, TestSubscribeFailed_OnActiveModeNotification)
25262525
GetLoopback().mNumMessagesToDrop = 0;
25272526
callback.ClearCounters();
25282527
InteractionModelEngine::GetInstance()->OnActiveModeNotification(
2529-
ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()));
2528+
ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()), static_cast<uint64_t>(0));
25302529
//
25312530
// Drive servicing IO till we have established a subscription.
25322531
//

0 commit comments

Comments
 (0)