[icd] add IdleSubscription to ReadClient to support LIT ICD (#30812)

* [IM] Support put ReadClient to on hold when device is sleeping * [icd] add IdleSubscription to ReadClient to support LIT ICD * Update ReadClient.h fix typo * address comments * Update src/app/ReadClient.cpp Co-authored-by: mkardous-silabs <[email protected]> * Update src/app/ReadClient.cpp Co-authored-by: mkardous-silabs <[email protected]> * Update src/app/ReadClient.h Co-authored-by: mkardous-silabs <[email protected]> * Update src/app/ReadClient.h Co-authored-by: mkardous-silabs <[email protected]> * Update src/app/ReadClient.h Co-authored-by: mkardous-silabs <[email protected]> * Update src/app/ReadClient.h Co-authored-by: mkardous-silabs <[email protected]> * Update src/app/ReadClient.h Co-authored-by: mkardous-silabs <[email protected]> * address comments --------- Co-authored-by: yunhanw-google <[email protected]> Co-authored-by: mkardous-silabs <[email protected]>
project-chip · Apr 18, 2024 · cfcf212 · cfcf212
1 parent 1af6bf4
commit cfcf212
Show file tree

Hide file tree

Showing 10 changed files with 307 additions and 20 deletions.
diff --git a/docs/ERROR_CODES.md b/docs/ERROR_CODES.md
@@ -37,6 +37,7 @@ This file was **AUTOMATICALLY** generated by
 | 19        | 0x13  | `CHIP_ERROR_INTEGRITY_CHECK_FAILED`                |
 | 20        | 0x14  | `CHIP_ERROR_INVALID_SIGNATURE`                     |
 | 21        | 0x15  | `CHIP_ERROR_INVALID_TLV_CHAR_STRING`               |
+| 22        | 0x16  | `CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT`        |
 | 23        | 0x17  | `CHIP_ERROR_UNSUPPORTED_SIGNATURE_TYPE`            |
 | 24        | 0x18  | `CHIP_ERROR_INVALID_MESSAGE_LENGTH`                |
 | 25        | 0x19  | `CHIP_ERROR_BUFFER_TOO_SMALL`                      |

diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp
@@ -980,6 +980,21 @@ void InteractionModelEngine::OnResponseTimeout(Messaging::ExchangeContext * ec)
 }
 
 #if CHIP_CONFIG_ENABLE_READ_CLIENT
+void InteractionModelEngine::OnActiveModeNotification(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)
+        {
+            pListItem->OnActiveModeNotification();
+        }
+        pListItem = pNextItem;
+    }
+}
+
 void InteractionModelEngine::AddReadClient(ReadClient * apReadClient)
 {
     apReadClient->SetNextClient(mpActiveReadClientList);

diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h
@@ -239,6 +239,14 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
                       const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);
 
 #if CHIP_CONFIG_ENABLE_READ_CLIENT
+    /**
+     *  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().
+     */
+    void OnActiveModeNotification(ScopedNodeId aPeer);
+
     /**
      * Add a read client to the internally tracked list of weak references. This list is used to
      * correctly dispatch unsolicited reports to the right matching handler by subscription ID.

diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp
@@ -67,6 +67,7 @@ void ReadClient::ClearActiveSubscriptionState()
     mMaxInterval                  = 0;
     mSubscriptionId               = 0;
     mIsResubscriptionScheduled    = false;
+
     MoveToState(ClientState::Idle);
 }
 
@@ -187,11 +188,20 @@ void ReadClient::Close(CHIP_ERROR aError, bool allowResubscription)
             if (allowResubscription &&
                 (mReadPrepareParams.mEventPathParamsListSize != 0 || mReadPrepareParams.mAttributePathParamsListSize != 0))
             {
+                CHIP_ERROR originalReason = aError;
+
                 aError = mpCallback.OnResubscriptionNeeded(this, aError);
                 if (aError == CHIP_NO_ERROR)
                 {
                     return;
                 }
+                if (aError == CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT)
+                {
+                    VerifyOrDie(originalReason == CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT);
+                    ChipLogProgress(DataManagement, "ICD device is inactive mark subscription as InactiveICDSubscription");
+                    MoveToState(ClientState::InactiveICDSubscription);
+                    return;
+                }
             }
 
             //
@@ -223,6 +233,8 @@ const char * ReadClient::GetStateStr() const
         return "AwaitingSubscribeResponse";
     case ClientState::SubscriptionActive:
         return "SubscriptionActive";
+    case ClientState::InactiveICDSubscription:
+        return "InactiveICDSubscription";
     }
 #endif // CHIP_DETAIL_LOGGING
     return "N/A";
@@ -427,6 +439,18 @@ CHIP_ERROR ReadClient::GenerateDataVersionFilterList(DataVersionFilterIBs::Build
     return CHIP_NO_ERROR;
 }
 
+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);
+}
+
 CHIP_ERROR ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,
                                          System::PacketBufferHandle && aPayload)
 {
@@ -884,6 +908,10 @@ void ReadClient::OnLivenessTimeoutCallback(System::Layer * apSystemLayer, void *
 {
     ReadClient * const _this = reinterpret_cast<ReadClient *>(apAppState);
 
+    // TODO: add a more specific error here for liveness timeout failure to distinguish between other classes of timeouts (i.e
+    // response timeouts).
+    CHIP_ERROR subscriptionTerminationCause = CHIP_ERROR_TIMEOUT;
+
     //
     // Might as well try to see if this instance exists in the tracked list in the IM.
     // This might blow-up if either the client has since been free'ed (use-after-free), or if the engine has since
@@ -895,32 +923,39 @@ void ReadClient::OnLivenessTimeoutCallback(System::Layer * apSystemLayer, void *
                  "Subscription Liveness timeout with SubscriptionID = 0x%08" PRIx32 ", Peer = %02x:" ChipLogFormatX64,
                  _this->mSubscriptionId, _this->GetFabricIndex(), ChipLogValueX64(_this->GetPeerNodeId()));
 
+    if (_this->mIsPeerLIT)
+    {
+        subscriptionTerminationCause = CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT;
+    }
+
+    _this->TriggerResubscriptionForLivenessTimeout(subscriptionTerminationCause);
+}
+
+void ReadClient::TriggerResubscriptionForLivenessTimeout(CHIP_ERROR aReason)
+{
     // We didn't get a message from the server on time; it's possible that it no
     // longer has a useful CASE session to us.  Mark defunct all sessions that
     // have not seen peer activity in at least as long as our session.
-    const auto & holder = _this->mReadPrepareParams.mSessionHolder;
+    const auto & holder = mReadPrepareParams.mSessionHolder;
     if (holder)
     {
         System::Clock::Timestamp lastPeerActivity = holder->AsSecureSession()->GetLastPeerActivityTime();
-        _this->mpImEngine->GetExchangeManager()->GetSessionManager()->ForEachMatchingSession(
-            _this->mPeer, [&lastPeerActivity](auto * session) {
-                if (!session->IsCASESession())
-                {
-                    return;
-                }
+        mpImEngine->GetExchangeManager()->GetSessionManager()->ForEachMatchingSession(mPeer, [&lastPeerActivity](auto * session) {
+            if (!session->IsCASESession())
+            {
+                return;
+            }
 
-                if (session->GetLastPeerActivityTime() > lastPeerActivity)
-                {
-                    return;
-                }
+            if (session->GetLastPeerActivityTime() > lastPeerActivity)
+            {
+                return;
+            }
 
-                session->MarkAsDefunct();
-            });
+            session->MarkAsDefunct();
+        });
     }
 
-    // TODO: add a more specific error here for liveness timeout failure to distinguish between other classes of timeouts (i.e
-    // response timeouts).
-    _this->Close(CHIP_ERROR_TIMEOUT);
+    Close(aReason);
 }
 
 CHIP_ERROR ReadClient::ProcessSubscribeResponse(System::PacketBufferHandle && aPayload)
@@ -999,6 +1034,8 @@ CHIP_ERROR ReadClient::SendSubscribeRequestImpl(const ReadPrepareParams & aReadP
         mReadPrepareParams.mSessionHolder = aReadPrepareParams.mSessionHolder;
     }
 
+    mIsPeerLIT = aReadPrepareParams.mIsPeerLIT;
+
     mMinIntervalFloorSeconds = aReadPrepareParams.mMinIntervalFloorSeconds;
 
     // Todo: Remove the below, Update span in ReadPrepareParams
@@ -1103,6 +1140,12 @@ CHIP_ERROR ReadClient::SendSubscribeRequestImpl(const ReadPrepareParams & aReadP
 
 CHIP_ERROR ReadClient::DefaultResubscribePolicy(CHIP_ERROR aTerminationCause)
 {
+    if (aTerminationCause == CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT)
+    {
+        ChipLogProgress(DataManagement, "ICD device is inactive, skipping scheduling resubscribe within DefaultResubscribePolicy");
+        return CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT;
+    }
+
     VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE);
 
     auto timeTillNextResubscription = ComputeTimeTillNextSubscription();
@@ -1111,8 +1154,7 @@ CHIP_ERROR ReadClient::DefaultResubscribePolicy(CHIP_ERROR aTerminationCause)
                     "ms due to error %" CHIP_ERROR_FORMAT,
                     GetFabricIndex(), ChipLogValueX64(GetPeerNodeId()), mNumRetries, timeTillNextResubscription,
                     aTerminationCause.Format());
-    ReturnErrorOnFailure(ScheduleResubscription(timeTillNextResubscription, NullOptional, aTerminationCause == CHIP_ERROR_TIMEOUT));
-    return CHIP_NO_ERROR;
+    return ScheduleResubscription(timeTillNextResubscription, NullOptional, aTerminationCause == CHIP_ERROR_TIMEOUT);
 }
 
 void ReadClient::HandleDeviceConnected(void * context, Messaging::ExchangeManager & exchangeMgr,

diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h
@@ -165,6 +165,12 @@ class ReadClient : public Messaging::ExchangeDelegate
          * ReadClient::DefaultResubscribePolicy is broken down into its constituent methods that are publicly available for
          * applications to call and sequence.
          *
+         * If the peer is LIT ICD, and the timeout is reached, `aTerminationCause` will be
+         * `CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT`. In this case, returning `CHIP_NO_ERROR` will still trigger a resubscribe
+         * attempt, while returning `CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT` will put the subscription in the
+         * `InactiveICDSubscription` state.  In the latter case, OnResubscriptionNeeded will be called again when
+         * `OnActiveModeNotification` is called.
+         *
          * If the method is over-ridden, it's the application's responsibility to take the appropriate steps needed to eventually
          * call-back into the ReadClient object to schedule a re-subscription (by invoking ReadClient::ScheduleResubscription).
          *
@@ -332,6 +338,18 @@ class ReadClient : public Messaging::ExchangeDelegate
      */
     CHIP_ERROR SendRequest(ReadPrepareParams & aReadPrepareParams);
 
+    /**
+     *  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().
+     *
+     *  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.
+     */
+    void OnActiveModeNotification();
+
     void OnUnsolicitedReportData(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload);
 
     void OnUnsolicitedMessageFromPublisher()
@@ -486,6 +504,7 @@ class ReadClient : public Messaging::ExchangeDelegate
         AwaitingInitialReport,     ///< The client is waiting for initial report
         AwaitingSubscribeResponse, ///< The client is waiting for subscribe response
         SubscriptionActive,        ///< The client is maintaining subscription
+        InactiveICDSubscription,   ///< The client is waiting to resubscribe for LIT device
     };
 
     enum class ReportType
@@ -510,6 +529,7 @@ class ReadClient : public Messaging::ExchangeDelegate
      *
      */
     bool IsIdle() const { return mState == ClientState::Idle; }
+    bool IsInactiveICDSubscription() const { return mState == ClientState::InactiveICDSubscription; }
     bool IsSubscriptionActive() const { return mState == ClientState::SubscriptionActive; }
     bool IsAwaitingInitialReport() const { return mState == ClientState::AwaitingInitialReport; }
     bool IsAwaitingSubscribeResponse() const { return mState == ClientState::AwaitingSubscribeResponse; }
@@ -533,6 +553,7 @@ class ReadClient : public Messaging::ExchangeDelegate
     CHIP_ERROR ComputeLivenessCheckTimerTimeout(System::Clock::Timeout * aTimeout);
     void CancelLivenessCheckTimer();
     void CancelResubscribeTimer();
+    void TriggerResubscriptionForLivenessTimeout(CHIP_ERROR aReason);
     void MoveToState(const ClientState aTargetState);
     CHIP_ERROR ProcessAttributePath(AttributePathIB::Parser & aAttributePath, ConcreteDataAttributePath & aClusterInfo);
     CHIP_ERROR ProcessReportData(System::PacketBufferHandle && aPayload, ReportType aReportType);
@@ -621,6 +642,8 @@ class ReadClient : public Messaging::ExchangeDelegate
 
     System::Clock::Timeout mLivenessTimeoutOverride = System::Clock::kZero;
 
+    bool mIsPeerLIT = false;
+
     // End Of Container (0x18) uses one byte.
     static constexpr uint16_t kReservedSizeForEndOfContainer = 1;
     // Reserved size for the uint8_t InteractionModelRevision flag, which takes up 1 byte for the control tag and 1 byte for the

diff --git a/src/app/ReadPrepareParams.h b/src/app/ReadPrepareParams.h
@@ -47,6 +47,7 @@ struct ReadPrepareParams
     uint16_t mMaxIntervalCeilingSeconds = 0;
     bool mKeepSubscriptions             = false;
     bool mIsFabricFiltered              = true;
+    bool mIsPeerLIT                     = false;
 
     ReadPrepareParams() {}
     ReadPrepareParams(const SessionHandle & sessionHandle) { mSessionHolder.Grab(sessionHandle); }
@@ -64,6 +65,7 @@ struct ReadPrepareParams
         mMaxIntervalCeilingSeconds         = other.mMaxIntervalCeilingSeconds;
         mTimeout                           = other.mTimeout;
         mIsFabricFiltered                  = other.mIsFabricFiltered;
+        mIsPeerLIT                         = other.mIsPeerLIT;
         other.mpEventPathParamsList        = nullptr;
         other.mEventPathParamsListSize     = 0;
         other.mpAttributePathParamsList    = nullptr;
@@ -88,6 +90,7 @@ struct ReadPrepareParams
         mMaxIntervalCeilingSeconds         = other.mMaxIntervalCeilingSeconds;
         mTimeout                           = other.mTimeout;
         mIsFabricFiltered                  = other.mIsFabricFiltered;
+        mIsPeerLIT                         = other.mIsPeerLIT;
         other.mpEventPathParamsList        = nullptr;
         other.mEventPathParamsListSize     = 0;
         other.mpAttributePathParamsList    = nullptr;

diff --git a/src/app/icd/client/DefaultCheckInDelegate.cpp b/src/app/icd/client/DefaultCheckInDelegate.cpp
@@ -16,6 +16,7 @@
  */
 
 #include "CheckInHandler.h"
+#include <app/InteractionModelEngine.h>
 #include <app/icd/client/DefaultCheckInDelegate.h>
 #include <crypto/CHIPCryptoPAL.h>
 #include <lib/support/CodeUtils.h>
@@ -38,6 +39,9 @@ void DefaultCheckInDelegate::OnCheckInComplete(const ICDClientInfo & clientInfo)
     ChipLogProgress(
         ICD, "Check In Message processing complete: start_counter=%" PRIu32 " offset=%" PRIu32 " nodeid=" ChipLogFormatScopedNodeId,
         clientInfo.start_icd_counter, clientInfo.offset, ChipLogValueScopedNodeId(clientInfo.peer_node));
+#if CHIP_CONFIG_ENABLE_READ_CLIENT
+    InteractionModelEngine::GetInstance()->OnActiveModeNotification(clientInfo.peer_node);
+#endif
 }
 
 } // namespace app

diff --git a/src/controller/tests/data_model/BUILD.gn b/src/controller/tests/data_model/BUILD.gn
@@ -25,8 +25,8 @@ chip_test_suite_using_nltest("data_model") {
   if (chip_device_platform != "mbed" && chip_device_platform != "efr32" &&
       chip_device_platform != "esp32" && chip_device_platform != "fake") {
     test_sources = [ "TestCommands.cpp" ]
-    test_sources += [ "TestRead.cpp" ]
     test_sources += [ "TestWrite.cpp" ]
+    test_sources += [ "TestRead.cpp" ]
   }
 
   public_deps = [