Skip to content

Commit

Permalink
Address API review issues in MTRDeviceController.
Browse files Browse the repository at this point in the history 
This is a re-landing of PR project-chip#22596 but with changes made for backwards compat.

* Make MTRBaseDevice creation synchronous.  This requires updates to
  MTRBaseDeviceOverXPC to do the possible async getting of the controller id it
  needs during its async operations, not when getting the device object.
* Deprecate pairDevice methods.
* Rename "commissionDevice" to "commissionNodeWithID".
* Rename "stopDevicePairing" to "cancelCommissioningForNodeID" and document.
* Rename "getDeviceBeingCommissioned" to "getDeviceBeingCommissionedWithNodeID".
* Various documentation improvements.
* Fix signature of computePaseVerifier.

* Address review comments.
  • Loading branch information
@bzbarsky-apple
bzbarsky-apple committed Nov 4, 2022
1 parent 092c91c commit 2063606
Show file tree
Hide file tree
Showing 19 changed files with 1,246 additions and 1,279 deletions.
28 changes: 6 additions & 22 deletions examples/darwin-framework-tool/commands/clusters/ModelCommandBridge.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,31 +25,15 @@

CHIP_ERROR ModelCommand::RunCommand()
{
dispatch_queue_t callbackQueue = dispatch_queue_create("com.chip-tool.command", DISPATCH_QUEUE_SERIAL);

MTRDeviceController * commissioner = CurrentCommissioner();
ChipLogProgress(chipTool, "Sending command to node 0x" ChipLogFormatX64, ChipLogValueX64(mNodeId));
[commissioner getBaseDevice:mNodeId
queue:callbackQueue
completion:^(MTRBaseDevice * _Nullable device, NSError * _Nullable error) {
if (error != nil) {
SetCommandExitStatus(error, "Error getting connected device");
return;
}

CHIP_ERROR err;
if (device == nil) {
err = CHIP_ERROR_INTERNAL;
} else {
err = SendCommand(device, mEndPointId);
}
auto * device = [MTRBaseDevice deviceWithNodeID:@(mNodeId) controller:commissioner];
CHIP_ERROR err = SendCommand(device, mEndPointId);

if (err != CHIP_NO_ERROR) {
ChipLogError(chipTool, "Error: %s", chip::ErrorStr(err));
SetCommandExitStatus(err);
return;
}
}];
if (err != CHIP_NO_ERROR) {
ChipLogError(chipTool, "Error: %s", chip::ErrorStr(err));
return err;
}
return CHIP_NO_ERROR;
}

Expand Down
71 changes: 28 additions & 43 deletions examples/darwin-framework-tool/commands/pairing/PairingCommandBridge.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,47 +101,32 @@
{
dispatch_queue_t callbackQueue = dispatch_queue_create("com.chip-tool.command", DISPATCH_QUEUE_SERIAL);
MTRDeviceController * commissioner = CurrentCommissioner();
[commissioner
getBaseDevice:mNodeId
queue:callbackQueue
completion:^(MTRBaseDevice * _Nullable device, NSError * _Nullable error) {
CHIP_ERROR err = CHIP_NO_ERROR;
if (error) {
err = MTRErrorToCHIPErrorCode(error);
LogNSError("Error: ", error);
SetCommandExitStatus(err);
} else if (device == nil) {
ChipLogError(chipTool, "Error: %s", chip::ErrorStr(CHIP_ERROR_INTERNAL));
SetCommandExitStatus(CHIP_ERROR_INTERNAL);
} else {
ChipLogProgress(chipTool, "Attempting to unpair device %llu", mNodeId);
MTRBaseClusterOperationalCredentials * opCredsCluster =
[[MTRBaseClusterOperationalCredentials alloc] initWithDevice:device endpointID:@(0) queue:callbackQueue];
[opCredsCluster
readAttributeCurrentFabricIndexWithCompletion:^(NSNumber * _Nullable value, NSError * _Nullable readError) {
if (readError) {
CHIP_ERROR readErr = MTRErrorToCHIPErrorCode(readError);
LogNSError("Failed to get current fabric: ", readError);
SetCommandExitStatus(readErr);
return;
}
MTROperationalCredentialsClusterRemoveFabricParams * params =
[[MTROperationalCredentialsClusterRemoveFabricParams alloc] init];
params.fabricIndex = value;
[opCredsCluster
removeFabricWithParams:params
completion:^(MTROperationalCredentialsClusterNOCResponseParams * _Nullable data,
NSError * _Nullable removeError) {
CHIP_ERROR removeErr = CHIP_NO_ERROR;
if (removeError) {
removeErr = MTRErrorToCHIPErrorCode(removeError);
LogNSError("Failed to remove current fabric: ", removeError);
} else {
ChipLogProgress(chipTool, "Successfully unpaired deviceId %llu", mNodeId);
}
SetCommandExitStatus(removeErr);
}];
}];
}
}];
auto * device = [MTRBaseDevice deviceWithNodeID:@(mNodeId) controller:commissioner];

ChipLogProgress(chipTool, "Attempting to unpair device %llu", mNodeId);
MTRBaseClusterOperationalCredentials * opCredsCluster =
[[MTRBaseClusterOperationalCredentials alloc] initWithDevice:device endpoint:@(0) queue:callbackQueue];
[opCredsCluster readAttributeCurrentFabricIndexWithCompletion:^(NSNumber * _Nullable value, NSError * _Nullable readError) {
if (readError) {
CHIP_ERROR readErr = MTRErrorToCHIPErrorCode(readError);
LogNSError("Failed to get current fabric: ", readError);
SetCommandExitStatus(readErr);
return;
}
MTROperationalCredentialsClusterRemoveFabricParams * params =
[[MTROperationalCredentialsClusterRemoveFabricParams alloc] init];
params.fabricIndex = value;
[opCredsCluster removeFabricWithParams:params
completion:^(MTROperationalCredentialsClusterNOCResponseParams * _Nullable data,
NSError * _Nullable removeError) {
CHIP_ERROR removeErr = CHIP_NO_ERROR;
if (removeError) {
removeErr = MTRErrorToCHIPErrorCode(removeError);
LogNSError("Failed to remove current fabric: ", removeError);
} else {
ChipLogProgress(chipTool, "Successfully unpaired deviceId %llu", mNodeId);
}
SetCommandExitStatus(removeErr);
}];
}];
}
2 changes: 1 addition & 1 deletion examples/darwin-framework-tool/commands/pairing/PairingDelegateBridge.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ - (void)onPairingComplete:(NSError *)error
ChipLogProgress(chipTool, "Pairing Success");
ChipLogProgress(chipTool, "PASE establishment successful");
NSError * commissionError;
[_commissioner commissionDevice:_deviceID commissioningParams:_params error:&commissionError];
[_commissioner commissionNodeWithID:@(_deviceID) commissioningParams:_params error:&commissionError];
if (commissionError != nil) {
_commandBridge->SetCommandExitStatus(commissionError);
return;
Expand Down
26 changes: 11 additions & 15 deletions examples/darwin-framework-tool/commands/tests/TestCommandBridge.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,27 +156,21 @@ class TestCommandBridge : public CHIPCommandBridge,

SetIdentity(identity);

// Invalidate our existing CASE session; otherwise getConnectedDevice
// will just hand it right back to us without establishing a new CASE
// session when a reboot is done on the server.
// Invalidate our existing CASE session; otherwise trying to work with
// our device will just reuse it without establishing a new CASE
// session when a reboot is done on the server, and then our next
// interaction will time out.
if (value.expireExistingSession.ValueOr(true)) {
if (GetDevice(identity) != nil) {
[GetDevice(identity) invalidateCASESession];
mConnectedDevices[identity] = nil;
}
}

[controller getBaseDevice:value.nodeId
queue:mCallbackQueue
completion:^(MTRBaseDevice * _Nullable device, NSError * _Nullable error) {
if (error != nil) {
SetCommandExitStatus(error);
return;
}

mConnectedDevices[identity] = device;
NextTest();
}];
mConnectedDevices[identity] = [MTRBaseDevice deviceWithNodeID:@(value.nodeId) controller:controller];
dispatch_async(mCallbackQueue, ^{
NextTest();
});
return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -238,7 +232,9 @@ class TestCommandBridge : public CHIPCommandBridge,
VerifyOrReturn(commissioner != nil, Exit("No current commissioner"));

NSError * commissionError = nil;
[commissioner commissionDevice:nodeId commissioningParams:[[MTRCommissioningParameters alloc] init] error:&commissionError];
[commissioner commissionNodeWithID:@(nodeId)
commissioningParams:[[MTRCommissioningParameters alloc] init]
error:&commissionError];
CHIP_ERROR err = MTRErrorToCHIPErrorCode(commissionError);
if (err != CHIP_NO_ERROR) {
Exit("Failed to kick off commissioning", err);
Expand Down
9 changes: 9 additions & 0 deletions src/darwin/Framework/CHIP/MTRBaseDevice.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#import <Matter/MTRCluster.h>

@class MTRSetupPayload;
@class MTRDeviceController;

NS_ASSUME_NONNULL_BEGIN

Expand Down Expand Up @@ -125,6 +126,14 @@ extern NSString * const MTRArrayValueType;
- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;

/**
* Create a device object with the given node id and controller. This
* will always succeed, even if there is no such node id on the controller's
* fabric, but attempts to actually use the MTRBaseDevice will fail
* (asynchronously) in that case.
*/
+ (instancetype)deviceWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller;

/**
* Subscribe to receive attribute reports for everything (all endpoints, all
* clusters, all attributes, all events) on the device.
Expand Down
7 changes: 7 additions & 0 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,13 @@ - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControlle
return self;
}

+ (instancetype)deviceWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller
{
// Indirect through the controller to give it a chance to create an
// MTRBaseDeviceOverXPC instead of just an MTRBaseDevice.
return [controller baseDeviceForNodeID:nodeID];
}

- (void)invalidateCASESession
{
if (self.isPASEDevice) {
Expand Down
139 changes: 75 additions & 64 deletions src/darwin/Framework/CHIP/MTRDeviceController.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

NS_ASSUME_NONNULL_BEGIN

MTR_NEWLY_DEPRECATED("Please use MTRBaseDevice deviceWithNodeID")
typedef void (^MTRDeviceConnectionCallback)(MTRBaseDevice * _Nullable device, NSError * _Nullable error);

@class MTRCommissioningParameters;
Expand All @@ -31,7 +32,10 @@ typedef void (^MTRDeviceConnectionCallback)(MTRBaseDevice * _Nullable device, NS

@interface MTRDeviceController : NSObject

@property (readonly, nonatomic) BOOL isRunning;
/**
* If true, the controller has not been shut down yet.
*/
@property (readonly, nonatomic, getter=isRunning) BOOL running;

/**
* Return the Node ID assigned to the controller. Will return nil if the
Expand Down Expand Up @@ -73,70 +77,29 @@ typedef void (^MTRDeviceConnectionCallback)(MTRBaseDevice * _Nullable device, NS
API_AVAILABLE(ios(16.2), macos(13.1), watchos(9.2), tvos(16.2));

/**
* Start pairing for a device with the given ID, using the provided setup PIN
* to establish a PASE connection.
*
* The IP and port for the device will be discovered automatically based on the
* provided discriminator.
*
* The pairing process will proceed until a PASE session is established or an
* error occurs, then notify onPairingComplete on the MTRDevicePairingDelegate
* for this controller. That delegate is expected to call commissionDevice
* after that point if it wants to commission the device.
*/
- (BOOL)pairDevice:(uint64_t)deviceID
discriminator:(uint16_t)discriminator
setupPINCode:(uint32_t)setupPINCode
error:(NSError * __autoreleasing *)error;

/**
* Start pairing for a device with the given ID, using the provided IP address
* and port to connect to the device and the provided setup PIN to establish a
* PASE connection.
*
* The pairing process will proceed until a PASE session is established or an
* error occurs, then notify onPairingComplete on the MTRDevicePairingDelegate
* for this controller. That delegate is expected to call commissionDevice
* after that point if it wants to commission the device.
*/
- (BOOL)pairDevice:(uint64_t)deviceID
address:(NSString *)address
port:(uint16_t)port
setupPINCode:(uint32_t)setupPINCode
error:(NSError * __autoreleasing *)error;

/**
* Start pairing for a device with the given ID and onboarding payload (QR code
* or manual setup code). The payload will be used to discover the device and
* establish a PASE connection.
*
* The pairing process will proceed until a PASE session is established or an
* error occurs, then notify onPairingComplete on the MTRDevicePairingDelegate
* for this controller. That delegate is expected to call commissionDevice
* after that point if it wants to commission the device.
* Commission the node with the given node ID. The node ID must match the node
* ID that was used to set up the commissioning session.
*/
- (BOOL)pairDevice:(uint64_t)deviceID onboardingPayload:(NSString *)onboardingPayload error:(NSError * __autoreleasing *)error;
- (BOOL)commissionDevice:(uint64_t)deviceId
commissioningParams:(MTRCommissioningParameters *)commissioningParams
error:(NSError * __autoreleasing *)error;
- (BOOL)commissionNodeWithID:(NSNumber *)nodeID
commissioningParams:(MTRCommissioningParameters *)commissioningParams
error:(NSError * __autoreleasing *)error MTR_NEWLY_AVAILABLE;

- (BOOL)continueCommissioningDevice:(void *)device
ignoreAttestationFailure:(BOOL)ignoreAttestationFailure
error:(NSError * __autoreleasing *)error;

- (BOOL)stopDevicePairing:(uint64_t)deviceID error:(NSError * __autoreleasing *)error;

- (nullable MTRBaseDevice *)getDeviceBeingCommissioned:(uint64_t)deviceId error:(NSError * __autoreleasing *)error;
- (BOOL)getBaseDevice:(uint64_t)deviceID
queue:(dispatch_queue_t)queue
completion:(MTRDeviceConnectionCallback)completion MTR_NEWLY_AVAILABLE;
/**
* Cancel commissioning for the given node id. This will shut down any existing
* commissioning session for that node id.
*/
- (BOOL)cancelCommissioningForNodeID:(NSNumber *)nodeID error:(NSError * __autoreleasing *)error MTR_NEWLY_AVAILABLE;

- (BOOL)openPairingWindow:(uint64_t)deviceID duration:(NSUInteger)duration error:(NSError * __autoreleasing *)error;
- (nullable NSString *)openPairingWindowWithPIN:(uint64_t)deviceID
duration:(NSUInteger)duration
discriminator:(NSUInteger)discriminator
setupPIN:(NSUInteger)setupPIN
error:(NSError * __autoreleasing *)error;
/**
* Get an MTRBaseDevice for a commissioning session that was set up for the
* given node ID. Returns nil if no such commissioning session is available.
*/
- (nullable MTRBaseDevice *)deviceBeingCommissionedWithNodeID:(NSNumber *)nodeID
error:(NSError * __autoreleasing *)error MTR_NEWLY_AVAILABLE;

/**
* Controllers are created via the MTRControllerFactory object.
Expand Down Expand Up @@ -176,19 +139,24 @@ typedef void (^MTRDeviceConnectionCallback)(MTRBaseDevice * _Nullable device, NS
- (NSData * _Nullable)attestationChallengeForDeviceID:(NSNumber *)deviceID MTR_NEWLY_AVAILABLE;

/**
* Compute a PASE verifier and passcode ID for the desired setup pincode.
* Compute a PASE verifier for the desired setup passcode.
*
* @param[in] setupPincode The desired PIN code to use
* @param[in] iterations The number of iterations to use when generating the verifier
* @param[in] salt The 16-byte salt for verifier computation
* @param[in] setupPasscode The desired passcode to use.
* @param[in] iterations The number of iterations to use when generating the verifier.
* @param[in] salt The 16-byte salt for verifier computation.
*
* Returns nil on errors (e.g. salt has the wrong size), otherwise the computed
* verifier bytes.
*/
- (nullable NSData *)computePaseVerifier:(uint32_t)setupPincode iterations:(uint32_t)iterations salt:(NSData *)salt;
+ (nullable NSData *)computePASEVerifierForSetupPasscode:(NSNumber *)setupPasscode
iterations:(NSNumber *)iterations
salt:(NSData *)salt
error:(NSError * __autoreleasing *)error MTR_NEWLY_AVAILABLE;

/**
* Shutdown the controller. Calls to shutdown after the first one are NO-OPs.
* Shut down the controller. Calls to shutdown after the first one are NO-OPs.
* This must be called, either directly or via shutting down the
* MTRDeviceControllerFactory, to avoid leaking the controller.
*/
- (void)shutdown;

Expand All @@ -206,6 +174,49 @@ typedef void (^MTRDeviceConnectionCallback)(MTRBaseDevice * _Nullable device, NS
completionHandler:(MTRDeviceConnectionCallback)completionHandler
MTR_NEWLY_DEPRECATED("Please use [MTRBaseDevice deviceWithNodeID:controller:]");

- (BOOL)pairDevice:(uint64_t)deviceID
discriminator:(uint16_t)discriminator
setupPINCode:(uint32_t)setupPINCode
error:(NSError * __autoreleasing *)error
MTR_NEWLY_DEPRECATED("Please use setupCommissioningSessionWithPayload:newNodeID:error:");
- (BOOL)pairDevice:(uint64_t)deviceID
address:(NSString *)address
port:(uint16_t)port
setupPINCode:(uint32_t)setupPINCode
error:(NSError * __autoreleasing *)error
MTR_NEWLY_DEPRECATED("Please use setupCommissioningSessionWithPayload:newNodeID:error:");
- (BOOL)pairDevice:(uint64_t)deviceID
onboardingPayload:(NSString *)onboardingPayload
error:(NSError * __autoreleasing *)error
MTR_NEWLY_DEPRECATED("Please use setupCommissioningSessionWithPayload:newNodeID:error:");
- (BOOL)commissionDevice:(uint64_t)deviceId
commissioningParams:(MTRCommissioningParameters *)commissioningParams
error:(NSError * __autoreleasing *)error
MTR_NEWLY_DEPRECATED("Please use commissionNodeWithID:commissioningParams:error:");

- (BOOL)stopDevicePairing:(uint64_t)deviceID
error:(NSError * __autoreleasing *)error MTR_NEWLY_DEPRECATED("Please use cancelCommissioningForNodeID:error:");

- (nullable MTRBaseDevice *)getDeviceBeingCommissioned:(uint64_t)deviceId
error:(NSError * __autoreleasing *)error
MTR_NEWLY_DEPRECATED("Please use deviceBeingCommissionedWithNodeID:error:");

- (BOOL)openPairingWindow:(uint64_t)deviceID
duration:(NSUInteger)duration
error:(NSError * __autoreleasing *)error
MTR_NEWLY_DEPRECATED("Please use MTRDevice or MTRBaseDevice openCommissioningWindowWithSetupPasscode");
- (nullable NSString *)openPairingWindowWithPIN:(uint64_t)deviceID
duration:(NSUInteger)duration
discriminator:(NSUInteger)discriminator
setupPIN:(NSUInteger)setupPIN
error:(NSError * __autoreleasing *)error
MTR_NEWLY_DEPRECATED("Please use MTRDevice or MTRBaseDevice openCommissioningWindowWithSetupPasscode");

- (nullable NSData *)computePaseVerifier:(uint32_t)setupPincode
iterations:(uint32_t)iterations
salt:(NSData *)salt
MTR_NEWLY_DEPRECATED("Please use computePASEVerifierForSetupPasscode:iterations:salt:error:");

@end

NS_ASSUME_NONNULL_END
Loading

0 comments on commit 2063606

Please sign in to comment.