address comments

Author: yunhanw-google

src/app/InteractionModelEngine.cpp

@@ -1069,14 +1069,15 @@ void InteractionModelEngine::OnResponseTimeout(Messaging::ExchangeContext * ec)
 }
 
 #if CHIP_CONFIG_ENABLE_READ_CLIENT
-void InteractionModelEngine::OnActiveModeNotification(ScopedNodeId aPeer)
+void InteractionModelEngine::OnActiveModeNotification(ScopedNodeId aCheckInNode, ScopedNodeId aPeer)
 {
     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)
+        if (ScopedNodeId(pListItem->GetPeerNodeId(), pListItem->GetFabricIndex()) == aPeer && 
+            aCheckInNode == pListItem->GetLocalScopedNodeId())
         {
             pListItem->OnActiveModeNotification();
         }

src/app/InteractionModelEngine.h

@@ -236,10 +236,9 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
     /**
      *  Activate the idle subscriptions.
      *
-     *  When subscribing to ICD and liveness timeout reached, the read client will move to `InactiveICDSubscription` state and
-     * resubscription can be triggered via OnActiveModeNotification().
+     *  See ReadClient::OnActiveModeNotification
      */
-    void OnActiveModeNotification(ScopedNodeId aPeer);
+    void OnActiveModeNotification(ScopedNodeId aCheckInNode, ScopedNodeId aPeer);
 
     /**
      *  Used to notify when a peer becomes LIT ICD or vice versa.

src/app/ReadClient.cpp

@@ -484,8 +484,12 @@ CHIP_ERROR ReadClient::GenerateDataVersionFilterList(DataVersionFilterIBs::Build
 
 void ReadClient::OnActiveModeNotification()
 {
-    // Note: this API only works when issuing subscription via SendAutoResubscribeRequest.
     VerifyOrDie(mpImEngine->InActiveReadClientList(this));
+
+    // Note: this API only works when issuing subscription via SendAutoResubscribeRequest, when SendAutoResubscribeRequest is called,
+    // either mEventPathParamsListSize or mAttributePathParamsListSize is not 0.
+    VerifyOrDie(mReadPrepareParams.mEventPathParamsListSize != 0 || mReadPrepareParams.mAttributePathParamsListSize != 0)
+
     // When we reach here, the subscription definitely exceeded the liveness timeout. Just continue the unfinished resubscription
     // logic in `OnLivenessTimeoutCallback`.
     if (IsInactiveICDSubscription())

src/app/ReadClient.h

@@ -351,11 +351,16 @@ class ReadClient : public Messaging::ExchangeDelegate
      *  Re-activate an inactive subscription.
      *
      *  When subscribing to LIT-ICD and liveness timeout reached and OnResubscriptionNeeded returns
-     * CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT, the read client will move to the InactiveICDSubscription state and
-     * resubscription can be triggered via OnActiveModeNotification().
+     *  CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT, the read client will move to the InactiveICDSubscription state and
+     *  resubscription can be triggered via OnActiveModeNotification().
      *
      *  If the subscription is not in the `InactiveICDSubscription` state, this function will do nothing. So it is always safe to
-     * call this function when a check-in message is received.
+     *  call this function when a check-in message is received.
+     *  
+     *  If the server sends out check-in message, and there is no reschedule subscription yet in client side at the same time, it 
+     *  means current client does not realize subscription has gone, and we should forcibly timeout current subscription, and schedule a new one.
+     *  
+     *  This API only works when issuing subscription via SendAutoResubscribeRequest.
      */
     void OnActiveModeNotification();
 
@@ -503,6 +508,13 @@ class ReadClient : public Messaging::ExchangeDelegate
      */
     Optional<System::Clock::Timeout> GetSubscriptionTimeout();
 
+
+    Optional<ScopedNodeId> GetLocalScopedNodeId() const 
+    { 
+        VerifyOrReturnValue(aReadPrepareParams.mSessionHolder, NullOptional);
+        return MakeOptional(mReadPrepareParams.mSessionHolder->AsSecureSession()->GetLocalScopedNodeId()); 
+    }
+
 private:
     friend class TestReadInteraction;
     friend class InteractionModelEngine;

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.check_in_node, clientInfo.peer_node);
 #endif // CHIP_CONFIG_ENABLE_READ_CLIENT
     }
 

src/controller/tests/data_model/TestRead.cpp

@@ -2329,7 +2329,7 @@ TEST_F(TestRead, TestSubscribe_OnActiveModeNotification)
 
         GetLoopback().mNumMessagesToDrop = 0;
         callback.ClearCounters();
-        InteractionModelEngine::GetInstance()->OnActiveModeNotification(
+        InteractionModelEngine::GetInstance()->OnActiveModeNotification(readClient.GetLocalScopedNodeId().Value(),
             ScopedNodeId(readClient.GetPeerNodeId(), readClient.GetFabricIndex()));
         EXPECT_EQ(callback.mOnResubscriptionsAttempted, 1);
         EXPECT_EQ(callback.mLastError, CHIP_ERROR_TIMEOUT);