Skip to content

Commit

Permalink
Darwin: Consistently use ivars in MTRDeviceController and a few other…
Browse files Browse the repository at this point in the history 
… places (project-chip#31141)
  • Loading branch information
@ksperling-apple
ksperling-apple authored Dec 21, 2023
1 parent bd24a65 commit b7e3bc0
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 69 deletions.
15 changes: 7 additions & 8 deletions examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,14 @@
static NSString * const kOperationalCredentialsIssuerKeypairStorage = @"ChipToolOpCredsCAKey";
static NSString * const kOperationalCredentialsIPK = @"ChipToolOpCredsIPK";

@interface CHIPToolKeypair ()
@property (nonatomic) chip::Crypto::P256Keypair mKeyPair;
@property (nonatomic) chip::Crypto::P256Keypair mIssuer;
@property (nonatomic) NSData * ipk;
@property (atomic) uint32_t mNow;
@property (nonatomic, readonly) SecKeyRef mPublicKey;
@end
@implementation CHIPToolKeypair {
chip::Crypto::P256Keypair _mKeyPair;
chip::Crypto::P256Keypair _mIssuer;
NSData * _ipk;
uint32_t _mNow;
SecKeyRef _mPublicKey;
}

@implementation CHIPToolKeypair
- (instancetype)init
{
if (self = [super init]) {
Expand Down
3 changes: 0 additions & 3 deletions examples/darwin-framework-tool/commands/pairing/DeviceControllerDelegateBridge.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@
#include "DeviceControllerDelegateBridge.h"
#import <Matter/Matter.h>

@interface CHIPToolDeviceControllerDelegate ()
@end

@implementation CHIPToolDeviceControllerDelegate
- (void)controller:(MTRDeviceController *)controller statusUpdate:(MTRCommissioningStatus)status
{
Expand Down
4 changes: 4 additions & 0 deletions src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
* limitations under the License.
*/

// NOTE: This class was not intended to be part of the public Matter API;
// internally this class has been replaced by MTRAsyncWorkQueue. This code
// remains here simply to preserve API/ABI compatibility.

#import <dispatch/dispatch.h>
#import <os/lock.h>

Expand Down
4 changes: 1 addition & 3 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2744,12 +2744,10 @@ - (id)copyWithZone:(NSZone *)zone

@end

@interface MTREventReport () {
@implementation MTREventReport {
NSNumber * _timestampValue;
}
@end

@implementation MTREventReport
+ (void)initialize
{
// One of our init methods ends up doing Platform::MemoryAlloc.
Expand Down
89 changes: 43 additions & 46 deletions src/darwin/Framework/CHIP/MTRDeviceController.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,31 +101,27 @@
typedef id (^SyncWorkQueueBlockWithReturnValue)(void);
typedef BOOL (^SyncWorkQueueBlockWithBoolReturnValue)(void);

@interface MTRDeviceController () {
@implementation MTRDeviceController {
// Atomic because it can be touched from multiple threads.
std::atomic<chip::FabricIndex> _storedFabricIndex;
}

// queue used to serialize all work performed by the MTRDeviceController
@property (atomic, readonly) dispatch_queue_t chipWorkQueue;

@property (readonly) chip::Controller::DeviceCommissioner * cppCommissioner;
@property (readonly) chip::Credentials::PartialDACVerifier * partialDACVerifier;
@property (readonly) chip::Credentials::DefaultDACVerifier * defaultDACVerifier;
@property (readonly) MTRDeviceControllerDelegateBridge * deviceControllerDelegateBridge;
@property (readonly) MTROperationalCredentialsDelegate * operationalCredentialsDelegate;
@property (readonly) MTRP256KeypairBridge signingKeypairBridge;
@property (readonly) MTRP256KeypairBridge operationalKeypairBridge;
@property (readonly) MTRDeviceAttestationDelegateBridge * deviceAttestationDelegateBridge;
@property (readonly) MTRDeviceControllerFactory * factory;
@property (readonly) NSMutableDictionary * nodeIDToDeviceMap;
@property (readonly) os_unfair_lock deviceMapLock; // protects nodeIDToDeviceMap
@property (readonly) MTRCommissionableBrowser * commissionableBrowser;
@property (readonly) MTRAttestationTrustStoreBridge * attestationTrustStoreBridge;
// queue used to serialize all work performed by the MTRDeviceController
dispatch_queue_t _chipWorkQueue;

@end

@implementation MTRDeviceController
chip::Controller::DeviceCommissioner * _cppCommissioner;
chip::Credentials::PartialDACVerifier * _partialDACVerifier;
chip::Credentials::DefaultDACVerifier * _defaultDACVerifier;
MTRDeviceControllerDelegateBridge * _deviceControllerDelegateBridge;
MTROperationalCredentialsDelegate * _operationalCredentialsDelegate;
MTRP256KeypairBridge _signingKeypairBridge;
MTRP256KeypairBridge _operationalKeypairBridge;
MTRDeviceAttestationDelegateBridge * _deviceAttestationDelegateBridge;
MTRDeviceControllerFactory * _factory;
NSMutableDictionary * _nodeIDToDeviceMap;
os_unfair_lock _deviceMapLock; // protects nodeIDToDeviceMap
MTRCommissionableBrowser * _commissionableBrowser;
MTRAttestationTrustStoreBridge * _attestationTrustStoreBridge;
}

- (nullable instancetype)initWithParameters:(MTRDeviceControllerAbstractParameters *)parameters error:(NSError * __autoreleasing *)error
{
Expand Down Expand Up @@ -249,7 +245,7 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory

- (BOOL)isRunning
{
return self.cppCommissioner != nullptr;
return _cppCommissioner != nullptr;
}

- (void)shutdown
Expand All @@ -271,8 +267,8 @@ - (void)cleanupAfterStartup
// while calling out into arbitrary invalidation code, snapshot the list of
// devices before we start invalidating.
os_unfair_lock_lock(&_deviceMapLock);
NSArray<MTRDevice *> * devices = [self.nodeIDToDeviceMap allValues];
[self.nodeIDToDeviceMap removeAllObjects];
NSArray<MTRDevice *> * devices = [_nodeIDToDeviceMap allValues];
[_nodeIDToDeviceMap removeAllObjects];
os_unfair_lock_unlock(&_deviceMapLock);

for (MTRDevice * device in devices) {
Expand Down Expand Up @@ -590,7 +586,7 @@ - (BOOL)setupCommissioningSessionWithPayload:(MTRSetupPayload *)payload

chip::NodeId nodeId = [newNodeID unsignedLongLongValue];
self->_operationalCredentialsDelegate->SetDeviceID(nodeId);
auto errorCode = self.cppCommissioner->EstablishPASEConnection(nodeId, [pairingCode UTF8String]);
auto errorCode = self->_cppCommissioner->EstablishPASEConnection(nodeId, [pairingCode UTF8String]);
return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
};

Expand All @@ -612,7 +608,7 @@ - (BOOL)setupCommissioningSessionWithDiscoveredDevice:(MTRCommissionableBrowserR
auto pinCode = static_cast<uint32_t>(payload.setupPasscode.unsignedLongValue);
params.Value().SetSetupPINCode(pinCode);

errorCode = self.cppCommissioner->EstablishPASEConnection(nodeId, params.Value());
errorCode = self->_cppCommissioner->EstablishPASEConnection(nodeId, params.Value());
} else {
// Try to get a QR code if possible (because it has a better
// discriminator, etc), then fall back to manual code if that fails.
Expand All @@ -630,7 +626,7 @@ - (BOOL)setupCommissioningSessionWithDiscoveredDevice:(MTRCommissionableBrowserR
continue;
}

errorCode = self.cppCommissioner->EstablishPASEConnection(
errorCode = self->_cppCommissioner->EstablishPASEConnection(
nodeId, [pairingCode UTF8String], chip::Controller::DiscoveryType::kDiscoveryNetworkOnly, resolutionData);
if (CHIP_NO_ERROR != errorCode) {
break;
Expand Down Expand Up @@ -745,7 +741,7 @@ - (BOOL)commissionNodeWithID:(NSNumber *)nodeID

chip::NodeId deviceId = [nodeID unsignedLongLongValue];
self->_operationalCredentialsDelegate->SetDeviceID(deviceId);
auto errorCode = self.cppCommissioner->Commission(deviceId, params);
auto errorCode = self->_cppCommissioner->Commission(deviceId, params);
return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
};

Expand All @@ -762,7 +758,7 @@ - (BOOL)continueCommissioningDevice:(void *)device
: chip::Credentials::AttestationVerificationResult::kSuccess;

auto deviceProxy = static_cast<chip::DeviceProxy *>(device);
auto errorCode = self.cppCommissioner->ContinueCommissioningAfterDeviceAttestation(deviceProxy,
auto errorCode = self->_cppCommissioner->ContinueCommissioningAfterDeviceAttestation(deviceProxy,
ignoreAttestationFailure ? chip::Credentials::AttestationVerificationResult::kSuccess : lastAttestationResult);
return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
};
Expand All @@ -774,7 +770,7 @@ - (BOOL)cancelCommissioningForNodeID:(NSNumber *)nodeID error:(NSError * __autor
{
auto block = ^BOOL {
self->_operationalCredentialsDelegate->ResetDeviceID();
auto errorCode = self.cppCommissioner->StopPairing([nodeID unsignedLongLongValue]);
auto errorCode = self->_cppCommissioner->StopPairing([nodeID unsignedLongLongValue]);
return ![MTRDeviceController checkForError:errorCode logMsg:kErrorStopPairing error:error];
};

Expand All @@ -784,7 +780,7 @@ - (BOOL)cancelCommissioningForNodeID:(NSNumber *)nodeID error:(NSError * __autor
- (BOOL)startBrowseForCommissionables:(id<MTRCommissionableBrowserDelegate>)delegate queue:(dispatch_queue_t)queue
{
auto block = ^BOOL {
VerifyOrReturnValue(self.commissionableBrowser == nil, NO);
VerifyOrReturnValue(self->_commissionableBrowser == nil, NO);

auto commissionableBrowser = [[MTRCommissionableBrowser alloc] initWithDelegate:delegate controller:self queue:queue];
VerifyOrReturnValue([commissionableBrowser start], NO);
Expand All @@ -799,9 +795,9 @@ - (BOOL)startBrowseForCommissionables:(id<MTRCommissionableBrowserDelegate>)dele
- (BOOL)stopBrowseForCommissionables
{
auto block = ^BOOL {
VerifyOrReturnValue(self.commissionableBrowser != nil, NO);
VerifyOrReturnValue(self->_commissionableBrowser != nil, NO);

auto commissionableBrowser = self.commissionableBrowser;
auto commissionableBrowser = self->_commissionableBrowser;
VerifyOrReturnValue([commissionableBrowser stop], NO);

self->_commissionableBrowser = nil;
Expand Down Expand Up @@ -845,15 +841,15 @@ - (MTRBaseDevice *)baseDeviceForNodeID:(NSNumber *)nodeID
- (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID
{
os_unfair_lock_lock(&_deviceMapLock);
MTRDevice * deviceToReturn = self.nodeIDToDeviceMap[nodeID];
MTRDevice * deviceToReturn = _nodeIDToDeviceMap[nodeID];
if (!deviceToReturn) {
deviceToReturn = [[MTRDevice alloc] initWithNodeID:nodeID controller:self];
// If we're not running, don't add the device to our map. That would
// create a cycle that nothing would break. Just return the device,
// which will be in exactly the state it would be in if it were created
// while we were running and then we got shut down.
if ([self isRunning]) {
self.nodeIDToDeviceMap[nodeID] = deviceToReturn;
_nodeIDToDeviceMap[nodeID] = deviceToReturn;
}
}
os_unfair_lock_unlock(&_deviceMapLock);
Expand All @@ -864,12 +860,13 @@ - (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID
- (void)removeDevice:(MTRDevice *)device
{
os_unfair_lock_lock(&_deviceMapLock);
MTRDevice * deviceToRemove = self.nodeIDToDeviceMap[device.nodeID];
auto * nodeID = device.nodeID;
MTRDevice * deviceToRemove = _nodeIDToDeviceMap[nodeID];
if (deviceToRemove == device) {
[deviceToRemove invalidate];
self.nodeIDToDeviceMap[device.nodeID] = nil;
_nodeIDToDeviceMap[nodeID] = nil;
} else {
MTR_LOG_ERROR("Error: Cannot remove device %p with nodeID %llu", device, device.nodeID.unsignedLongLongValue);
MTR_LOG_ERROR("Error: Cannot remove device %p with nodeID %llu", device, nodeID.unsignedLongLongValue);
}
os_unfair_lock_unlock(&_deviceMapLock);
}
Expand Down Expand Up @@ -935,7 +932,7 @@ - (NSData * _Nullable)attestationChallengeForDeviceID:(NSNumber *)deviceID
auto block = ^NSData *
{
chip::CommissioneeDeviceProxy * deviceProxy;
auto errorCode = self.cppCommissioner->GetDeviceBeingCommissioned([deviceID unsignedLongLongValue], &deviceProxy);
auto errorCode = self->_cppCommissioner->GetDeviceBeingCommissioned([deviceID unsignedLongLongValue], &deviceProxy);
VerifyOrReturnValue(![MTRDeviceController checkForError:errorCode logMsg:kErrorGetCommissionee error:nil], nil);

uint8_t challengeBuffer[chip::Crypto::kAES_CCM128_Key_Length];
Expand Down Expand Up @@ -1098,7 +1095,7 @@ - (void)asyncGetCommissionerOnMatterQueue:(void (^)(chip::Controller::DeviceComm
return;
}

block(self.cppCommissioner);
block(self->_cppCommissioner);
});
}

Expand Down Expand Up @@ -1208,7 +1205,7 @@ - (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(&_deviceMapLock);
MTRDevice * device = self.nodeIDToDeviceMap[@(nodeID)];
MTRDevice * device = _nodeIDToDeviceMap[@(nodeID)];
os_unfair_lock_unlock(&_deviceMapLock);

if (device == nil) {
Expand Down Expand Up @@ -1400,7 +1397,7 @@ - (BOOL)pairDevice:(uint64_t)deviceID
VerifyOrReturnValue(![MTRDeviceController checkForError:errorCode logMsg:kErrorSetupCodeGen error:error], NO);

self->_operationalCredentialsDelegate->SetDeviceID(deviceID);
errorCode = self.cppCommissioner->EstablishPASEConnection(deviceID, manualPairingCode.c_str());
errorCode = self->_cppCommissioner->EstablishPASEConnection(deviceID, manualPairingCode.c_str());
return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
};

Expand All @@ -1421,7 +1418,7 @@ - (BOOL)pairDevice:(uint64_t)deviceID
self->_operationalCredentialsDelegate->SetDeviceID(deviceID);

auto params = chip::RendezvousParameters().SetSetupPINCode(setupPINCode).SetPeerAddress(peerAddress);
auto errorCode = self.cppCommissioner->EstablishPASEConnection(deviceID, params);
auto errorCode = self->_cppCommissioner->EstablishPASEConnection(deviceID, params);
return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
};

Expand All @@ -1432,7 +1429,7 @@ - (BOOL)pairDevice:(uint64_t)deviceID onboardingPayload:(NSString *)onboardingPa
{
auto block = ^BOOL {
self->_operationalCredentialsDelegate->SetDeviceID(deviceID);
auto errorCode = self.cppCommissioner->EstablishPASEConnection(deviceID, [onboardingPayload UTF8String]);
auto errorCode = self->_cppCommissioner->EstablishPASEConnection(deviceID, [onboardingPayload UTF8String]);
return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
};

Expand Down Expand Up @@ -1468,7 +1465,7 @@ - (BOOL)openPairingWindow:(uint64_t)deviceID duration:(NSUInteger)duration error

auto block = ^BOOL {
auto errorCode = chip::Controller::AutoCommissioningWindowOpener::OpenBasicCommissioningWindow(
self.cppCommissioner, deviceID, chip::System::Clock::Seconds16(static_cast<uint16_t>(duration)));
self->_cppCommissioner, deviceID, chip::System::Clock::Seconds16(static_cast<uint16_t>(duration)));
return ![MTRDeviceController checkForError:errorCode logMsg:kErrorOpenPairingWindow error:error];
};

Expand Down Expand Up @@ -1508,7 +1505,7 @@ - (NSString *)openPairingWindowWithPIN:(uint64_t)deviceID
auto block = ^NSString *
{
chip::SetupPayload setupPayload;
auto errorCode = chip::Controller::AutoCommissioningWindowOpener::OpenCommissioningWindow(self.cppCommissioner, deviceID,
auto errorCode = chip::Controller::AutoCommissioningWindowOpener::OpenCommissioningWindow(self->_cppCommissioner, deviceID,
chip::System::Clock::Seconds16(static_cast<uint16_t>(duration)), chip::Crypto::kSpake2p_Min_PBKDF_Iterations,
static_cast<uint16_t>(discriminator), chip::MakeOptional(static_cast<uint32_t>(setupPIN)), chip::NullOptional,
setupPayload);
Expand Down
13 changes: 4 additions & 9 deletions src/darwin/Framework/CHIP/MTRThreadOperationalDataset.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,9 @@
size_t const MTRSizeThreadPSKc = chip::Thread::kSizePSKc;
size_t const MTRSizeThreadPANID = 2; // Thread's PAN ID is 2 bytes

@interface MTRThreadOperationalDataset ()

@property (readonly) chip::Thread::OperationalDataset cppThreadOperationalDataset;
@property (nonatomic, copy) NSNumber * channelNumber;

@end

@implementation MTRThreadOperationalDataset
@implementation MTRThreadOperationalDataset {
chip::Thread::OperationalDataset _cppThreadOperationalDataset;
}

- (instancetype _Nullable)initWithNetworkName:(NSString *)networkName
extendedPANID:(NSData *)extendedPANID
Expand Down Expand Up @@ -157,7 +152,7 @@ @implementation MTRThreadOperationalDataset (Deprecated)

- (void)setChannel:(uint16_t)channel
{
self.channelNumber = @(channel);
_channelNumber = @(channel);
}

- (uint16_t)channel
Expand Down

0 comments on commit b7e3bc0

Please sign in to comment.