address comments

Author: yunhanw-google

src/app/InteractionModelEngine.cpp

@@ -1069,14 +1069,16 @@ void InteractionModelEngine::OnResponseTimeout(Messaging::ExchangeContext * ec)
 }
 
 #if CHIP_CONFIG_ENABLE_READ_CLIENT
-void InteractionModelEngine::OnActiveModeNotification(ScopedNodeId aPeer)
+void InteractionModelEngine::OnActiveModeNotification(ScopedNodeId aPeer, uint64_t aMonitoredSubject)
 {
     for (ReadClient * pListItem = mpActiveReadClientList; pListItem != nullptr;)
     {
         auto pNextItem = pListItem->GetNextClient();
         // It is possible that pListItem is destroyed by the app in OnActiveModeNotification.
         // Get the next item before invoking `OnActiveModeNotification`.
-        if (ScopedNodeId(pListItem->GetPeerNodeId(), pListItem->GetFabricIndex()) == aPeer)
+        CATValues cats;
+        mpFabricTable->FetchCATs(pListItem->GetFabricIndex(), cats);
+        if (ScopedNodeId(pListItem->GetPeerNodeId(), pListItem->GetFabricIndex()) == aPeer && cats.CheckSubjectAgainstCATs(aMonitoredSubject))
         {
             pListItem->OnActiveModeNotification();
         }

src/app/InteractionModelEngine.h

@@ -238,13 +238,13 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
      *
      *  See ReadClient::OnActiveModeNotification
      */
-    void OnActiveModeNotification(ScopedNodeId aPeer);
+    void OnActiveModeNotification(ScopedNodeId aPeer, uint64_t aMonitoredSubject);
 
     /**
      *  Used to notify when a peer becomes LIT ICD or vice versa.
      *
      *  ReadClient will call this function when it finds any updates of the OperatingMode attribute from ICD management
-     * cluster. The application doesn't need to call this function, usually.
+     *  cluster. The application doesn't need to call this function, usually.
      */
     void OnPeerTypeChange(ScopedNodeId aPeer, ReadClient::PeerType aType);
 

src/app/ReadClient.cpp

@@ -490,10 +490,6 @@ void ReadClient::OnActiveModeNotification()
     // called, either mEventPathParamsListSize or mAttributePathParamsListSize is not 0.
     VerifyOrReturn(mReadPrepareParams.mEventPathParamsListSize != 0 || mReadPrepareParams.mAttributePathParamsListSize != 0);
 
-    // If mCatsMatchCheckIn is true, it means cats used in icd registration matches with the one in current subscription,
-    // OnActiveModeNotification continues to revive the subscription as needed, otherwise, do nothing.
-    VerifyOrReturn(mReadPrepareParams.mCatsMatchCheckIn);
-
     // When we reach here, the subscription definitely exceeded the liveness timeout. Just continue the unfinished resubscription
     // logic in `OnLivenessTimeoutCallback`.
     if (IsInactiveICDSubscription())
@@ -502,9 +498,8 @@ void ReadClient::OnActiveModeNotification()
         return;
     }
 
-    // If the server sends out check-in message, and there is a tracked active subscription in client side at the same time, it
-    // means current client does not realize this tracked subscription has gone, and we should forcibly timeout current
-    // subscription, and schedule a new one.
+    // When receiving check-in message, it means all tracked subscriptions for this node has gone in server side, if there is a related active subscription
+    // and subscription has not yet rescheduled in client side, we should forcibly timeout the current subscription, and schedule a new one.
     if (!mIsResubscriptionScheduled)
     {
         // Closing will ultimately trigger ScheduleResubscription with the aReestablishCASE argument set to true, effectively
@@ -525,6 +520,12 @@ void ReadClient::OnPeerTypeChange(PeerType aType)
     mIsPeerLIT = (aType == PeerType::kLITICD);
 
     ChipLogProgress(DataManagement, "Peer is now %s LIT ICD.", mIsPeerLIT ? "a" : "not a");
+
+    // If the peer is no longer LIT and in inactive ICD subscription status, try to wake up the subscription and do resubscribe.
+    if (!mIsPeerLIT && IsInactiveICDSubscription())
+    {
+        TriggerResubscriptionForLivenessTimeout(CHIP_ERROR_TIMEOUT);
+    }
 }
 
 CHIP_ERROR ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,

src/app/ReadPrepareParams.h

@@ -49,9 +49,6 @@ struct ReadPrepareParams
     bool mIsFabricFiltered              = true;
     bool mIsPeerLIT                     = false;
 
-    // see ReadClient::OnActiveModeNotification
-    bool mCatsMatchCheckIn = true;
-
     ReadPrepareParams() {}
     ReadPrepareParams(const SessionHandle & sessionHandle) { mSessionHolder.Grab(sessionHandle); }
     ReadPrepareParams(ReadPrepareParams && other) : mSessionHolder(other.mSessionHolder)
@@ -69,7 +66,6 @@ struct ReadPrepareParams
         mTimeout                           = other.mTimeout;
         mIsFabricFiltered                  = other.mIsFabricFiltered;
         mIsPeerLIT                         = other.mIsPeerLIT;
-        mCatsMatchCheckIn                  = other.mCatsMatchCheckIn;
         other.mpEventPathParamsList        = nullptr;
         other.mEventPathParamsListSize     = 0;
         other.mpAttributePathParamsList    = nullptr;
@@ -95,7 +91,6 @@ struct ReadPrepareParams
         mTimeout                           = other.mTimeout;
         mIsFabricFiltered                  = other.mIsFabricFiltered;
         mIsPeerLIT                         = other.mIsPeerLIT;
-        mCatsMatchCheckIn                  = other.mCatsMatchCheckIn;
         other.mpEventPathParamsList        = nullptr;
         other.mEventPathParamsListSize     = 0;
         other.mpAttributePathParamsList    = nullptr;

src/app/icd/client/CheckInHandler.cpp

@@ -138,7 +138,7 @@ CHIP_ERROR CheckInHandler::OnMessageReceived(Messaging::ExchangeContext * ec, co
         mpICDClientStorage->StoreEntry(clientInfo);
         mpCheckInDelegate->OnCheckInComplete(clientInfo);
 #if CHIP_CONFIG_ENABLE_READ_CLIENT
-        mpImEngine->OnActiveModeNotification(clientInfo.peer_node);
+        mpImEngine->OnActiveModeNotification(clientInfo.peer_node, clientInfo.monitored_subject);
 #endif // CHIP_CONFIG_ENABLE_READ_CLIENT
     }
 

src/app/icd/client/RefreshKeySender.cpp

@@ -69,7 +69,7 @@ CHIP_ERROR RefreshKeySender::RegisterClientWithNewKey(Messaging::ExchangeManager
 
         mpCheckInDelegate->OnCheckInComplete(mICDClientInfo);
 #if CHIP_CONFIG_ENABLE_READ_CLIENT
-        mpImEngine->OnActiveModeNotification(mICDClientInfo.peer_node);
+        mpImEngine->OnActiveModeNotification(mICDClientInfo.peer_node,mICDClientInfo.monitored_subject);
 #endif // CHIP_CONFIG_ENABLE_READ_CLIENT
         mpCheckInDelegate->OnKeyRefreshDone(this, CHIP_NO_ERROR);
     };

src/controller/tests/data_model/TestRead.cpp

@@ -2330,7 +2330,7 @@ TEST_F(TestRead, TestSubscribe_OnActiveModeNotification)
         GetLoopback().mNumMessagesToDrop = 0;
         callback.ClearCounters();
         InteractionModelEngine::GetInstance()->OnActiveModeNotification(
-            ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()));
+            ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()), static_cast<uint64_t>(0));
         EXPECT_EQ(callback.mOnResubscriptionsAttempted, 1);
         EXPECT_EQ(callback.mLastError, CHIP_ERROR_TIMEOUT);
 
@@ -2402,7 +2402,7 @@ TEST_F(TestRead, TestSubscribe_SubGoAwayInserverOnActiveModeNotification)
         GetLoopback().mNumMessagesToDrop = 0;
         callback.ClearCounters();
         InteractionModelEngine::GetInstance()->OnActiveModeNotification(
-            ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()));
+            ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()), static_cast<uint64_t>(0));
         EXPECT_EQ(callback.mOnResubscriptionsAttempted, 1);
         EXPECT_EQ(callback.mLastError, CHIP_ERROR_TIMEOUT);
 
@@ -2453,7 +2453,6 @@ TEST_F(TestRead, TestSubscribe_MismatchedSubGoAwayInserverOnActiveModeNotificati
         attributePathParams[0].mEndpointId             = kTestEndpointId;
         attributePathParams[0].mClusterId              = Clusters::UnitTesting::Id;
         attributePathParams[0].mAttributeId            = Clusters::UnitTesting::Attributes::Boolean::Id;
-        readPrepareParams.mCatsMatchCheckIn            = false;
         constexpr uint16_t maxIntervalCeilingSeconds   = 1;
 
         readPrepareParams.mMaxIntervalCeilingSeconds = maxIntervalCeilingSeconds;
@@ -2474,7 +2473,7 @@ TEST_F(TestRead, TestSubscribe_MismatchedSubGoAwayInserverOnActiveModeNotificati
         GetLoopback().mNumMessagesToDrop = 0;
         callback.ClearCounters();
         InteractionModelEngine::GetInstance()->OnActiveModeNotification(
-            ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()));
+            ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()), static_cast<uint64_t>(0));
         EXPECT_EQ(callback.mOnResubscriptionsAttempted, 0);
         EXPECT_EQ(callback.mLastError, CHIP_NO_ERROR);
         EXPECT_EQ(callback.mOnError, 0);
@@ -2526,7 +2525,7 @@ TEST_F(TestRead, TestSubscribeFailed_OnActiveModeNotification)
         GetLoopback().mNumMessagesToDrop = 0;
         callback.ClearCounters();
         InteractionModelEngine::GetInstance()->OnActiveModeNotification(
-            ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()));
+            ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()), static_cast<uint64_t>(0));
         //
         // Drive servicing IO till we have established a subscription.
         //