From 1f8bc3a998c8d75d469227b4d5b1a7e814bdd7b7 Mon Sep 17 00:00:00 2001 From: Jeff Tung <100387939+jtung-apple@users.noreply.github.com> Date: Mon, 9 Sep 2024 18:55:48 -0700 Subject: [PATCH] [Darwin] Follow-up changes for PR#35475 --- .../Framework/CHIP/MTRDefines_Internal.h | 4 +--- src/darwin/Framework/CHIP/MTRDevice.mm | 2 +- .../Framework/CHIP/MTRDeviceController.h | 7 +++++++ .../Framework/CHIP/MTRDeviceController.mm | 21 ++++++++++++++----- .../Framework/CHIP/MTRDevice_Concrete.mm | 6 +++--- src/darwin/Framework/CHIP/MTRDevice_XPC.mm | 4 ++-- .../Framework/CHIPTests/MTRPairingTests.m | 2 +- 7 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDefines_Internal.h b/src/darwin/Framework/CHIP/MTRDefines_Internal.h index 94a1cbb3da61f4..1ff3a79c970e83 100644 --- a/src/darwin/Framework/CHIP/MTRDefines_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDefines_Internal.h @@ -151,6 +151,4 @@ typedef struct {} variable_hidden_by_mtr_hide; } #endif -#ifndef YES_NO -#define YES_NO(x) ((x) ? @"YES" : @"NO") -#endif +#define MTR_YES_NO(x) ((x) ? @"YES" : @"NO") diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index ddc47965f9c8b1..ea4599ffe612a5 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -1170,7 +1170,7 @@ - (nullable MTRDeviceClusterData *)_clusterDataForPath:(MTRClusterPath *)cluster // Page in the stored value for the data. MTRDeviceClusterData * data = [_deviceController.controllerDataStore getStoredClusterDataForNodeID:_nodeID endpointID:clusterPath.endpoint clusterID:clusterPath.cluster]; - MTR_LOG("%@ cluster path %@ cache miss - load from storage success %@", self, clusterPath, YES_NO(data)); + MTR_LOG("%@ cluster path %@ cache miss - load from storage success %@", self, clusterPath, MTR_YES_NO(data)); if (data != nil) { [_persistedClusterData setObject:data forKey:clusterPath]; } else { diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.h b/src/darwin/Framework/CHIP/MTRDeviceController.h index d731b13052798c..0b553c9d43d25e 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController.h @@ -198,6 +198,13 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1)) /** * Adds a Delegate to the device controller as well as the Queue on which the Delegate callbacks will be triggered * + * Multiple delegates can be added to monitor MTRDeviceController state changes. Note that there should only + * be one delegate that responds to pairing related callbacks. + * + * If a delegate is added a second time, the call would be ignored. + * + * All delegates are held by weak references, and so if a delegate object goes away, it will be automatically removed. + * * @param[in] delegate The delegate the commissioning process should use * * @param[in] queue The queue on which the callbacks will be delivered diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 9dc9f5a147db81..5f7d3e3eae6ca9 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -161,7 +161,7 @@ @implementation MTRDeviceController { BOOL _shutdownPending; os_unfair_lock _assertionLock; - NSMutableSet * _delegates; + NSMutableArray * _delegates; id _strongDelegateForSetDelegateAPI; } @@ -192,7 +192,7 @@ - (instancetype)initForSubclasses:(BOOL)startSuspended _nodeIDToDeviceMap = [NSMapTable strongToWeakObjectsMapTable]; - _delegates = [NSMutableSet set]; + _delegates = [NSMutableArray array]; return self; } @@ -1795,6 +1795,17 @@ - (void)setDeviceControllerDelegate:(id)delegate qu - (void)addDeviceControllerDelegate:(id)delegate queue:(dispatch_queue_t)queue { @synchronized(self) { + __block BOOL delegateAlreadyAdded = NO; + [self _iterateDelegateInfoWithBlock:^(MTRDeviceControllerDelegateInfo *delegateInfo) { + if (delegateInfo.delegate == delegate) { + delegateAlreadyAdded = YES; + } + }]; + if (delegateAlreadyAdded) { + MTR_LOG("%@ addDeviceControllerDelegate: delegate already added", self); + return; + } + MTRDeviceControllerDelegateInfo * newDelegateInfo = [[MTRDeviceControllerDelegateInfo alloc] initWithDelegate:delegate queue:queue]; [_delegates addObject:newDelegateInfo]; MTR_LOG("%@ addDeviceControllerDelegate: added %p total %lu", self, delegate, static_cast(_delegates.count)); @@ -1838,7 +1849,7 @@ - (NSUInteger)_iterateDelegateInfoWithBlock:(void (^_Nullable)(MTRDeviceControll } // Opportunistically remove defunct delegate references on every iteration - NSMutableSet * delegatesToRemove = nil; + NSMutableArray * delegatesToRemove = nil; for (MTRDeviceControllerDelegateInfo * delegateInfo in _delegates) { id strongDelegate = delegateInfo.delegate; if (strongDelegate) { @@ -1847,14 +1858,14 @@ - (NSUInteger)_iterateDelegateInfoWithBlock:(void (^_Nullable)(MTRDeviceControll } } else { if (!delegatesToRemove) { - delegatesToRemove = [NSMutableSet set]; + delegatesToRemove = [NSMutableArray array]; } [delegatesToRemove addObject:delegateInfo]; } } if (delegatesToRemove.count) { - [_delegates minusSet:delegatesToRemove]; + [_delegates removeObjectsInArray:delegatesToRemove]; MTR_LOG("%@ _iterateDelegatesWithBlock: removed %lu remaining %lu", self, static_cast(delegatesToRemove.count), static_cast(_delegates.count)); } diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm index 5bf35f742c1e38..a35bc13431ad08 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm @@ -442,8 +442,8 @@ - (NSString *)description wifi = @"NO"; thread = @"NO"; } else { - wifi = YES_NO(networkFeatures.unsignedLongLongValue & MTRNetworkCommissioningFeatureWiFiNetworkInterface); - thread = YES_NO(networkFeatures.unsignedLongLongValue & MTRNetworkCommissioningFeatureThreadNetworkInterface); + wifi = MTR_YES_NO(networkFeatures.unsignedLongLongValue & MTRNetworkCommissioningFeatureWiFiNetworkInterface); + thread = MTR_YES_NO(networkFeatures.unsignedLongLongValue & MTRNetworkCommissioningFeatureThreadNetworkInterface); } NSString * reportAge; @@ -2087,7 +2087,7 @@ - (nullable MTRDeviceClusterData *)_clusterDataForPath:(MTRClusterPath *)cluster // Page in the stored value for the data. MTRDeviceClusterData * data = [_deviceController.controllerDataStore getStoredClusterDataForNodeID:_nodeID endpointID:clusterPath.endpoint clusterID:clusterPath.cluster]; - MTR_LOG("%@ cluster path %@ cache miss - load from storage success %@", self, clusterPath, YES_NO(data)); + MTR_LOG("%@ cluster path %@ cache miss - load from storage success %@", self, clusterPath, MTR_YES_NO(data)); if (data != nil) { [_persistedClusterData setObject:data forKey:clusterPath]; } else { diff --git a/src/darwin/Framework/CHIP/MTRDevice_XPC.mm b/src/darwin/Framework/CHIP/MTRDevice_XPC.mm index 46f143e4bc189e..d7fbab4b505eec 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_XPC.mm +++ b/src/darwin/Framework/CHIP/MTRDevice_XPC.mm @@ -111,8 +111,8 @@ - (NSString *)description wifi = @"NO"; thread = @"NO"; } else { - wifi = YES_NO(networkFeatures.unsignedLongLongValue & MTRNetworkCommissioningFeatureWiFiNetworkInterface); - thread = YES_NO(networkFeatures.unsignedLongLongValue & MTRNetworkCommissioningFeatureThreadNetworkInterface); + wifi = MTR_YES_NO(networkFeatures.unsignedLongLongValue & MTRNetworkCommissioningFeatureWiFiNetworkInterface); + thread = MTR_YES_NO(networkFeatures.unsignedLongLongValue & MTRNetworkCommissioningFeatureThreadNetworkInterface); } // TODO: Add these to the description diff --git a/src/darwin/Framework/CHIPTests/MTRPairingTests.m b/src/darwin/Framework/CHIPTests/MTRPairingTests.m index 4e597961603ee9..d4fcfb786288b2 100644 --- a/src/darwin/Framework/CHIPTests/MTRPairingTests.m +++ b/src/darwin/Framework/CHIPTests/MTRPairingTests.m @@ -143,7 +143,7 @@ @interface MTRPairingTestMonitoringControllerDelegate : NSObject ", self, YES_NO(_statusUpdateCalled), YES_NO(_commissioningSessionEstablishmentDoneCalled), YES_NO(_commissioningCompleteCalled), YES_NO(_readCommissioningInfoCalled)]; + return [NSString stringWithFormat:@"", self, MTR_YES_NO(_statusUpdateCalled), MTR_YES_NO(_commissioningSessionEstablishmentDoneCalled), MTR_YES_NO(_commissioningCompleteCalled), MTR_YES_NO(_readCommissioningInfoCalled)]; } - (void)controller:(MTRDeviceController *)controller statusUpdate:(MTRCommissioningStatus)status {