From 5557626149017e920f82d792e1ca0b159627e2ff Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Sat, 7 Sep 2024 19:52:10 -0700 Subject: [PATCH] Fixing shadow variables, and also fixing registration (#35473) --- src/darwin/Framework/CHIP/MTRDeviceController.mm | 4 +++- .../Framework/CHIP/MTRDeviceController_Concrete.h | 5 ----- .../CHIP/MTRDeviceController_Concrete.mm | 15 +++++---------- .../Framework/CHIP/MTRDeviceController_Internal.h | 2 ++ .../Framework/CHIP/MTRDeviceController_XPC.mm | 8 ++++++++ 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index a55946c237c364..4f41330b54c3dd 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -142,6 +142,8 @@ @implementation MTRDeviceController { os_unfair_lock _assertionLock; } +@synthesize uniqueIdentifier = _uniqueIdentifier; + - (os_unfair_lock_t)deviceMapLock { return &_underlyingDeviceMapLock; @@ -340,7 +342,7 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory - (NSString *)description { - return [NSString stringWithFormat:@"<%@: %p uuid %@>", NSStringFromClass(self.class), self, _uniqueIdentifier]; + return [NSString stringWithFormat:@"<%@: %p uuid %@>", NSStringFromClass(self.class), self, self.uniqueIdentifier]; } - (BOOL)isRunning diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h index c9587e910d3b1a..34c5ca8efad1da 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h @@ -58,11 +58,6 @@ typedef void (^MTRDeviceConnectionCallback)(MTRBaseDevice * _Nullable device, NS */ @property (readonly, nonatomic, getter=isRunning) BOOL running; -/** - * The ID assigned to this controller at creation time. - */ -@property (readonly, nonatomic) NSUUID * uniqueIdentifier MTR_AVAILABLE(ios(17.6), macos(14.6), watchos(10.6), tvos(17.6)); - /** * Return the Node ID assigned to the controller. Will return nil if the * controller is not running (and hence does not know its node id). diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm index fbfc072826ad8a..4f19035f27da85 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm @@ -101,7 +101,6 @@ @interface MTRDeviceController_Concrete () @property (nonatomic, readonly) MTRDeviceControllerDelegateBridge * deviceControllerDelegateBridge; @property (nonatomic, readonly) MTROperationalCredentialsDelegate * operationalCredentialsDelegate; @property (nonatomic, readonly) MTRDeviceAttestationDelegateBridge * deviceAttestationDelegateBridge; -@property (nonatomic, readwrite) NSUUID * uniqueIdentifier; @property (nonatomic, readonly) dispatch_queue_t chipWorkQueue; @property (nonatomic, readonly, nullable) MTRDeviceControllerFactory * factory; @property (nonatomic, readonly, nullable) id otaProviderDelegate; @@ -130,9 +129,6 @@ @implementation MTRDeviceController_Concrete { os_unfair_lock _assertionLock; } -// TODO: Figure out whether uniqueIdentifier storage should live in the superclass. It -// probably should! -@synthesize uniqueIdentifier = _uniqueIdentifier; // TODO: Figure out whether the work queue storage lives here or in the superclass // Right now we seem to have both? @synthesize chipWorkQueue = _chipWorkQueue; @@ -208,7 +204,7 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory if (self = [super initForSubclasses:startSuspended]) { // Make sure our storage is all set up to work as early as possible, // before we start doing anything else with the controller. - _uniqueIdentifier = uniqueIdentifier; + self.uniqueIdentifier = uniqueIdentifier; // Setup assertion variables _keepRunningAssertionCounter = 0; @@ -340,7 +336,7 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory - (NSString *)description { - return [NSString stringWithFormat:@"<%@: %p uuid %@>", NSStringFromClass(self.class), self, _uniqueIdentifier]; + return [NSString stringWithFormat:@"<%@: %p uuid %@>", NSStringFromClass(self.class), self, self.uniqueIdentifier]; } - (BOOL)isRunning @@ -744,7 +740,7 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams if (_controllerDataStore) { // If the storage delegate supports the bulk read API, then a dictionary of nodeID => cluster data dictionary would be passed to the handler. Otherwise this would be a no-op, and stored attributes for MTRDevice objects will be loaded lazily in -deviceForNodeID:. [_controllerDataStore fetchAttributeDataForAllDevices:^(NSDictionary *> * _Nonnull clusterDataByNode) { - MTR_LOG("%@ Loaded attribute values for %lu nodes from storage for controller uuid %@", self, static_cast(clusterDataByNode.count), self->_uniqueIdentifier); + MTR_LOG("%@ Loaded attribute values for %lu nodes from storage for controller uuid %@", self, static_cast(clusterDataByNode.count), self.uniqueIdentifier); std::lock_guard lock(*self.deviceMapLock); NSMutableArray * deviceList = [NSMutableArray array]; @@ -1290,7 +1286,7 @@ - (BOOL)addServerEndpoint:(MTRServerEndpoint *)endpoint [self->_serverEndpoints addObject:endpoint]; [endpoint registerMatterEndpoint]; MTR_LOG("%@ Added server endpoint %u to controller %@", self, static_cast(endpoint.endpointID.unsignedLongLongValue), - self->_uniqueIdentifier); + self.uniqueIdentifier); } errorHandler:^(NSError * error) { MTR_LOG_ERROR("%@ Unexpected failure dispatching to Matter queue on running controller in addServerEndpoint, adding endpoint %u", self, @@ -1317,8 +1313,7 @@ - (void)removeServerEndpointInternal:(MTRServerEndpoint *)endpoint queue:(dispat // tearing it down. [self asyncDispatchToMatterQueue:^() { [self removeServerEndpointOnMatterQueue:endpoint]; - MTR_LOG("%@ Removed server endpoint %u from controller %@", self, static_cast(endpoint.endpointID.unsignedLongLongValue), - self->_uniqueIdentifier); + MTR_LOG("%@ Removed server endpoint %u from controller %@", self, static_cast(endpoint.endpointID.unsignedLongLongValue), self.uniqueIdentifier); if (queue != nil && completion != nil) { dispatch_async(queue, completion); } diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h index 643a23225774fd..993298234f33ef 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h @@ -69,6 +69,8 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) NSMapTable * nodeIDToDeviceMap; @property (readonly, assign) os_unfair_lock_t deviceMapLock; +@property (readwrite, nonatomic) NSUUID * uniqueIdentifier; + // queue used to serialize all work performed by the MTRDeviceController // (moved here so subclasses can initialize differently) @property (readwrite, retain) dispatch_queue_t chipWorkQueue; diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_XPC.mm b/src/darwin/Framework/CHIP/MTRDeviceController_XPC.mm index eb21dadeb8462b..5b6dcf93f586f8 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_XPC.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController_XPC.mm @@ -236,6 +236,7 @@ - (nullable instancetype)initWithParameters:(MTRDeviceControllerAbstractParamete return nil; } + self.uniqueIdentifier = UUID; self.xpcParameters = xpcParameters; self.chipWorkQueue = dispatch_queue_create("MTRDeviceController_XPC_queue", DISPATCH_QUEUE_SERIAL_WITH_AUTORELEASE_POOL); @@ -285,6 +286,13 @@ - (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(N MTRDevice * deviceToReturn = [[MTRDevice_XPC alloc] initWithNodeID:nodeID controller:self]; [self.nodeIDToDeviceMap setObject:deviceToReturn forKey:nodeID]; MTR_LOG("%s: returning XPC device for node id %@", __PRETTY_FUNCTION__, nodeID); + + mtr_weakify(self); + [[self.xpcConnection synchronousRemoteObjectProxyWithErrorHandler:^(NSError * _Nonnull error) { + mtr_strongify(self); + MTR_LOG_ERROR("%@ Registration error for device nodeID: %@ : %@", self, nodeID, error); + }] deviceController:self.uniqueIdentifier registerNodeID:nodeID]; + return deviceToReturn; }