diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 211cc256a9ef91..54dcc92fc0b65e 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -429,29 +429,13 @@ CHIP_ERROR ReadClient::GenerateDataVersionFilterList(DataVersionFilterIBs::Build return CHIP_NO_ERROR; } -CHIP_ERROR ReadClient::OnActiveModeNotification() +void ReadClient::OnActiveModeNotification() { - VerifyOrReturnError(IsSubscriptionType() && IsIdle(), CHIP_ERROR_INCORRECT_STATE); - VerifyOrReturnError(mPeerActivePeriod != System::Clock::kZero, CHIP_ERROR_INCORRECT_STATE); - // TODO: Add Better the error code to inform the ActiveModeWakeUp and trigger the subscription retry - Close(CHIP_ERROR_TIMEOUT); - return CHIP_NO_ERROR; -} - -void ReadClient::UpdateActivePeriod(System::Clock::Timeout aActivePeriod) -{ - mPeerActivePeriod = aActivePeriod; -} - -CHIP_ERROR ReadClient::TouchPeerActiveTime(System::Clock::Timeout aActivePeriod) -{ - System::Clock::Timestamp time; - - ReturnLogErrorOnFailure(System::SystemClock().GetClock_RealTimeMS(time)); - - mPeerActiveUntilTimestamp = time + aActivePeriod; - - return CHIP_NO_ERROR; + VerifyOrDie(mpImEngine->InActiveReadClientList(this)); + // If the subscription is not at `IdleSubscription` state, then the liveness timeout is not triggerred. + // Simplily do nothing. + VerifyOrReturn(IsIdleSubscription()); + TriggerResubscription(); } CHIP_ERROR ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, @@ -921,51 +905,47 @@ 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->mPeerActivePeriod != System::Clock::kZero) + if (_this->mIsPeerICD) { - // If device is sleeping, we mark the subscription as "IdleSubscription", and trigger resubscription on + // If device is idle, we mark the subscription as "IdleSubscription", and trigger resubscription on // `OnActiveModeNotification`. - System::Clock::Timestamp time; - CHIP_ERROR error = System::SystemClock().GetClock_RealTimeMS(time); - if (error != CHIP_NO_ERROR) - { - ChipLogError(DataManagement, "Failed to get current time: %s", ErrorStr(error)); - _this->Close(error, false); - } - if (time > _this->mPeerActiveUntilTimestamp) - { - ChipLogProgress(DataManagement, "Peer is not active now, mark the subscription as IdleSubscription."); - _this->MoveToState(ClientState::IdleSubscription); - return; - } + // Note: the liveness timeout is always longer than the MaxInterval (and idle duration), so we can move the + // device to `IdleSubcription` when liveness timeout reached. + ChipLogProgress(DataManagement, "Peer is not active now, mark the subscription as IdleSubscription."); + _this->MoveToState(ClientState::IdleSubscription); + return; } + _this->TriggerResubscription(); +} + +void ReadClient::TriggerResubscription() +{ // 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(CHIP_ERROR_TIMEOUT); } CHIP_ERROR ReadClient::ProcessSubscribeResponse(System::PacketBufferHandle && aPayload) @@ -1044,17 +1024,7 @@ CHIP_ERROR ReadClient::SendSubscribeRequestImpl(const ReadPrepareParams & aReadP mReadPrepareParams.mSessionHolder = aReadPrepareParams.mSessionHolder; } - mPeerActivePeriod = aReadPrepareParams.mActivePeriod; - if (mPeerActivePeriod != System::Clock::kZero) - { - System::Clock::Timestamp time; - ReturnErrorOnFailure(System::SystemClock().GetClock_RealTimeMS(time)); - mPeerActiveUntilTimestamp = mPeerActivePeriod; - } - else - { - mPeerActiveUntilTimestamp = System::Clock::kZero; - } + mIsPeerICD = aReadPrepareParams.mIsPeerICD; mMinIntervalFloorSeconds = aReadPrepareParams.mMinIntervalFloorSeconds; diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h index d763d5023e0be4..1bf167944d69ff 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -326,33 +326,15 @@ class ReadClient : public Messaging::ExchangeDelegate /** * Activate the idle subscription. * - * When subscribing to an ICD, the subscriber is expected to set the `mActivePeriod` - * in `ReadPrepareParams`. + * When subscriptiing to ICD and liveness timeout reached, the read client + * will move to `IdleSubscription` state and resubscription can be triggerred + * via OnActiveModeNotification(). * - * @retval #others fail to send read request - * @retval #CHIP_NO_ERROR Successfully waked up the subscription. - * @retval #CHIP_ERROR_INCORRECT_STATE The transcation is not an active subscription - */ - CHIP_ERROR OnActiveModeNotification(); - - /** - * Update the active period of the device. This method **will not** update the - * timestamp used to determine whether the device is idle. - * - * A Zero period means the device will always be active. + * If the subscription is not in `IdleSubscription` state, this function will + * do nothing. So it is always safe to call this function when a check-in message + * is received. */ - void UpdateActivePeriod(System::Clock::Timeout aActivePeriod); - - /** - * Update the time used to determine whether the device is idle to the period from now. - * This method **will not** update the active period for future wake-ups. - * - * A zero period means the device will become idle immediately. - * - * @retval #CHIP_NO_ERROR Successfully updated the active time. - * @retval #others failed to update the active time. - */ - CHIP_ERROR TouchPeerActiveTime(System::Clock::Timeout aActivePeriod); + void OnActiveModeNotification(); void OnUnsolicitedReportData(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload); @@ -532,7 +514,8 @@ class ReadClient : public Messaging::ExchangeDelegate * Check if current read client is being used * */ - bool IsIdle() const { return mState == ClientState::Idle || mState == ClientState::IdleSubscription; } + bool IsIdle() const { return mState == ClientState::Idle; } + bool IsIdleSubscription() const { return mState == ClientState::IdleSubscription; } bool IsSubscriptionActive() const { return mState == ClientState::SubscriptionActive; } bool IsAwaitingInitialReport() const { return mState == ClientState::AwaitingInitialReport; } bool IsAwaitingSubscribeResponse() const { return mState == ClientState::AwaitingSubscribeResponse; } @@ -550,6 +533,8 @@ class ReadClient : public Messaging::ExchangeDelegate CHIP_ERROR ProcessAttributeReportIBs(TLV::TLVReader & aAttributeDataIBsReader); CHIP_ERROR ProcessEventReportIBs(TLV::TLVReader & aEventReportIBsReader); + void TriggerResubscription(); + static void OnLivenessTimeoutCallback(System::Layer * apSystemLayer, void * apAppState); CHIP_ERROR ProcessSubscribeResponse(System::PacketBufferHandle && aPayload); CHIP_ERROR RefreshLivenessCheckTimer(); @@ -644,11 +629,7 @@ class ReadClient : public Messaging::ExchangeDelegate System::Clock::Timeout mLivenessTimeoutOverride = System::Clock::kZero; - // mPeerActiveUntilTimestamp represents the active time for a subscription for an ICD, when the liveness timeout is fired after - // the mPeerActiveUntilTimestamp, the ReadClient will enter "IdleSubscription" state - // until OnActiveModeNotification() is called. The Zero value means the device should always be active. - System::Clock::Timeout mPeerActivePeriod = System::Clock::kZero; - System::Clock::Timestamp mPeerActiveUntilTimestamp = System::Clock::kZero; + bool mIsPeerICD = false; // End Of Container (0x18) uses one byte. static constexpr uint16_t kReservedSizeForEndOfContainer = 1; diff --git a/src/app/ReadPrepareParams.h b/src/app/ReadPrepareParams.h index 0cc78fd843cd10..fbbb15cd440e42 100644 --- a/src/app/ReadPrepareParams.h +++ b/src/app/ReadPrepareParams.h @@ -47,8 +47,7 @@ struct ReadPrepareParams uint16_t mMaxIntervalCeilingSeconds = 0; bool mKeepSubscriptions = false; bool mIsFabricFiltered = true; - // The active period of the device, a Zero value means the device is always active. - System::Clock::Timeout mActivePeriod = System::Clock::kZero; + bool mIsPeerICD = false; ReadPrepareParams() {} ReadPrepareParams(const SessionHandle & sessionHandle) { mSessionHolder.Grab(sessionHandle); } @@ -66,7 +65,7 @@ struct ReadPrepareParams mMaxIntervalCeilingSeconds = other.mMaxIntervalCeilingSeconds; mTimeout = other.mTimeout; mIsFabricFiltered = other.mIsFabricFiltered; - mActivePeriod = other.mActivePeriod; + mIsPeerICD = other.mIsPeerICD; other.mpEventPathParamsList = nullptr; other.mEventPathParamsListSize = 0; other.mpAttributePathParamsList = nullptr; @@ -91,7 +90,7 @@ struct ReadPrepareParams mMaxIntervalCeilingSeconds = other.mMaxIntervalCeilingSeconds; mTimeout = other.mTimeout; mIsFabricFiltered = other.mIsFabricFiltered; - mActivePeriod = other.mActivePeriod; + mIsPeerICD = other.mIsPeerICD; other.mpEventPathParamsList = nullptr; other.mEventPathParamsListSize = 0; other.mpAttributePathParamsList = nullptr; diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 62cb3b9a59a42d..41d434bffdf117 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -2661,7 +2661,7 @@ void TestReadInteraction::TestSubscribeIdleWakeUp(nlTestSuite * apSuite, void * constexpr uint16_t maxIntervalCeilingSeconds = 1; readPrepareParams.mMaxIntervalCeilingSeconds = maxIntervalCeilingSeconds; - readPrepareParams.mActivePeriod = 999_ms; + readPrepareParams.mIsPeerICD = true; auto err = readClient.SendAutoResubscribeRequest(std::move(readPrepareParams)); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); @@ -2696,8 +2696,7 @@ void TestReadInteraction::TestSubscribeIdleWakeUp(nlTestSuite * apSuite, void * ctx.GetLoopback().mNumMessagesToDrop = 0; callback.ClearCounters(); - err = readClient.OnActiveModeNotification(); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + readClient.OnActiveModeNotification(); NL_TEST_ASSERT(apSuite, callback.mOnResubscriptionsAttempted == 1); NL_TEST_ASSERT(apSuite, callback.mLastError == CHIP_ERROR_TIMEOUT);