From 294119ddc8b9cf3e498352bb5566369fca3dcb81 Mon Sep 17 00:00:00 2001 From: yunhanw Date: Wed, 26 Feb 2025 15:13:04 -0800 Subject: [PATCH 1/4] Resubscribe when check-in is received and it is not in inactive state --- src/app/ReadClient.cpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 2d609eb1afa05a..df2142d2eb7cd7 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -484,14 +484,24 @@ CHIP_ERROR ReadClient::GenerateDataVersionFilterList(DataVersionFilterIBs::Build void ReadClient::OnActiveModeNotification() { - // This function just tries to complete the deferred resubscription logic in `OnLivenessTimeoutCallback`. VerifyOrDie(mpImEngine->InActiveReadClientList(this)); - // If we are not in InactiveICDSubscription state, that means the liveness timeout has not been reached. Simply do nothing. - VerifyOrReturn(IsInactiveICDSubscription()); - // When we reach here, the subscription definitely exceeded the liveness timeout. Just continue the unfinished resubscription // logic in `OnLivenessTimeoutCallback`. - TriggerResubscriptionForLivenessTimeout(CHIP_ERROR_TIMEOUT); + if (IsInactiveICDSubscription()) + { + TriggerResubscriptionForLivenessTimeout(CHIP_ERROR_TIMEOUT); + return; + } + + if (mIsResubscriptionScheduled) + { + // If a subscription fails and resubscription is scheduled, a check-in message should cancel + // the pending resubscription and initiate a new subscription immediately." + mNumRetries = 0; + CancelResubscribeTimer(); + TriggerResubscriptionForLivenessTimeout(CHIP_ERROR_TIMEOUT); + } + return; } void ReadClient::OnPeerTypeChange(PeerType aType) From 375f1380ffbe49306b6760b09dff349022822aab Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Wed, 26 Feb 2025 23:27:34 +0000 Subject: [PATCH 2/4] Restyled by whitespace --- src/app/ReadClient.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index df2142d2eb7cd7..c1eb9eba023c14 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -495,7 +495,7 @@ void ReadClient::OnActiveModeNotification() if (mIsResubscriptionScheduled) { - // If a subscription fails and resubscription is scheduled, a check-in message should cancel + // If a subscription fails and resubscription is scheduled, a check-in message should cancel // the pending resubscription and initiate a new subscription immediately." mNumRetries = 0; CancelResubscribeTimer(); From 10db77849ec6d02fbbcc6bf7958ff359c8b9b0fc Mon Sep 17 00:00:00 2001 From: yunhanw Date: Thu, 27 Feb 2025 11:40:42 -0800 Subject: [PATCH 3/4] address comments --- src/app/ReadClient.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index c1eb9eba023c14..baa628b9cf0f94 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -493,14 +493,7 @@ void ReadClient::OnActiveModeNotification() return; } - if (mIsResubscriptionScheduled) - { - // If a subscription fails and resubscription is scheduled, a check-in message should cancel - // the pending resubscription and initiate a new subscription immediately." - mNumRetries = 0; - CancelResubscribeTimer(); - TriggerResubscriptionForLivenessTimeout(CHIP_ERROR_TIMEOUT); - } + TriggerResubscribeIfScheduled("check-in message"); return; } From 1ef00114ce0e5e50952ade9bd40ebb09bd1139ae Mon Sep 17 00:00:00 2001 From: Yunhan Wang Date: Fri, 28 Feb 2025 09:51:40 -0800 Subject: [PATCH 4/4] add test --- src/app/ReadClient.cpp | 1 - src/controller/tests/data_model/TestRead.cpp | 63 ++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index baa628b9cf0f94..da127253cf7d12 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -494,7 +494,6 @@ void ReadClient::OnActiveModeNotification() } TriggerResubscribeIfScheduled("check-in message"); - return; } void ReadClient::OnPeerTypeChange(PeerType aType) diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index f4999ab416bdba..4cb6cd1acf5679 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -2354,6 +2354,69 @@ TEST_F(TestRead, TestSubscribe_OnActiveModeNotification) EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u); } +TEST_F(TestRead, TestSubscribeFailed_OnActiveModeNotification) +{ + auto sessionHandle = GetSessionBobToAlice(); + + SetMRPMode(MessagingContext::MRPMode::kResponsive); + + { + TestResubscriptionCallback callback; + ReadClient readClient(InteractionModelEngine::GetInstance(), &GetExchangeManager(), callback, + ReadClient::InteractionType::Subscribe); + + callback.mScheduleLITResubscribeImmediately = false; + callback.SetReadClient(&readClient); + + ReadPrepareParams readPrepareParams(GetSessionBobToAlice()); + + AttributePathParams attributePathParams[1]; + readPrepareParams.mpAttributePathParamsList = attributePathParams; + readPrepareParams.mAttributePathParamsListSize = MATTER_ARRAY_SIZE(attributePathParams); + attributePathParams[0].mEndpointId = kTestEndpointId; + attributePathParams[0].mClusterId = Clusters::UnitTesting::Id; + attributePathParams[0].mAttributeId = Clusters::UnitTesting::Attributes::Boolean::Id; + + constexpr uint16_t maxIntervalCeilingSeconds = 1; + + readPrepareParams.mMaxIntervalCeilingSeconds = maxIntervalCeilingSeconds; + readPrepareParams.mIsPeerLIT = true; + + auto err = readClient.SendAutoResubscribeRequest(std::move(readPrepareParams)); + EXPECT_EQ(err, CHIP_NO_ERROR); + + GetLoopback().mNumMessagesToDrop = LoopbackTransport::kUnlimitedMessageCount; + GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(6000), + [&]() { return callback.mOnResubscriptionsAttempted == 1; }); + EXPECT_EQ(callback.mOnSubscriptionEstablishedCount, 0); + + GetLoopback().mNumMessagesToDrop = 0; + callback.ClearCounters(); + InteractionModelEngine::GetInstance()->OnActiveModeNotification( + ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex())); + // + // Drive servicing IO till we have established a subscription. + // + GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(2000), + [&]() { return callback.mOnSubscriptionEstablishedCount == 1; }); + EXPECT_EQ(callback.mOnSubscriptionEstablishedCount, 1); + + // + // With re-sub enabled, we shouldn't have encountered any errors + // + EXPECT_EQ(callback.mOnError, 0); + EXPECT_EQ(callback.mOnDone, 0); + + GetLoopback().mNumMessagesToDrop = 0; + callback.ClearCounters(); + } + + SetMRPMode(MessagingContext::MRPMode::kDefault); + + InteractionModelEngine::GetInstance()->ShutdownActiveReads(); + EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u); +} + /** * When the liveness timeout of a subscription to ICD is reached, the subscription will enter "InactiveICDSubscription" state, the * client should call "OnActiveModeNotification" to re-activate it again when the check-in message is received from the ICD.