Skip to content

Commit

Permalink
Fix handling of controllerNodeID on non-concrete controllers. (#35946)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bzbarsky-apple authored Oct 7, 2024
1 parent 3adf99e commit 3a7c922
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 26 deletions.
12 changes: 3 additions & 9 deletions src/darwin/Framework/CHIP/MTRDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -318,16 +318,10 @@ - (void)shutdown
// 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
Expand Down
19 changes: 2 additions & 17 deletions src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ @implementation MTRDeviceController_Concrete {
@synthesize commissionableBrowser = _commissionableBrowser;
@synthesize concurrentSubscriptionPool = _concurrentSubscriptionPool;
@synthesize storageBehaviorConfiguration = _storageBehaviorConfiguration;
@synthesize controllerNodeID = _controllerNodeID;

- (nullable instancetype)initWithParameters:(MTRDeviceControllerAbstractParameters *)parameters
error:(NSError * __autoreleasing *)error
Expand Down Expand Up @@ -729,6 +730,7 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams

self->_storedFabricIndex = fabricIdx;
self->_storedCompressedFabricID = _cppCommissioner->GetCompressedFabricId();
self->_controllerNodeID = @(_cppCommissioner->GetNodeId());

chip::Crypto::P256PublicKey rootPublicKey;
if (_cppCommissioner->GetRootPublicKey(rootPublicKey) == CHIP_NO_ERROR) {
Expand Down Expand Up @@ -793,18 +795,6 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams
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]);
Expand Down Expand Up @@ -1712,11 +1702,6 @@ - (instancetype)initWithIssuer:(id<MTRNOCChainIssuer>)nocChainIssuer;

@implementation MTRDeviceController_Concrete (Deprecated)

- (NSNumber *)controllerNodeId
{
return self.controllerNodeID;
}

- (nullable NSData *)fetchAttestationChallengeForDeviceId:(uint64_t)deviceId
{
return [self attestationChallengeForDeviceID:@(deviceId)];
Expand Down

0 comments on commit 3a7c922

Please sign in to comment.