From 20260173c6366e0c93007e9755833d4c3dafdb25 Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Tue, 1 Nov 2022 20:57:35 -0400
Subject: [PATCH] Select maxInterval based on device sleep interval in
 MTRDevice. (#23426)

* Select maxInterval based on device sleep interval in MTRDevice.

This lets us avoid hardcoding a value which might not be appropriate to all devices.

* Address review comment.
---
 src/darwin/Framework/CHIP/MTRDevice.mm        | 22 +++++++---
 .../Framework/CHIPTests/MTRDeviceTests.m      | 44 +++++++++++++++++++
 2 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm
index 694b779c13956a..51ec813ab53d1d 100644
--- a/src/darwin/Framework/CHIP/MTRDevice.mm
+++ b/src/darwin/Framework/CHIP/MTRDevice.mm
@@ -174,7 +174,8 @@ + (instancetype)deviceWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControl
 #pragma mark Subscription and delegate handling
 
 // subscription intervals are in seconds
-#define MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_DEFAULT (3600)
+#define MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MIN (2)
+#define MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX (60)
 
 - (void)setDelegate:(id<MTRDeviceDelegate>)delegate queue:(dispatch_queue_t)queue
 {
@@ -182,7 +183,7 @@ - (void)setDelegate:(id<MTRDeviceDelegate>)delegate queue:(dispatch_queue_t)queu
 
     _weakDelegate = [MTRWeakReference weakReferenceWithObject:delegate];
     _delegateQueue = queue;
-    [self subscribeWithMinInterval:0 maxInterval:MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_DEFAULT];
+    [self setupSubscription];
 
     os_unfair_lock_unlock(&self->_lock);
 }
@@ -285,7 +286,7 @@ - (void)_handleEventReport:(NSArray<NSDictionary<NSString *, id> *> *)eventRepor
     os_unfair_lock_unlock(&self->_lock);
 }
 
-- (void)subscribeWithMinInterval:(uint16_t)minInterval maxInterval:(uint16_t)maxInterval
+- (void)setupSubscription
 {
     // for now just subscribe once
     if (_subscriptionActive) {
@@ -310,8 +311,19 @@ - (void)subscribeWithMinInterval:(uint16_t)minInterval maxInterval:(uint16_t)max
                            // We want to get event reports at the minInterval, not the maxInterval.
                            eventPath->mIsUrgentEvent = true;
                            ReadPrepareParams readParams(session.Value());
-                           readParams.mMinIntervalFloorSeconds = minInterval;
-                           readParams.mMaxIntervalCeilingSeconds = maxInterval;
+
+                           readParams.mMinIntervalFloorSeconds = 0;
+                           // 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);
+                           }
+                           if (idleSleepInterval.count() > MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX) {
+                               idleSleepInterval = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX);
+                           }
+                           readParams.mMaxIntervalCeilingSeconds = static_cast<uint16_t>(idleSleepInterval.count());
+
                            readParams.mpAttributePathParamsList = attributePath.get();
                            readParams.mAttributePathParamsListSize = 1;
                            readParams.mpEventPathParamsList = eventPath.get();
diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
index 7e2d6006fc4ffe..ba59a4cc0286cd 100644
--- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
+++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
@@ -122,6 +122,28 @@ - (void)onAddressUpdated:(NSError *)error
 }
 @end
 
+@interface MTRDeviceTestDelegate : NSObject <MTRDeviceDelegate>
+@property (nonatomic) dispatch_block_t onSubscriptionEstablished;
+@end
+
+@implementation MTRDeviceTestDelegate
+- (void)device:(MTRDevice *)device stateChanged:(MTRDeviceState)state
+{
+    if (state == MTRDeviceStateReachable) {
+        self.onSubscriptionEstablished();
+    }
+}
+
+- (void)device:(MTRDevice *)device receivedAttributeReport:(NSArray<NSDictionary<NSString *, id> *> *)attributeReport
+{
+}
+
+- (void)device:(MTRDevice *)device receivedEventReport:(NSArray<NSDictionary<NSString *, id> *> *)eventReport
+{
+}
+
+@end
+
 @interface MTRDeviceTests : XCTestCase
 @end
 
@@ -1359,6 +1381,28 @@ - (void)test016_FailedSubscribeWithCacheReadDuringFailure
     [self waitForExpectations:@[ errorExpectation ] timeout:60];
 }
 
+- (void)test017_TestMTRDeviceBasics
+{
+#if MANUAL_INDIVIDUAL_TEST
+    [self initStack];
+    [self waitForCommissionee];
+#endif
+
+    __auto_type * device = [MTRDevice deviceWithNodeID:kDeviceId deviceController:sController];
+    dispatch_queue_t queue = dispatch_get_main_queue();
+
+    XCTestExpectation * subscriptionExpectation = [self expectationWithDescription:@"Subscription has been set up"];
+
+    __auto_type * delegate = [[MTRDeviceTestDelegate alloc] init];
+    delegate.onSubscriptionEstablished = ^() {
+        [subscriptionExpectation fulfill];
+    };
+
+    [device setDelegate:delegate queue:queue];
+
+    [self waitForExpectations:@[ subscriptionExpectation ] timeout:60];
+}
+
 - (void)test900_SubscribeAllAttributes
 {
 #if MANUAL_INDIVIDUAL_TEST