Skip to content

Commit

Permalink
Fix subscription liveness timeout computation to include MRP backoff.
Browse files Browse the repository at this point in the history
Right now the amount of time we allow for receiving a subscription report, after
the max-interval has elapsed, is computed by just multiplying our active
liveness timeout by the number of MRP retries.  For a typical always-active
client (hence 300ms active interval) that means 1500ms.

But an actual sender would do backoff in between the MRP messages, so would not
actually give up within 1500ms.  We should be taking that backoff into account
when computing the time we allow to receive the message.
  • Loading branch information
bzbarsky-apple committed Mar 9, 2023
1 parent 536a0d9 commit ad7dc23
Showing 1 changed file with 8 additions and 1 deletion.
9 changes: 8 additions & 1 deletion src/app/ReadClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include <lib/core/TLVTypes.h>
#include <lib/support/FibonacciUtils.h>
#include <messaging/ReliableMessageMgr.h>
#include <messaging/ReliableMessageProtocolConfig.h>

namespace chip {
namespace app {
Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit ad7dc23

Please sign in to comment.