From 12885602cffe265c811ca75ddd33466548a7001e Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Wed, 18 Oct 2023 22:12:53 -0700 Subject: [PATCH] The Darwin framework is clamping minimum intervals to 1 second for reports, 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 Co-authored-by: Restyled.io --- src/darwin/Framework/CHIP/MTRDevice.mm | 33 +++++++++++++++---- .../Framework/CHIPTests/MTRDeviceTests.m | 6 ++++ .../CHIPTests/MTRSwiftDeviceTests.swift | 7 ++++ 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index efcffb259a8b19..6201f002147a81 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -196,6 +196,7 @@ @protocol MTRDeviceUnitTestDelegate - (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)delegate queue:(dispatch_queue_t)queue { @@ -590,6 +591,17 @@ - (void)_handleEventReport:(NSArray *> *)eventRepor // assume lock is held - (void)_setupSubscription { +#ifdef DEBUG + id delegate = _weakDelegate.strongObject; + Optional 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( 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(idleSleepInterval.count()); readParams.mpAttributePathParamsList = attributePath.get(); diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 953056d02ddb1e..0e33264ef7e64e 100644 --- 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 index badcbabbca2fd2..5d963fb51176fd 100644 --- 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