Skip to content

Commit c3ef110

Browse files
Do not set up subscription when setting a delegate for a MTRDevice wi… (#33559)
* Do not set up subscription when setting a delegate for a MTRDevice with a remote controller over XPC - Add tests for testing the behavior when a MTRDevice is created with an XPC controller * Restyled by whitespace * Restyled by clang-format * Clean up * Restyled by clang-format * Add an API for checking if subscriptions are allowed and check that in _setUpSubscription * Restyled by clang-format --------- Co-authored-by: Restyled.io <commits@restyled.io>
1 parent 5ec28e2 commit c3ef110

File tree

3 files changed

+71
-1
lines changed

3 files changed

+71
-1
lines changed

src/darwin/Framework/CHIP/MTRDevice.mm

+20-1
Original file line numberDiff line numberDiff line change
@@ -685,11 +685,17 @@ - (void)_setDSTOffsets:(NSArray<MTRTimeSynchronizationClusterDSTOffsetStruct *>
685685
#define MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MIN (10 * 60) // 10 minutes (for now)
686686
#define MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX (60 * 60) // 60 minutes
687687

688+
- (BOOL)_subscriptionsAllowed
689+
{
690+
return ![_deviceController isKindOfClass:MTRDeviceControllerOverXPC.class];
691+
}
692+
688693
- (void)setDelegate:(id<MTRDeviceDelegate>)delegate queue:(dispatch_queue_t)queue
689694
{
690695
MTR_LOG("%@ setDelegate %@", self, delegate);
691696

692-
BOOL setUpSubscription = YES;
697+
// We should not set up a subscription for device controllers over XPC.
698+
BOOL setUpSubscription = [self _subscriptionsAllowed];
693699

694700
// For unit testing only
695701
#ifdef DEBUG
@@ -933,6 +939,14 @@ - (void)_changeInternalState:(MTRInternalDeviceState)state
933939
}
934940
}
935941

942+
#ifdef DEBUG
943+
- (MTRInternalDeviceState)_getInternalState
944+
{
945+
std::lock_guard lock(self->_lock);
946+
return _internalDeviceState;
947+
}
948+
#endif
949+
936950
// First Time Sync happens 2 minutes after reachability (this can be changed in the future)
937951
#define MTR_DEVICE_TIME_UPDATE_INITIAL_WAIT_TIME_SEC (60 * 2)
938952
- (void)_handleSubscriptionEstablished
@@ -1680,6 +1694,11 @@ - (void)_setupSubscription
16801694
{
16811695
os_unfair_lock_assert_owner(&self->_lock);
16821696

1697+
if (![self _subscriptionsAllowed]) {
1698+
MTR_LOG("_setupSubscription: Subscriptions not allowed. Do not set up subscription");
1699+
return;
1700+
}
1701+
16831702
#ifdef DEBUG
16841703
id delegate = _weakDelegate.strongObject;
16851704
Optional<System::Clock::Seconds32> maxIntervalOverride;

src/darwin/Framework/CHIPTests/MTRDeviceTests.m

+30
Original file line numberDiff line numberDiff line change
@@ -3618,6 +3618,36 @@ - (void)test034_TestMTRDeviceHistoricalEvents
36183618
XCTAssertTrue(eventReportsReceived > 0);
36193619
}
36203620

3621+
- (void)test035_TestMTRDeviceSubscriptionNotEstablishedOverXPC
3622+
{
3623+
NSString * const MTRDeviceControllerId = @"MTRController";
3624+
__auto_type remoteController = [MTRDeviceController
3625+
sharedControllerWithID:MTRDeviceControllerId
3626+
xpcConnectBlock:^NSXPCConnection * _Nonnull {
3627+
return nil;
3628+
}];
3629+
3630+
__auto_type * device = [MTRDevice deviceWithNodeID:kDeviceId deviceController:remoteController];
3631+
dispatch_queue_t queue = dispatch_get_main_queue();
3632+
3633+
// We should not set up a subscription when creating a MTRDevice with a remote controller.
3634+
XCTestExpectation * subscriptionExpectation = [self expectationWithDescription:@"Subscription has been set up"];
3635+
subscriptionExpectation.inverted = YES;
3636+
3637+
__auto_type * delegate = [[MTRDeviceTestDelegate alloc] init];
3638+
3639+
XCTAssertTrue([device _getInternalState] == MTRInternalDeviceStateUnsubscribed);
3640+
3641+
delegate.onAttributeDataReceived = ^(NSArray<NSDictionary<NSString *, id> *> * attributeReport) {
3642+
[subscriptionExpectation fulfill];
3643+
};
3644+
3645+
[device setDelegate:delegate queue:queue];
3646+
[self waitForExpectations:@[ subscriptionExpectation ] timeout:30];
3647+
3648+
XCTAssertTrue([device _getInternalState] == MTRInternalDeviceStateUnsubscribed);
3649+
}
3650+
36213651
@end
36223652

36233653
@interface MTRDeviceEncoderTests : XCTestCase

src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h

+21
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,31 @@ NS_ASSUME_NONNULL_BEGIN
5959
@end
6060

6161
@interface MTRDevice (TestDebug)
62+
typedef NS_ENUM(NSUInteger, MTRInternalDeviceState) {
63+
// Unsubscribed means we do not have a subscription and are not trying to set one up.
64+
MTRInternalDeviceStateUnsubscribed = 0,
65+
// Subscribing means we are actively trying to establish our initial subscription (e.g. doing
66+
// DNS-SD discovery, trying to establish CASE to the peer, getting priming reports, etc).
67+
MTRInternalDeviceStateSubscribing = 1,
68+
// InitialSubscriptionEstablished means we have at some point finished setting up a
69+
// subscription. That subscription may have dropped since then, but if so it's the ReadClient's
70+
// responsibility to re-establish it.
71+
MTRInternalDeviceStateInitialSubscriptionEstablished = 2,
72+
// Resubscribing means we had established a subscription, but then
73+
// detected a subscription drop due to not receiving a report on time. This
74+
// covers all the actions that happen when re-subscribing (discovery, CASE,
75+
// getting priming reports, etc).
76+
MTRInternalDeviceStateResubscribing = 3,
77+
// LaterSubscriptionEstablished meant that we had a subscription drop and
78+
// then re-created a subscription.
79+
MTRInternalDeviceStateLaterSubscriptionEstablished = 4,
80+
};
81+
6282
- (void)unitTestInjectEventReport:(NSArray<NSDictionary<NSString *, id> *> *)eventReport;
6383
- (void)unitTestInjectAttributeReport:(NSArray<NSDictionary<NSString *, id> *> *)attributeReport fromSubscription:(BOOL)isFromSubscription;
6484
- (NSUInteger)unitTestAttributesReportedSinceLastCheck;
6585
- (void)unitTestClearClusterData;
86+
- (MTRInternalDeviceState)_getInternalState;
6687
@end
6788
#endif
6889

0 commit comments

Comments
 (0)