Change Thread OTA throttling algorithm in Matter.framework. (#37562)

Throttle by rounding up to the nearest multiple of our throttle interval, not by
just comparing to that interval.  This matches the old "poll at 50ms" behavior
we had.

Author: bzbarsky-apple

src/darwin/Framework/CHIP/MTROTAImageTransferHandler.h

@@ -77,7 +77,7 @@ class MTROTAImageTransferHandler : public chip::bdx::AsyncResponder {
 
     bool mIsPeerNodeAKnownThreadDevice = NO;
 
-    chip::System::Clock::Milliseconds32 mBDXThrottleIntervalForThreadDevicesInMSecs;
+    chip::System::Clock::Milliseconds32 mBDXThrottleIntervalForThreadDevices;
 };
 
 NS_ASSUME_NONNULL_END

src/darwin/Framework/CHIP/MTROTAImageTransferHandler.mm

@@ -40,7 +40,7 @@
 // To override the throttle interval,
 // use ` defaults write <domain> BDXThrottleIntervalForThreadDevicesInMSecs <throttleIntervalinMsecs>`
 // See UserDefaults.mm for details.
-constexpr auto kBdxThrottleDefaultIntervalInMsecs = System::Clock::Milliseconds32(50);
+constexpr auto kBdxThrottleDefaultInterval = System::Clock::Milliseconds32(50);
 
 constexpr bdx::TransferRole kBdxRole = bdx::TransferRole::kSender;
 
@@ -64,7 +64,7 @@ - (instancetype)initWithMTROTAImageTransferHandler:(MTROTAImageTransferHandler *
 }
 @end
 
-MTROTAImageTransferHandler::MTROTAImageTransferHandler(chip::System::Layer * layer)
+MTROTAImageTransferHandler::MTROTAImageTransferHandler(System::Layer * layer)
 {
     assertChipStackLockedByCurrentThread();
 
@@ -91,8 +91,7 @@ - (instancetype)initWithMTROTAImageTransferHandler:(MTROTAImageTransferHandler *
 
     mIsPeerNodeAKnownThreadDevice = [controller definitelyUsesThreadForDevice:mPeer.GetNodeId()];
 
-    uint16_t throttleInterval = chip::Platform::GetUserDefaultBDXThrottleIntervalForThreadInMSecs().value_or(kBdxThrottleDefaultIntervalInMsecs.count());
-    mBDXThrottleIntervalForThreadDevicesInMSecs = System::Clock::Milliseconds32(throttleInterval);
+    mBDXThrottleIntervalForThreadDevices = Platform::GetUserDefaultBDXThrottleIntervalForThread().value_or(kBdxThrottleDefaultInterval);
 
     BitFlags<bdx::TransferControlFlags> flags(bdx::TransferControlFlags::kReceiverDrive);
 
@@ -249,9 +248,8 @@ - (instancetype)initWithMTROTAImageTransferHandler:(MTROTAImageTransferHandler *
 {
     assertChipStackLockedByCurrentThread();
 
-    // For thread devices, we need to throttle sending the response to BlockQuery, if the query is processed, before kBdxThrottleDefaultIntervalInMsecs
-    // has elapsed to prevent the BDX messages spamming up the network. Get the timestamp at which we start processing the BlockQuery message.
-
+    // For thread devices, we need to throttle sending the response to
+    // BlockQuery, so need to track when we started handling BlockQuery.
     auto startBlockQueryHandlingTimestamp = System::SystemClock().GetMonotonicTimestamp();
 
     auto blockSize = @(mTransfer.GetTransferBlockSize());
@@ -306,19 +304,20 @@ - (instancetype)initWithMTROTAImageTransferHandler:(MTROTAImageTransferHandler *
 
     void (^completionHandler)(NSData * _Nullable data, BOOL isEOF) = nil;
 
-    // If the peer node is a Thread device, check how much time has elapsed since we started processing the BlockQuery.
-    // If the time elapsed is greater than kBdxThrottleDefaultIntervalInMsecs, call the completion handler to respond with a Block right away.
-    // If time elapsed is less than kBdxThrottleDefaultIntervalInMsecs, dispatch the completion handler to respond with a Block after kBdxThrottleDefaultIntervalInMsecs has elapsed.
-
+    // If the peer node is a Thread device, check how much time has elapsed since we started processing the BlockQuery,
+    // round it up to the nearest multiple of kBdxThrottleDefaultInterval, and respond with the block at that point.
     if (mIsPeerNodeAKnownThreadDevice) {
         completionHandler = ^(NSData * _Nullable data, BOOL isEOF) {
             auto timeElapsed = System::SystemClock().GetMonotonicTimestamp() - startBlockQueryHandlingTimestamp;
+            // Integer division rounds down, so dividing by mBDXThrottleIntervalForThreadDevices and then multiplying
+            // by it again rounds down to the nearest multiple of mBDXThrottleIntervalForThreadDevices.
+            auto remainder = timeElapsed - (timeElapsed / mBDXThrottleIntervalForThreadDevices) * mBDXThrottleIntervalForThreadDevices;
 
-            if (timeElapsed >= mBDXThrottleIntervalForThreadDevicesInMSecs) {
+            if (remainder == System::Clock::Milliseconds32(0)) {
                 respondWithBlock(data, isEOF);
             } else {
-                auto timeRemaining = std::chrono::duration_cast<std::chrono::nanoseconds>(mBDXThrottleIntervalForThreadDevicesInMSecs - timeElapsed);
-                dispatch_time_t time = dispatch_time(DISPATCH_TIME_NOW, (int64_t) (timeRemaining.count()));
+                auto nsRemaining = std::chrono::duration_cast<std::chrono::nanoseconds>(remainder);
+                dispatch_time_t time = dispatch_time(DISPATCH_TIME_NOW, nsRemaining.count());
                 dispatch_after(time, delegateQueue, ^{
                     respondWithBlock(data, isEOF);
                 });

src/platform/Darwin/UserDefaults.h

@@ -18,13 +18,14 @@
 
 #include <cstdint>
 #include <optional>
+#include <system/SystemClock.h>
 
 namespace chip {
 namespace Platform {
 
 std::optional<uint16_t> GetUserDefaultDnssdSRPTimeoutInMSecs();
 
-std::optional<uint16_t> GetUserDefaultBDXThrottleIntervalForThreadInMSecs();
+std::optional<System::Clock::Milliseconds16> GetUserDefaultBDXThrottleIntervalForThread();
 
 } // namespace Platform
 } // namespace chip

src/platform/Darwin/UserDefaults.mm

@@ -40,15 +40,15 @@
         return std::nullopt;
     }
 
-    std::optional<uint16_t> GetUserDefaultBDXThrottleIntervalForThreadInMSecs()
+    std::optional<System::Clock::Milliseconds16> GetUserDefaultBDXThrottleIntervalForThread()
     {
         NSUserDefaults * defaults = [NSUserDefaults standardUserDefaults];
         NSInteger bdxThrottleIntervalInMsecs = [defaults integerForKey:kBDXThrottleIntervalInMsecsUserDefaultKey];
 
         if (bdxThrottleIntervalInMsecs > 0 && CanCastTo<uint16_t>(bdxThrottleIntervalInMsecs)) {
             uint16_t intervalInMsecs = static_cast<uint16_t>(bdxThrottleIntervalInMsecs);
             ChipLogProgress(BDX, "Got a user default value for BDX Throttle Interval for Thread devices - %d msecs", intervalInMsecs);
-            return std::make_optional(intervalInMsecs);
+            return std::make_optional(System::Clock::Milliseconds16(intervalInMsecs));
         }
 
         // For now return NullOptional if value returned in bdxThrottleIntervalInMsecs is 0, since that either means the key was not found or value was zero.