Skip to content

Commit 287a093

Browse files
yunhanw-googleyunhanw-google
committed
address comments and add tests
1 parent 9cda649 commit 287a093

File tree

3 files changed

+139
-6
lines changed

3 files changed

+139
-6
lines changed

‎src/app/ReadClient.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -486,9 +486,9 @@ void ReadClient::OnActiveModeNotification()
486486
{
487487
VerifyOrDie(mpImEngine->InActiveReadClientList(this));
488488

489-
// Note: this API only works when issuing subscription via SendAutoResubscribeRequest, when SendAutoResubscribeRequest is
489+
// Note: this API only works when issuing subscription via SendAutoResubscribeRequest. When SendAutoResubscribeRequest is
490490
// called, either mEventPathParamsListSize or mAttributePathParamsListSize is not 0.
491-
VerifyOrDie(mReadPrepareParams.mEventPathParamsListSize != 0 || mReadPrepareParams.mAttributePathParamsListSize != 0);
491+
VerifyOrReturn(mReadPrepareParams.mEventPathParamsListSize != 0 || mReadPrepareParams.mAttributePathParamsListSize != 0);
492492

493493
// If mCatsMatchCheckIn is true, it means cats used in icd registration matches with the one in current subscription, OnActiveModeNotification
494494
// continues to revive the subscription as needed, otherwise, do nothing.
@@ -502,8 +502,8 @@ void ReadClient::OnActiveModeNotification()
502502
return;
503503
}
504504

505-
// If the server sends out check-in message, and there is no reschedule subscription yet in client side at the same time, it
506-
// means current client does not realize subscription has gone, and we should forcibly timeout current subscription, and
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 subscription, and
507507
// schedule a new one.
508508
if (!mIsResubscriptionScheduled)
509509
{

‎src/app/ReadClient.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,8 @@ class ReadClient : public Messaging::ExchangeDelegate
357357
* If the subscription is not in the `InactiveICDSubscription` state, this function will do nothing. So it is always safe to
358358
* call this function when a check-in message is received.
359359
*
360-
* If the server sends out check-in message, and there is no reschedule subscription yet in client side at the same time, it
361-
* means current client does not realize subscription has gone, and we should forcibly timeout current subscription, and
360+
* If the server sends out check-in message, and there is a active tracked active subscription in client side at the same time, it
361+
* means current client does not realize this tracked subscription has gone, and we should forcibly timeout current subscription, and
362362
* schedule a new one.
363363
*
364364
* This API only works when issuing subscription via SendAutoResubscribeRequest.

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

+133
Original file line numberDiff line numberDiff line change
@@ -2354,6 +2354,139 @@ TEST_F(TestRead, TestSubscribe_OnActiveModeNotification)
23542354
EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);
23552355
}
23562356

2357+
/**
2358+
* When all tracked subscriptions go away in server, check-in message is received and OnActiveModeNotification is called in client side, the tracked
2359+
* subscription would be torn down and a new one would be rescheduled in client side.
2360+
*/
2361+
TEST_F(TestRead, TestSubscribe_SubGoAwayInserverOnActiveModeNotification)
2362+
{
2363+
auto sessionHandle = GetSessionBobToAlice();
2364+
2365+
SetMRPMode(MessagingContext::MRPMode::kResponsive);
2366+
2367+
{
2368+
TestResubscriptionCallback callback;
2369+
ReadClient readClient(InteractionModelEngine::GetInstance(), &GetExchangeManager(), callback,
2370+
ReadClient::InteractionType::Subscribe);
2371+
2372+
callback.mScheduleLITResubscribeImmediately = false;
2373+
callback.SetReadClient(&readClient);
2374+
2375+
ReadPrepareParams readPrepareParams(GetSessionBobToAlice());
2376+
2377+
// Read full wildcard paths, repeat twice to ensure chunking.
2378+
AttributePathParams attributePathParams[1];
2379+
readPrepareParams.mpAttributePathParamsList = attributePathParams;
2380+
readPrepareParams.mAttributePathParamsListSize = MATTER_ARRAY_SIZE(attributePathParams);
2381+
attributePathParams[0].mEndpointId = kTestEndpointId;
2382+
attributePathParams[0].mClusterId = Clusters::UnitTesting::Id;
2383+
attributePathParams[0].mAttributeId = Clusters::UnitTesting::Attributes::Boolean::Id;
2384+
2385+
constexpr uint16_t maxIntervalCeilingSeconds = 1;
2386+
2387+
readPrepareParams.mMaxIntervalCeilingSeconds = maxIntervalCeilingSeconds;
2388+
readPrepareParams.mIsPeerLIT = true;
2389+
2390+
auto err = readClient.SendAutoResubscribeRequest(std::move(readPrepareParams));
2391+
EXPECT_EQ(err, CHIP_NO_ERROR);
2392+
2393+
//
2394+
// Drive servicing IO till we have established a subscription.
2395+
//
2396+
GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(2000),
2397+
[&]() { return callback.mOnSubscriptionEstablishedCount >= 1; });
2398+
EXPECT_EQ(callback.mOnSubscriptionEstablishedCount, 1);
2399+
EXPECT_EQ(callback.mOnError, 0);
2400+
EXPECT_EQ(callback.mOnResubscriptionsAttempted, 0);
2401+
2402+
GetLoopback().mNumMessagesToDrop = 0;
2403+
callback.ClearCounters();
2404+
InteractionModelEngine::GetInstance()->OnActiveModeNotification(
2405+
ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()));
2406+
EXPECT_EQ(callback.mOnResubscriptionsAttempted, 1);
2407+
EXPECT_EQ(callback.mLastError, CHIP_ERROR_TIMEOUT);
2408+
2409+
//
2410+
// Drive servicing IO till we have established a subscription.
2411+
//
2412+
GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(2000),
2413+
[&]() { return callback.mOnSubscriptionEstablishedCount == 1; });
2414+
EXPECT_EQ(callback.mOnSubscriptionEstablishedCount, 1);
2415+
2416+
//
2417+
// With re-sub enabled, we shouldn't have encountered any errors
2418+
//
2419+
EXPECT_EQ(callback.mOnError, 0);
2420+
EXPECT_EQ(callback.mOnDone, 0);
2421+
}
2422+
2423+
SetMRPMode(MessagingContext::MRPMode::kDefault);
2424+
2425+
InteractionModelEngine::GetInstance()->ShutdownActiveReads();
2426+
EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);
2427+
}
2428+
2429+
/**
2430+
* When all tracked subscriptions go away in server, check-in message is received and OnActiveModeNotification is called in client side, the
2431+
* untracked subscription would be kept.
2432+
*/
2433+
TEST_F(TestRead, TestSubscribe_MismatchedSubGoAwayInserverOnActiveModeNotification)
2434+
{
2435+
auto sessionHandle = GetSessionBobToAlice();
2436+
2437+
SetMRPMode(MessagingContext::MRPMode::kResponsive);
2438+
2439+
{
2440+
TestResubscriptionCallback callback;
2441+
ReadClient readClient(InteractionModelEngine::GetInstance(), &GetExchangeManager(), callback,
2442+
ReadClient::InteractionType::Subscribe);
2443+
2444+
callback.mScheduleLITResubscribeImmediately = false;
2445+
callback.SetReadClient(&readClient);
2446+
2447+
ReadPrepareParams readPrepareParams(GetSessionBobToAlice());
2448+
2449+
// Read full wildcard paths, repeat twice to ensure chunking.
2450+
AttributePathParams attributePathParams[1];
2451+
readPrepareParams.mpAttributePathParamsList = attributePathParams;
2452+
readPrepareParams.mAttributePathParamsListSize = MATTER_ARRAY_SIZE(attributePathParams);
2453+
attributePathParams[0].mEndpointId = kTestEndpointId;
2454+
attributePathParams[0].mClusterId = Clusters::UnitTesting::Id;
2455+
attributePathParams[0].mAttributeId = Clusters::UnitTesting::Attributes::Boolean::Id;
2456+
readPrepareParams.mCatsMatchCheckIn = false;
2457+
constexpr uint16_t maxIntervalCeilingSeconds = 1;
2458+
2459+
readPrepareParams.mMaxIntervalCeilingSeconds = maxIntervalCeilingSeconds;
2460+
readPrepareParams.mIsPeerLIT = true;
2461+
2462+
auto err = readClient.SendAutoResubscribeRequest(std::move(readPrepareParams));
2463+
EXPECT_EQ(err, CHIP_NO_ERROR);
2464+
2465+
//
2466+
// Drive servicing IO till we have established a subscription.
2467+
//
2468+
GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(2000),
2469+
[&]() { return callback.mOnSubscriptionEstablishedCount >= 1; });
2470+
EXPECT_EQ(callback.mOnSubscriptionEstablishedCount, 1);
2471+
EXPECT_EQ(callback.mOnError, 0);
2472+
EXPECT_EQ(callback.mOnResubscriptionsAttempted, 0);
2473+
2474+
GetLoopback().mNumMessagesToDrop = 0;
2475+
callback.ClearCounters();
2476+
InteractionModelEngine::GetInstance()->OnActiveModeNotification(
2477+
ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()));
2478+
EXPECT_EQ(callback.mOnResubscriptionsAttempted, 0);
2479+
EXPECT_EQ(callback.mLastError, CHIP_NO_ERROR);
2480+
EXPECT_EQ(callback.mOnError, 0);
2481+
EXPECT_EQ(callback.mOnDone, 0);
2482+
}
2483+
2484+
SetMRPMode(MessagingContext::MRPMode::kDefault);
2485+
2486+
InteractionModelEngine::GetInstance()->ShutdownActiveReads();
2487+
EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);
2488+
}
2489+
23572490
TEST_F(TestRead, TestSubscribeFailed_OnActiveModeNotification)
23582491
{
23592492
auto sessionHandle = GetSessionBobToAlice();

0 commit comments

Comments
 (0)