diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 552c99c7e873b4..1b3ebedbe687fd 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -432,13 +432,6 @@ - (void)invalidate [_delegates removeAllObjects]; } -- (void)nodeMayBeAdvertisingOperational -{ - assertChipStackLockedByCurrentThread(); - - MTR_LOG("%@ saw new operational advertisement", self); -} - - (BOOL)_delegateExists { os_unfair_lock_assert_owner(&self->_lock); diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index d1759e3008eee8..09a7f99d03b0ad 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -729,22 +729,6 @@ - (void)invalidateCASESessionForNode:(chip::NodeId)nodeID; [self syncRunOnWorkQueue:block error:nil]; } -- (void)operationalInstanceAdded:(chip::NodeId)nodeID -{ - // Don't use deviceForNodeID here, because we don't want to create the - // device if it does not already exist. - os_unfair_lock_lock(self.deviceMapLock); - MTRDevice * device = [_nodeIDToDeviceMap objectForKey:@(nodeID)]; - os_unfair_lock_unlock(self.deviceMapLock); - - if (device == nil) { - return; - } - - ChipLogProgress(Controller, "Notifying device about node 0x" ChipLogFormatX64 " advertising", ChipLogValueX64(nodeID)); - [device nodeMayBeAdvertisingOperational]; -} - - (void)downloadLogFromNodeWithID:(NSNumber *)nodeID type:(MTRDiagnosticLogType)type timeout:(NSTimeInterval)timeout diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm index 4cff8a633105b2..c19fb835b614ee 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm @@ -1132,7 +1132,7 @@ - (void)operationalInstanceAdded:(chip::PeerId &)operationalID if (compressedFabricId != nil && compressedFabricId.unsignedLongLongValue == operationalID.GetCompressedFabricId()) { ChipLogProgress(Controller, "Notifying controller at fabric index %u about new operational node 0x" ChipLogFormatX64, controller.fabricIndex, ChipLogValueX64(operationalID.GetNodeId())); - [controller operationalInstanceAdded:operationalID.GetNodeId()]; + [controller operationalInstanceAdded:@(operationalID.GetNodeId())]; } // Keep going: more than one controller might match a given compressed diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h index ea71f351c72377..cb138f7edcd3fd 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h @@ -109,6 +109,12 @@ NS_ASSUME_NONNULL_BEGIN */ - (void)clearPendingShutdown; +/** + * Notify the controller that a new operational instance with the given node id + * and a compressed fabric id that matches this controller has been observed. + */ +- (void)operationalInstanceAdded:(NSNumber *)nodeID; + @end NS_ASSUME_NONNULL_END diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm index d0cab185e40bf6..cb86c5d2db044f 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm @@ -1633,20 +1633,28 @@ - (void)invalidateCASESessionForNode:(chip::NodeId)nodeID; [self syncRunOnWorkQueue:block error:nil]; } -- (void)operationalInstanceAdded:(chip::NodeId)nodeID +- (void)operationalInstanceAdded:(NSNumber *)nodeID { // Don't use deviceForNodeID here, because we don't want to create the // device if it does not already exist. os_unfair_lock_lock(self.deviceMapLock); - MTRDevice * device = [self.nodeIDToDeviceMap objectForKey:@(nodeID)]; + MTRDevice * device = [self.nodeIDToDeviceMap objectForKey:nodeID]; os_unfair_lock_unlock(self.deviceMapLock); if (device == nil) { return; } - ChipLogProgress(Controller, "Notifying device about node 0x" ChipLogFormatX64 " advertising", ChipLogValueX64(nodeID)); - [device nodeMayBeAdvertisingOperational]; + // TODO: Can we not just assume this isKindOfClass test is true? Would be + // really nice if we had compile-time checking for this somehow... + if (![device isKindOfClass:MTRDevice_Concrete.class]) { + MTR_LOG_ERROR("%@ somehow has %@ instead of MTRDevice_Concrete for node ID 0x%016llX (%llu)", self, device, nodeID.unsignedLongLongValue, nodeID.unsignedLongLongValue); + return; + } + + MTR_LOG("%@ Notifying %@ about its node advertising", self, device); + auto * concreteDevice = static_cast(device); + [concreteDevice nodeMayBeAdvertisingOperational]; } - (void)downloadLogFromNodeWithID:(NSNumber *)nodeID diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h index a052d527b89b86..e961910cc96b41 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Internal.h @@ -201,12 +201,6 @@ NS_ASSUME_NONNULL_BEGIN */ - (MTRBaseDevice *)baseDeviceForNodeID:(NSNumber *)nodeID; -/** - * Notify the controller that a new operational instance with the given node id - * and a compressed fabric id that matches this controller has been observed. - */ -- (void)operationalInstanceAdded:(chip::NodeId)nodeID; - /** * Download log of the desired type from the device. */ diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.h b/src/darwin/Framework/CHIP/MTRDevice_Concrete.h index 74a9a5788c5b8b..70dac9a362b10d 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.h +++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.h @@ -26,6 +26,11 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController_Concrete *)controller; +// Called by controller when a new operational advertisement for what we think +// is this device's identity has been observed. This could have +// false-positives, for example due to compressed fabric id collisions. +- (void)nodeMayBeAdvertisingOperational; + @end NS_ASSUME_NONNULL_END diff --git a/src/darwin/Framework/CHIP/MTRDevice_Internal.h b/src/darwin/Framework/CHIP/MTRDevice_Internal.h index 6aea99eeda1ffb..9d7f9db21b00b7 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDevice_Internal.h @@ -127,11 +127,6 @@ MTR_DIRECT_MEMBERS // called by controller to clean up and shutdown - (void)invalidate; -// Called by controller when a new operational advertisement for what we think -// is this device's identity has been observed. This could have -// false-positives, for example due to compressed fabric id collisions. -- (void)nodeMayBeAdvertisingOperational; - - (BOOL)_callDelegatesWithBlock:(void (^)(id delegate))block; // Called by MTRDevice_XPC to forward delegate callbacks