From ab70e461c0c53a7788aea0a43759c2691887d329 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 1 Nov 2022 15:27:11 -0400 Subject: [PATCH 1/2] Select maxInterval based on device sleep interval in MTRDevice. This lets us avoid hardcoding a value which might not be appropriate to all devices. --- src/darwin/Framework/CHIP/MTRDevice.mm | 20 +++++++-- .../Framework/CHIPTests/MTRDeviceTests.m | 44 +++++++++++++++++++ 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 694b779c13956a..a5fc0ab40ac95f 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)delegate queue:(dispatch_queue_t)queue { @@ -182,7 +183,7 @@ - (void)setDelegate:(id)delegate queue:(dispatch_queue_t)queu _weakDelegate = [MTRWeakReference weakReferenceWithObject:delegate]; _delegateQueue = queue; - [self subscribeWithMinInterval:0 maxInterval:MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_DEFAULT]; + [self subscribeWithMinInterval:0]; os_unfair_lock_unlock(&self->_lock); } @@ -285,7 +286,7 @@ - (void)_handleEventReport:(NSArray *> *)eventRepor os_unfair_lock_unlock(&self->_lock); } -- (void)subscribeWithMinInterval:(uint16_t)minInterval maxInterval:(uint16_t)maxInterval +- (void)subscribeWithMinInterval:(uint16_t)minInterval { // 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; + // Select a max interval based on the device's claimed idle sleep interval. + auto idleSleepInterval = std::chrono::duration_cast( + 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 = 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 +@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 *> *)attributeReport +{ +} + +- (void)device:(MTRDevice *)device receivedEventReport:(NSArray *> *)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 From a999e491a188918825430c2ced848a67089afde6 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 1 Nov 2022 18:07:21 -0400 Subject: [PATCH 2/2] Address review comment. --- src/darwin/Framework/CHIP/MTRDevice.mm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index a5fc0ab40ac95f..51ec813ab53d1d 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -183,7 +183,7 @@ - (void)setDelegate:(id)delegate queue:(dispatch_queue_t)queu _weakDelegate = [MTRWeakReference weakReferenceWithObject:delegate]; _delegateQueue = queue; - [self subscribeWithMinInterval:0]; + [self setupSubscription]; os_unfair_lock_unlock(&self->_lock); } @@ -286,7 +286,7 @@ - (void)_handleEventReport:(NSArray *> *)eventRepor os_unfair_lock_unlock(&self->_lock); } -- (void)subscribeWithMinInterval:(uint16_t)minInterval +- (void)setupSubscription { // for now just subscribe once if (_subscriptionActive) { @@ -312,7 +312,7 @@ - (void)subscribeWithMinInterval:(uint16_t)minInterval eventPath->mIsUrgentEvent = true; ReadPrepareParams readParams(session.Value()); - readParams.mMinIntervalFloorSeconds = minInterval; + readParams.mMinIntervalFloorSeconds = 0; // Select a max interval based on the device's claimed idle sleep interval. auto idleSleepInterval = std::chrono::duration_cast( session.Value()->GetRemoteMRPConfig().mIdleRetransTimeout); @@ -322,7 +322,7 @@ - (void)subscribeWithMinInterval:(uint16_t)minInterval if (idleSleepInterval.count() > MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX) { idleSleepInterval = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX); } - readParams.mMaxIntervalCeilingSeconds = idleSleepInterval.count(); + readParams.mMaxIntervalCeilingSeconds = static_cast(idleSleepInterval.count()); readParams.mpAttributePathParamsList = attributePath.get(); readParams.mAttributePathParamsListSize = 1;