The Darwin framework is clamping minimum intervals to 1 second for re…

…ports, we should up this (#29852) * Updating intervals * Update unit tests to still use a 2-second MaxIntervalCeiling. * Lowering this for now * Restyled by clang-format --------- Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> Co-authored-by: Restyled.io <commits@restyled.io>
project-chip · Nov 14, 2023 · a05d0d4 · a05d0d4
1 parent 420df60
commit a05d0d4
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 6 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm
@@ -196,6 +196,7 @@ @protocol MTRDeviceUnitTestDelegate <MTRDeviceDelegate>
 - (void)unitTestReportEndForDevice:(MTRDevice *)device;
 - (BOOL)unitTestShouldSetUpSubscriptionForDevice:(MTRDevice *)device;
 - (BOOL)unitTestShouldSkipExpectedValuesForWrite:(MTRDevice *)device;
+- (NSNumber *)unitTestMaxIntervalOverrideForSubscription:(MTRDevice *)device;
 @end
 #endif
 
@@ -233,8 +234,8 @@ + (MTRDevice *)deviceWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControll
 #pragma mark Subscription and delegate handling
 
 // subscription intervals are in seconds
-#define MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MIN (2)
-#define MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX (60)
+#define MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MIN (1 * 60) // 1 minute (for now)
+#define MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX (60 * 60) // 60 minutes
 
 - (void)setDelegate:(id<MTRDeviceDelegate>)delegate queue:(dispatch_queue_t)queue
 {
@@ -590,6 +591,17 @@ - (void)_handleEventReport:(NSArray<NSDictionary<NSString *, id> *> *)eventRepor
 // assume lock is held
 - (void)_setupSubscription
 {
+#ifdef DEBUG
+    id delegate = _weakDelegate.strongObject;
+    Optional<System::Clock::Seconds32> maxIntervalOverride;
+    if (delegate) {
+        if ([delegate respondsToSelector:@selector(unitTestMaxIntervalOverrideForSubscription:)]) {
+            NSNumber * delegateMin = [delegate unitTestMaxIntervalOverrideForSubscription:self];
+            maxIntervalOverride.Emplace(delegateMin.unsignedIntValue);
+        }
+    }
+#endif
+
     os_unfair_lock_assert_owner(&self->_lock);
 
     // for now just subscribe once
@@ -623,12 +635,21 @@ - (void)_setupSubscription
                    // Select a max interval based on the device's claimed idle sleep interval.
                    auto idleSleepInterval = std::chrono::duration_cast<System::Clock::Seconds32>(
                        session.Value()->GetRemoteMRPConfig().mIdleRetransTimeout);
-                   if (idleSleepInterval.count() < MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MIN) {
-                       idleSleepInterval = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MIN);
+
+                   auto maxIntervalCeilingMin = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MIN);
+                   if (idleSleepInterval < maxIntervalCeilingMin) {
+                       idleSleepInterval = maxIntervalCeilingMin;
+                   }
+
+                   auto maxIntervalCeilingMax = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX);
+                   if (idleSleepInterval > maxIntervalCeilingMax) {
+                       idleSleepInterval = maxIntervalCeilingMax;
                    }
-                   if (idleSleepInterval.count() > MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX) {
-                       idleSleepInterval = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX);
+#ifdef DEBUG
+                   if (maxIntervalOverride.HasValue()) {
+                       idleSleepInterval = maxIntervalOverride.Value();
                    }
+#endif
                    readParams.mMaxIntervalCeilingSeconds = static_cast<uint16_t>(idleSleepInterval.count());
 
                    readParams.mpAttributePathParamsList = attributePath.get();

diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
@@ -160,6 +160,12 @@ - (void)unitTestReportEndForDevice:(MTRDevice *)device
     }
 }
 
+- (NSNumber *)unitTestMaxIntervalOverrideForSubscription:(MTRDevice *)device
+{
+    // Make sure our subscriptions time out in finite time.
+    return @(2); // seconds
+}
+
 @end
 
 @interface MTRDeviceTests : XCTestCase

diff --git a/src/darwin/Framework/CHIPTests/MTRSwiftDeviceTests.swift b/src/darwin/Framework/CHIPTests/MTRSwiftDeviceTests.swift
@@ -88,6 +88,13 @@ class MTRSwiftDeviceTestDelegate : NSObject, MTRDeviceDelegate {
     {
         onReportEnd?()
     }
+
+    @objc func unitTestMaxIntervalOverrideForSubscription(_ device : MTRDevice) -> NSNumber
+    {
+      // Make sure our subscriptions time out in finite time.
+      return 2; // seconds
+    }
+
 }
 
 // Because we are using things from Matter.framework that are flagged