diff --git a/src/darwin/Framework/CHIP/MTRDefines_Internal.h b/src/darwin/Framework/CHIP/MTRDefines_Internal.h index e22f00dd4e6a0b..94a1cbb3da61f4 100644 --- a/src/darwin/Framework/CHIP/MTRDefines_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDefines_Internal.h @@ -150,3 +150,7 @@ typedef struct {} variable_hidden_by_mtr_hide; } \ } #endif + +#ifndef YES_NO +#define YES_NO(x) ((x) ? @"YES" : @"NO") +#endif diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.h b/src/darwin/Framework/CHIP/MTRDeviceController.h index ad025c87d64043..d731b13052798c 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController.h @@ -186,7 +186,7 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1)) - (void)preWarmCommissioningSession MTR_DEPRECATED("-[MTRDeviceControllerFactory preWarmCommissioningSession]", ios(16.4, 17.6), macos(13.3, 14.6), watchos(9.4, 10.6), tvos(16.4, 17.6)); /** - * Set the Delegate for the device controller as well as the Queue on which the Delegate callbacks will be triggered + * Set the Delegate for the device controller as well as the Queue on which the Delegate callbacks will be triggered * * @param[in] delegate The delegate the commissioning process should use * @@ -195,6 +195,23 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1)) - (void)setDeviceControllerDelegate:(id)delegate queue:(dispatch_queue_t)queue MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4)); +/** + * Adds a Delegate to the device controller as well as the Queue on which the Delegate callbacks will be triggered + * + * @param[in] delegate The delegate the commissioning process should use + * + * @param[in] queue The queue on which the callbacks will be delivered + */ +- (void)addDeviceControllerDelegate:(id)delegate + queue:(dispatch_queue_t)queue MTR_NEWLY_AVAILABLE; + +/** + * Removes a Delegate from the device controller + * + * @param[in] delegate The delegate to be removed + */ +- (void)removeDeviceControllerDelegate:(id)delegate MTR_NEWLY_AVAILABLE; + /** * Start scanning for commissionable devices. * diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 021ada1852cf9c..9dc9f5a147db81 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -112,6 +112,27 @@ using namespace chip::Tracing::DarwinFramework; +@interface MTRDeviceControllerDelegateInfo : NSObject +- (instancetype)initWithDelegate:(id)delegate queue:(dispatch_queue_t)queue; +@property (nonatomic, weak, readonly) id delegate; +@property (nonatomic, readonly) dispatch_queue_t queue; +@end + +@implementation MTRDeviceControllerDelegateInfo +@synthesize delegate = _delegate, queue = _queue; +- (instancetype)initWithDelegate:(id)delegate queue:(dispatch_queue_t)queue +{ + if (!(self = [super init])) { + return nil; + } + + _delegate = delegate; + _queue = queue; + + return self; +} +@end + @implementation MTRDeviceController { chip::Controller::DeviceCommissioner * _cppCommissioner; chip::Credentials::PartialDACVerifier * _partialDACVerifier; @@ -139,6 +160,9 @@ @implementation MTRDeviceController { NSUInteger _keepRunningAssertionCounter; BOOL _shutdownPending; os_unfair_lock _assertionLock; + + NSMutableSet * _delegates; + id _strongDelegateForSetDelegateAPI; } @synthesize uniqueIdentifier = _uniqueIdentifier; @@ -168,6 +192,8 @@ - (instancetype)initForSubclasses:(BOOL)startSuspended _nodeIDToDeviceMap = [NSMapTable strongToWeakObjectsMapTable]; + _delegates = [NSMutableSet set]; + return self; } @@ -567,6 +593,10 @@ - (void)cleanup if (_deviceControllerDelegateBridge) { delete _deviceControllerDelegateBridge; _deviceControllerDelegateBridge = nullptr; + @synchronized(self) { + _strongDelegateForSetDelegateAPI = nil; + [_delegates removeAllObjects]; + } } } @@ -1243,15 +1273,6 @@ - (void)removeDevice:(MTRDevice *)device } #endif -- (void)setDeviceControllerDelegate:(id)delegate queue:(dispatch_queue_t)queue -{ - [self - asyncDispatchToMatterQueue:^() { - self->_deviceControllerDelegateBridge->setDelegate(self, delegate, queue); - } - errorHandler:nil]; -} - - (BOOL)setOperationalCertificateIssuer:(nullable id)operationalCertificateIssuer queue:(nullable dispatch_queue_t)queue { @@ -1751,6 +1772,158 @@ + (void)forceLocalhostAdvertisingOnly } #endif // DEBUG +#pragma mark - MTRDeviceControllerDelegate management + +// Note these are implemented in the base class so that XPC subclass can use it as well when it +- (void)setDeviceControllerDelegate:(id)delegate queue:(dispatch_queue_t)queue +{ + @synchronized(self) { + if (_strongDelegateForSetDelegateAPI) { + if (_strongDelegateForSetDelegateAPI == delegate) { + MTR_LOG("%@ setDeviceControllerDelegate: delegate %p is already set", self, delegate); + return; + } + + MTR_LOG("%@ setDeviceControllerDelegate: replacing %p with %p", self, _strongDelegateForSetDelegateAPI, delegate); + [self removeDeviceControllerDelegate:_strongDelegateForSetDelegateAPI]; + } + _strongDelegateForSetDelegateAPI = delegate; + [self addDeviceControllerDelegate:delegate queue:queue]; + } +} + +- (void)addDeviceControllerDelegate:(id)delegate queue:(dispatch_queue_t)queue +{ + @synchronized(self) { + MTRDeviceControllerDelegateInfo * newDelegateInfo = [[MTRDeviceControllerDelegateInfo alloc] initWithDelegate:delegate queue:queue]; + [_delegates addObject:newDelegateInfo]; + MTR_LOG("%@ addDeviceControllerDelegate: added %p total %lu", self, delegate, static_cast(_delegates.count)); + } +} + +- (void)removeDeviceControllerDelegate:(id)delegate +{ + @synchronized(self) { + if (_strongDelegateForSetDelegateAPI == delegate) { + _strongDelegateForSetDelegateAPI = nil; + } + + __block MTRDeviceControllerDelegateInfo * delegateInfoToRemove = nil; + [self _iterateDelegateInfoWithBlock:^(MTRDeviceControllerDelegateInfo * delegateInfo) { + if (delegateInfo.delegate == delegate) { + delegateInfoToRemove = delegateInfo; + } + }]; + + if (delegateInfoToRemove) { + [_delegates removeObject:delegateInfoToRemove]; + if (_strongDelegateForSetDelegateAPI == delegate) { + _strongDelegateForSetDelegateAPI = nil; + } + MTR_LOG("%@ removeDeviceControllerDelegate: removed %p remaining %lu", self, delegate, static_cast(_delegates.count)); + } else { + MTR_LOG("%@ removeDeviceControllerDelegate: delegate %p not found in %lu", self, delegate, static_cast(_delegates.count)); + } + } +} + +// Iterates the delegates, and remove delegate info objects if the delegate object has dealloc'ed +// Returns number of delegates called +- (NSUInteger)_iterateDelegateInfoWithBlock:(void (^_Nullable)(MTRDeviceControllerDelegateInfo * delegateInfo))block +{ + @synchronized(self) { + if (!_delegates.count) { + MTR_LOG("%@ No delegates to iterate", self); + return 0; + } + + // Opportunistically remove defunct delegate references on every iteration + NSMutableSet * delegatesToRemove = nil; + for (MTRDeviceControllerDelegateInfo * delegateInfo in _delegates) { + id strongDelegate = delegateInfo.delegate; + if (strongDelegate) { + if (block) { + block(delegateInfo); + } + } else { + if (!delegatesToRemove) { + delegatesToRemove = [NSMutableSet set]; + } + [delegatesToRemove addObject:delegateInfo]; + } + } + + if (delegatesToRemove.count) { + [_delegates minusSet:delegatesToRemove]; + MTR_LOG("%@ _iterateDelegatesWithBlock: removed %lu remaining %lu", self, static_cast(delegatesToRemove.count), static_cast(_delegates.count)); + } + + return _delegates.count; + } +} + +- (void)_callDelegatesWithBlock:(void (^_Nullable)(id delegate))block logString:(const char *)logString; +{ + NSUInteger delegatesCalled = [self _iterateDelegateInfoWithBlock:^(MTRDeviceControllerDelegateInfo * delegateInfo) { + id strongDelegate = delegateInfo.delegate; + dispatch_async(delegateInfo.queue, ^{ + block(strongDelegate); + }); + }]; + + MTR_LOG("%@ %lu delegates called for %s", self, static_cast(delegatesCalled), logString); +} + +#if DEBUG +- (NSUInteger)unitTestDelegateCount +{ + return [self _iterateDelegateInfoWithBlock:nil]; +} +#endif + +- (void)controller:(MTRDeviceController *)controller statusUpdate:(MTRCommissioningStatus)status +{ + [self _callDelegatesWithBlock:^(id delegate) { + if ([delegate respondsToSelector:@selector(controller:statusUpdate:)]) { + [delegate controller:controller statusUpdate:status]; + } + } logString:__PRETTY_FUNCTION__]; +} + +- (void)controller:(MTRDeviceController *)controller commissioningSessionEstablishmentDone:(NSError * _Nullable)error +{ + [self _callDelegatesWithBlock:^(id delegate) { + if ([delegate respondsToSelector:@selector(controller:commissioningSessionEstablishmentDone:)]) { + [delegate controller:controller commissioningSessionEstablishmentDone:error]; + } + } logString:__PRETTY_FUNCTION__]; +} + +- (void)controller:(MTRDeviceController *)controller + commissioningComplete:(NSError * _Nullable)error + nodeID:(NSNumber * _Nullable)nodeID + metrics:(MTRMetrics *)metrics +{ + [self _callDelegatesWithBlock:^(id delegate) { + if ([delegate respondsToSelector:@selector(controller:commissioningComplete:nodeID:metrics:)]) { + [delegate controller:controller commissioningComplete:error nodeID:nodeID metrics:metrics]; + } else if ([delegate respondsToSelector:@selector(controller:commissioningComplete:nodeID:)]) { + [delegate controller:controller commissioningComplete:error nodeID:nodeID]; + } else if ([delegate respondsToSelector:@selector(controller:commissioningComplete:)]) { + [delegate controller:controller commissioningComplete:error]; + } + } logString:__PRETTY_FUNCTION__]; +} + +- (void)controller:(MTRDeviceController *)controller readCommissioningInfo:(MTRProductIdentity *)info +{ + [self _callDelegatesWithBlock:^(id delegate) { + if ([delegate respondsToSelector:@selector(controller:readCommissioningInfo:)]) { + [delegate controller:controller readCommissioningInfo:info]; + } + } logString:__PRETTY_FUNCTION__]; +} + @end // TODO: This should not be in the superclass: either move to diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h index 34c5ca8efad1da..3e83317d8fe780 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h @@ -168,15 +168,7 @@ typedef void (^MTRDeviceConnectionCallback)(MTRBaseDevice * _Nullable device, NS - (void)preWarmCommissioningSession MTR_DEPRECATED("-[MTRDeviceControllerFactory preWarmCommissioningSession]", ios(16.4, 17.6), macos(13.3, 14.6), watchos(9.4, 10.6), tvos(16.4, 17.6)); -/** - * Set the Delegate for the device controller as well as the Queue on which the Delegate callbacks will be triggered - * - * @param[in] delegate The delegate the commissioning process should use - * - * @param[in] queue The queue on which the callbacks will be delivered - */ -- (void)setDeviceControllerDelegate:(id)delegate - queue:(dispatch_queue_t)queue MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4)); +// Use super class implementation for -setDeviceControllerDelegate:queue: /** * Start scanning for commissionable devices. diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm index 1edcb4928943a1..31de6111a328e3 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm @@ -717,6 +717,10 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams commissionerInitialized = YES; + // Set self as delegate, which fans out delegate callbacks to all added delegates + id selfDelegate = static_cast>(self); + self->_deviceControllerDelegateBridge->setDelegate(self, selfDelegate, _chipWorkQueue); + MTR_LOG("%@ startup succeeded for nodeID 0x%016llX", self, self->_cppCommissioner->GetNodeId()); }); @@ -1185,15 +1189,6 @@ - (void)removeDevice:(MTRDevice *)device } #endif -- (void)setDeviceControllerDelegate:(id)delegate queue:(dispatch_queue_t)queue -{ - [self - asyncDispatchToMatterQueue:^() { - self->_deviceControllerDelegateBridge->setDelegate(self, delegate, queue); - } - errorHandler:nil]; -} - - (BOOL)setOperationalCertificateIssuer:(nullable id)operationalCertificateIssuer queue:(nullable dispatch_queue_t)queue { diff --git a/src/darwin/Framework/CHIP/MTRError_Internal.h b/src/darwin/Framework/CHIP/MTRError_Internal.h index af19f4888ff050..c79cc17d95f198 100644 --- a/src/darwin/Framework/CHIP/MTRError_Internal.h +++ b/src/darwin/Framework/CHIP/MTRError_Internal.h @@ -26,10 +26,6 @@ NS_ASSUME_NONNULL_BEGIN -#ifndef YES_NO -#define YES_NO(x) ((x) ? @"YES" : @"NO") -#endif - MTR_DIRECT_MEMBERS @interface MTRError : NSObject + (NSError *)errorWithCode:(MTRErrorCode)code; diff --git a/src/darwin/Framework/CHIPTests/MTRPairingTests.m b/src/darwin/Framework/CHIPTests/MTRPairingTests.m index cd1802dff3dd47..4e597961603ee9 100644 --- a/src/darwin/Framework/CHIPTests/MTRPairingTests.m +++ b/src/darwin/Framework/CHIPTests/MTRPairingTests.m @@ -18,9 +18,11 @@ // module headers #import +#import "MTRDefines_Internal.h" #import "MTRErrorTestUtils.h" #import "MTRTestCase+ServerAppRunner.h" #import "MTRTestCase.h" +#import "MTRTestDeclarations.h" #import "MTRTestKeys.h" #import "MTRTestStorage.h" @@ -131,6 +133,42 @@ - (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSEr @end +@interface MTRPairingTestMonitoringControllerDelegate : NSObject +@property (atomic, readwrite) BOOL statusUpdateCalled; +@property (atomic, readwrite) BOOL commissioningSessionEstablishmentDoneCalled; +@property (atomic, readwrite) BOOL commissioningCompleteCalled; +@property (atomic, readwrite) BOOL readCommissioningInfoCalled; +@end + +@implementation MTRPairingTestMonitoringControllerDelegate +- (NSString *)description +{ + return [NSString stringWithFormat:@"", self, YES_NO(_statusUpdateCalled), YES_NO(_commissioningSessionEstablishmentDoneCalled), YES_NO(_commissioningCompleteCalled), YES_NO(_readCommissioningInfoCalled)]; +} +- (void)controller:(MTRDeviceController *)controller statusUpdate:(MTRCommissioningStatus)status +{ + self.statusUpdateCalled = YES; +} + +- (void)controller:(MTRDeviceController *)controller commissioningSessionEstablishmentDone:(NSError * _Nullable)error +{ + self.commissioningSessionEstablishmentDoneCalled = YES; +} + +- (void)controller:(MTRDeviceController *)controller + commissioningComplete:(NSError * _Nullable)error + nodeID:(NSNumber * _Nullable)nodeID + metrics:(MTRMetrics *)metrics +{ + self.commissioningCompleteCalled = YES; +} + +- (void)controller:(MTRDeviceController *)controller readCommissioningInfo:(MTRProductIdentity *)info +{ + self.readCommissioningInfoCalled = YES; +} +@end + @interface MTRPairingTests : MTRTestCase @property (nullable) MTRPairingTestControllerDelegate * controllerDelegate; @end @@ -219,6 +257,19 @@ - (void)doPairingTestWithAttestationDelegate:(id)a [sController setDeviceControllerDelegate:controllerDelegate queue:callbackQueue]; self.controllerDelegate = controllerDelegate; + // Test that a monitoring delegate works + __auto_type * monitoringControllerDelegate = [[MTRPairingTestMonitoringControllerDelegate alloc] init]; + [sController addDeviceControllerDelegate:monitoringControllerDelegate queue:callbackQueue]; + XCTAssertEqual([sController unitTestDelegateCount], 2); + + // Test that the addDeviceControllerDelegate delegate is held weakly by the controller + @autoreleasepool { + __auto_type * monitoringControllerDelegate = [[MTRPairingTestMonitoringControllerDelegate alloc] init]; + [sController addDeviceControllerDelegate:monitoringControllerDelegate queue:callbackQueue]; + XCTAssertEqual([sController unitTestDelegateCount], 3); + } + XCTAssertEqual([sController unitTestDelegateCount], 2); + NSError * error; __auto_type * payload = [MTRSetupPayload setupPayloadWithOnboardingPayload:kOnboardingPayload error:&error]; XCTAssertNotNil(payload); @@ -229,6 +280,13 @@ - (void)doPairingTestWithAttestationDelegate:(id)a [self waitForExpectations:@[ expectation ] timeout:kPairingTimeoutInSeconds]; XCTAssertNil(controllerDelegate.commissioningCompleteError); + + // Test that the monitoring delegate got all the callbacks + XCTAssertTrue(monitoringControllerDelegate.statusUpdateCalled); + XCTAssertTrue(monitoringControllerDelegate.commissioningSessionEstablishmentDoneCalled); + XCTAssertTrue(monitoringControllerDelegate.commissioningCompleteCalled); + XCTAssertTrue(monitoringControllerDelegate.readCommissioningInfoCalled); + [sController removeDeviceControllerDelegate:monitoringControllerDelegate]; } - (void)test001_PairWithoutAttestationDelegate diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h index 46d6c61e950f2d..22998eb4496216 100644 --- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h +++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h @@ -18,6 +18,9 @@ #import #import +// For MTRDeviceDataValueDictionary: +#import "MTRDevice_Internal.h" + NS_ASSUME_NONNULL_BEGIN #pragma mark - Declarations for internal methods @@ -55,6 +58,7 @@ NS_ASSUME_NONNULL_BEGIN #ifdef DEBUG @interface MTRDeviceController (TestDebug) - (NSDictionary *)unitTestGetDeviceAttributeCounts; +- (NSUInteger)unitTestDelegateCount; @end @interface MTRBaseDevice (TestDebug)