diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index cbd131f288c3b0..939c599704724d 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -31,6 +31,7 @@ #include #include #include +#include namespace chip { namespace app { @@ -808,11 +809,17 @@ CHIP_ERROR ReadClient::RefreshLivenessCheckTimer() // computing the Ack timeout from the publisher for the ReportData message being sent to us using our IDLE interval as the // basis for that computation. // + // Make sure to use the retransmission computation that includes backoff. For purposes of that computation, treat us as + // active now (since we are right now sending/receiving messages), and use the default "how long are we guaranteed to stay + // active" threshold for now. + // // TODO: We need to find a good home for this logic that will correctly compute this based on transport. For now, this will // suffice since we don't use TCP as a transport currently and subscriptions over BLE aren't really a thing. // + const auto & ourMrpConfig = GetDefaultMRPConfig(); auto publisherTransmissionTimeout = - GetLocalMRPConfig().ValueOr(GetDefaultMRPConfig()).mIdleRetransTimeout * (CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS + 1); + GetRetransmissionTimeout(ourMrpConfig.mActiveRetransTimeout, ourMrpConfig.mIdleRetransTimeout, + System::SystemClock().GetMonotonicTimestamp(), Transport::kMinActiveTime); timeout = System::Clock::Seconds16(mMaxInterval) + publisherTransmissionTimeout; } diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 1bfbac35ef93b4..c9fb756200e273 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -317,6 +317,10 @@ class TestReadInteraction : public app::ReadHandler::ApplicationCallback // of reads in parallel and wait for them all to succeed. static void SubscribeThenReadHelper(nlTestSuite * apSuite, TestContext & aCtx, size_t aSubscribeCount, size_t aReadCount); + // Compute the amount of time it would take a subscription with a given + // max-interval to time out. + static System::Clock::Timeout ComputeSubscriptionTimeout(System::Clock::Seconds16 aMaxInterval); + bool mEmitSubscriptionError = false; int32_t mNumActiveSubscriptions = 0; bool mAlterSubscriptionIntervals = false; @@ -1684,7 +1688,9 @@ void TestReadInteraction::TestResubscribeAttributeTimeout(nlTestSuite * apSuite, attributePathParams[0].mClusterId = app::Clusters::UnitTesting::Id; attributePathParams[0].mAttributeId = app::Clusters::UnitTesting::Attributes::Boolean::Id; - readPrepareParams.mMaxIntervalCeilingSeconds = 1; + constexpr uint16_t maxIntervalCeilingSeconds = 1; + + readPrepareParams.mMaxIntervalCeilingSeconds = maxIntervalCeilingSeconds; auto err = readClient.SendAutoResubscribeRequest(std::move(readPrepareParams)); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); @@ -1701,10 +1707,9 @@ void TestReadInteraction::TestResubscribeAttributeTimeout(nlTestSuite * apSuite, // // Disable packet transmission, and drive IO till we have reported a re-subscription attempt. // - // 1.5s should cover the liveness timeout in the client of 1s max interval + 50ms ACK timeout. // ctx.GetLoopback().mNumMessagesToDrop = chip::Test::LoopbackTransport::kUnlimitedMessageCount; - ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(1500), + ctx.GetIOContext().DriveIOUntil(ComputeSubscriptionTimeout(System::Clock::Seconds16(maxIntervalCeilingSeconds)), [&]() { return callback.mOnResubscriptionsAttempted > 0; }); NL_TEST_ASSERT(apSuite, callback.mOnResubscriptionsAttempted == 1); @@ -1763,7 +1768,8 @@ void TestReadInteraction::TestSubscribeAttributeTimeout(nlTestSuite * apSuite, v // // Request a max interval that's very small to reduce time to discovering a liveness failure. // - readPrepareParams.mMaxIntervalCeilingSeconds = 1; + constexpr uint16_t maxIntervalCeilingSeconds = 1; + readPrepareParams.mMaxIntervalCeilingSeconds = maxIntervalCeilingSeconds; auto err = readClient.SendRequest(readPrepareParams); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); @@ -1782,11 +1788,11 @@ void TestReadInteraction::TestSubscribeAttributeTimeout(nlTestSuite * apSuite, v // // Drive IO until we get an error on the subscription, which should be caused - // by the liveness timer firing within ~1s of the establishment of the subscription. + // by the liveness timer firing once we hit our max-interval plus + // retransmit timeouts. // - // 1.5s should cover the liveness timeout in the client of 1s max interval + 50ms ACK timeout. - // - ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(1500), [&]() { return callback.mOnError >= 1; }); + ctx.GetIOContext().DriveIOUntil(ComputeSubscriptionTimeout(System::Clock::Seconds16(maxIntervalCeilingSeconds)), + [&]() { return callback.mOnError >= 1; }); NL_TEST_ASSERT(apSuite, callback.mOnError == 1); NL_TEST_ASSERT(apSuite, callback.mLastError == CHIP_ERROR_TIMEOUT); @@ -4617,6 +4623,18 @@ void TestReadInteraction::TestReadHandler_KeepSubscriptionTest(nlTestSuite * apS ctx.DrainAndServiceIO(); } +System::Clock::Timeout TestReadInteraction::ComputeSubscriptionTimeout(System::Clock::Seconds16 aMaxInterval) +{ + // Add 50ms of slack to our max interval to make sure we hit the + // subscription liveness timer. + const auto & ourMrpConfig = GetDefaultMRPConfig(); + auto publisherTransmissionTimeout = + GetRetransmissionTimeout(ourMrpConfig.mActiveRetransTimeout, ourMrpConfig.mIdleRetransTimeout, + System::SystemClock().GetMonotonicTimestamp(), Transport::kMinActiveTime); + + return publisherTransmissionTimeout + aMaxInterval + System::Clock::Milliseconds32(50); +} + // clang-format off const nlTest sTests[] = {