Move the operationalInstanceAdded/nodeMayBeAdvertisingOperational bit…

…s to concrete device/controller. These are only used in the concrete case.
bzbarsky-apple · Sep 26, 2024 · 1084acb · 1084acb
1 parent 8323604
commit 1084acb
Show file tree

Hide file tree

Showing 8 changed files with 24 additions and 39 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<MTRDevice_Concrete *>(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
@@ -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
@@ -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
@@ -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<MTRDeviceDelegate> delegate))block;
 
 // Called by MTRDevice_XPC to forward delegate callbacks