commit 3a7c922137297ed196cc3411eef0827694aab68d
author Boris Zbarsky <bzbarsky@apple.com> Mon Oct 07 17:23:24 2024 -0400
committer GitHub <noreply@github.com> Mon Oct 07 21:23:24 2024 +0000
tree 9967a9b3de18bfcb2c942502c0887c197992c1b5
parent 3adf99ef53ada8ab70c9ee3cb167537973c3c5c5

Fix handling of controllerNodeID on non-concrete controllers. (#35946)

There isn't actually a _cppCommissioner on the base controller, so that
implementation made no sense.  In practice, the XPC and concrete controller just
have different implementations here, and the base should not try to implement
anything.

Also fixes the concrete implementation to avoid a sync dispatch by caching the
value (which should be immutable) during startup and removes the unnecessary
controllerNodeId override from the concrete controller: the base class handles
that backwards compat shim already.

src/darwin/Framework/CHIP/MTRDeviceController.mm

diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm
index 32f4294..63fb3b5 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceController.mm
+++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm
@@ -318,16 +318,10 @@
     // Subclass hook; nothing to do.
 }
 
-- (NSNumber *)controllerNodeID
+- (nullable NSNumber *)controllerNodeID
 {
-    auto block = ^NSNumber * { return @(self->_cppCommissioner->GetNodeId()); };
-
-    NSNumber * nodeID = [self syncRunOnWorkQueueWithReturnValue:block error:nil];
-    if (!nodeID) {
-        MTR_LOG_ERROR("%@ A controller has no node id if it has not been started", self);
-    }
-
-    return nodeID;
+    MTR_ABSTRACT_METHOD();
+    return nil;
 }
 
 - (BOOL)setupCommissioningSessionWithPayload:(MTRSetupPayload *)payload

src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm

diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm
index 92d04cd..cae4103 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm
+++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm
@@ -141,6 +141,7 @@
 @synthesize commissionableBrowser = _commissionableBrowser;
 @synthesize concurrentSubscriptionPool = _concurrentSubscriptionPool;
 @synthesize storageBehaviorConfiguration = _storageBehaviorConfiguration;
+@synthesize controllerNodeID = _controllerNodeID;
 
 - (nullable instancetype)initWithParameters:(MTRDeviceControllerAbstractParameters *)parameters
                                       error:(NSError * __autoreleasing *)error
@@ -729,6 +730,7 @@
 
         self->_storedFabricIndex = fabricIdx;
         self->_storedCompressedFabricID = _cppCommissioner->GetCompressedFabricId();
+        self->_controllerNodeID = @(_cppCommissioner->GetNodeId());
 
         chip::Crypto::P256PublicKey rootPublicKey;
         if (_cppCommissioner->GetRootPublicKey(rootPublicKey) == CHIP_NO_ERROR) {
@@ -793,18 +795,6 @@
     return YES;
 }
 
-- (NSNumber *)controllerNodeID
-{
-    auto block = ^NSNumber * { return @(self->_cppCommissioner->GetNodeId()); };
-
-    NSNumber * nodeID = [self syncRunOnWorkQueueWithReturnValue:block error:nil];
-    if (!nodeID) {
-        MTR_LOG_ERROR("%@ A controller has no node id if it has not been started", self);
-    }
-
-    return nodeID;
-}
-
 static inline void emitMetricForSetupPayload(MTRSetupPayload * payload)
 {
     MATTER_LOG_METRIC(kMetricDeviceVendorID, [payload.vendorID unsignedIntValue]);
@@ -1712,11 +1702,6 @@
 
 @implementation MTRDeviceController_Concrete (Deprecated)
 
-- (NSNumber *)controllerNodeId
-{
-    return self.controllerNodeID;
-}
-
 - (nullable NSData *)fetchAttestationChallengeForDeviceId:(uint64_t)deviceId
 {
     return [self attestationChallengeForDeviceID:@(deviceId)];