Skip to content

Commit 489bcf9

Browse files
bzbarsky-applebzbarsky-apple
committed
Address API review issues in MTRDeviceController.
* 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. * Rename "pairDevice" to "setupCommissioningSessionWithPayload", fix its signature, improve documentation. * Rename "commissionDevice" to "commissionNodeWithID". * Rename "stopDevicePairing" to "cancelCommissioningForNodeID" and document. * Rename "getDeviceBeingCommissioned" to "getDeviceBeingCommissionedWithNodeID". * Various documentation improvements. * Add a way to generate a QR code from an MTRSetupPayload to allow correct recovery of long discriminators in setupCommissioningSessionWithPayload. * Fix signature of computePaseVerifier. Fixes #22533 Addresses part of #22420
1 parent 9c131da commit 489bcf9

29 files changed

+1127
-1167
lines changed

‎examples/darwin-framework-tool/commands/clusters/ModelCommandBridge.mm

+6-22
Original file line numberDiff line numberDiff line change
@@ -25,31 +25,15 @@
2525

2626
CHIP_ERROR ModelCommand::RunCommand()
2727
{
28-
dispatch_queue_t callbackQueue = dispatch_queue_create("com.chip-tool.command", DISPATCH_QUEUE_SERIAL);
29-
3028
MTRDeviceController * commissioner = CurrentCommissioner();
3129
ChipLogProgress(chipTool, "Sending command to node 0x" ChipLogFormatX64, ChipLogValueX64(mNodeId));
32-
[commissioner getBaseDevice:mNodeId
33-
queue:callbackQueue
34-
completion:^(MTRBaseDevice * _Nullable device, NSError * _Nullable error) {
35-
if (error != nil) {
36-
SetCommandExitStatus(error, "Error getting connected device");
37-
return;
38-
}
39-
40-
CHIP_ERROR err;
41-
if (device == nil) {
42-
err = CHIP_ERROR_INTERNAL;
43-
} else {
44-
err = SendCommand(device, mEndPointId);
45-
}
30+
auto * device = [MTRBaseDevice deviceWithNodeID:@(mNodeId) controller:commissioner];
31+
CHIP_ERROR err = SendCommand(device, mEndPointId);
4632

47-
if (err != CHIP_NO_ERROR) {
48-
ChipLogError(chipTool, "Error: %s", chip::ErrorStr(err));
49-
SetCommandExitStatus(err);
50-
return;
51-
}
52-
}];
33+
if (err != CHIP_NO_ERROR) {
34+
ChipLogError(chipTool, "Error: %s", chip::ErrorStr(err));
35+
return err;
36+
}
5337
return CHIP_NO_ERROR;
5438
}
5539

‎examples/darwin-framework-tool/commands/pairing/Commands.h

+7-10
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,6 @@ class PairCodeThread : public PairingCommandBridge
4141
PairCodeThread() : PairingCommandBridge("code-thread", PairingMode::Code, PairingNetworkType::Thread) {}
4242
};
4343

44-
class PairWithIPAddress : public PairingCommandBridge
45-
{
46-
public:
47-
PairWithIPAddress() : PairingCommandBridge("ethernet", PairingMode::Ethernet, PairingNetworkType::Ethernet) {}
48-
};
49-
5044
class PairBleWiFi : public PairingCommandBridge
5145
{
5246
public:
@@ -70,10 +64,13 @@ void registerCommandsPairing(Commands & commands)
7064
const char * clusterName = "Pairing";
7165

7266
commands_list clusterCommands = {
73-
make_unique<PairCode>(), make_unique<PairWithIPAddress>(),
74-
make_unique<PairCodeWifi>(), make_unique<PairCodeThread>(),
75-
make_unique<PairBleWiFi>(), make_unique<PairBleThread>(),
76-
make_unique<Unpair>(), make_unique<OpenCommissioningWindowCommand>(),
67+
make_unique<PairCode>(),
68+
make_unique<PairCodeWifi>(),
69+
make_unique<PairCodeThread>(),
70+
make_unique<PairBleWiFi>(),
71+
make_unique<PairBleThread>(),
72+
make_unique<Unpair>(),
73+
make_unique<OpenCommissioningWindowCommand>(),
7774
};
7875

7976
commands.Register(clusterName, clusterCommands);

‎examples/darwin-framework-tool/commands/pairing/OpenCommissioningWindowCommand.mm

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
{
2626
mWorkQueue = dispatch_queue_create("com.chip.open_commissioning_window", DISPATCH_QUEUE_SERIAL);
2727
auto * controller = CurrentCommissioner();
28-
auto * device = [[MTRBaseDevice alloc] initWithNodeID:@(mNodeId) controller:controller];
28+
auto * device = [MTRBaseDevice deviceWithNodeID:@(mNodeId) controller:controller];
2929

3030
auto * self = this;
3131
if (mCommissioningWindowOption == 0) {

‎examples/darwin-framework-tool/commands/pairing/PairingCommandBridge.h

-10
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ enum class PairingMode
2424
{
2525
None,
2626
Code,
27-
Ethernet,
2827
Ble,
2928
};
3029

@@ -64,12 +63,6 @@ class PairingCommandBridge : public CHIPCommandBridge
6463
case PairingMode::Code:
6564
AddArgument("payload", &mOnboardingPayload);
6665
break;
67-
case PairingMode::Ethernet:
68-
AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
69-
AddArgument("discriminator", 0, 4096, &mDiscriminator);
70-
AddArgument("device-remote-ip", &ipAddress);
71-
AddArgument("device-remote-port", 0, UINT16_MAX, &mRemotePort);
72-
break;
7366
case PairingMode::Ble:
7467
AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
7568
AddArgument("discriminator", 0, 4096, &mDiscriminator);
@@ -84,7 +77,6 @@ class PairingCommandBridge : public CHIPCommandBridge
8477
private:
8578
void PairWithCode(NSError * __autoreleasing * error);
8679
void PairWithPayload(NSError * __autoreleasing * error);
87-
void PairWithIPAddress(NSError * __autoreleasing * error);
8880
void Unpair();
8981
void SetUpPairingDelegate();
9082

@@ -94,9 +86,7 @@ class PairingCommandBridge : public CHIPCommandBridge
9486
chip::ByteSpan mSSID;
9587
chip::ByteSpan mPassword;
9688
chip::NodeId mNodeId;
97-
uint16_t mRemotePort;
9889
uint16_t mDiscriminator;
9990
uint32_t mSetupPINCode;
10091
char * mOnboardingPayload;
101-
char * ipAddress;
10292
};

‎examples/darwin-framework-tool/commands/pairing/PairingCommandBridge.mm

+36-61
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,6 @@
6666
case PairingMode::Code:
6767
PairWithPayload(&error);
6868
break;
69-
case PairingMode::Ethernet:
70-
PairWithIPAddress(&error);
71-
break;
7269
case PairingMode::Ble:
7370
PairWithCode(&error);
7471
break;
@@ -83,75 +80,53 @@
8380
void PairingCommandBridge::PairWithCode(NSError * __autoreleasing * error)
8481
{
8582
SetUpPairingDelegate();
83+
auto * payload = [[MTRSetupPayload alloc] initWithSetupPasscode:@(mSetupPINCode) discriminator:@(mDiscriminator)];
8684
MTRDeviceController * commissioner = CurrentCommissioner();
87-
[commissioner pairDevice:mNodeId discriminator:mDiscriminator setupPINCode:mSetupPINCode error:error];
85+
[commissioner setupCommissioningSessionWithPayload:payload newNodeID:@(mNodeId) error:error];
8886
}
8987

9088
void PairingCommandBridge::PairWithPayload(NSError * __autoreleasing * error)
9189
{
92-
NSString * payload = [NSString stringWithUTF8String:mOnboardingPayload];
93-
94-
SetUpPairingDelegate();
95-
MTRDeviceController * commissioner = CurrentCommissioner();
96-
[commissioner pairDevice:mNodeId onboardingPayload:payload error:error];
97-
}
98-
99-
void PairingCommandBridge::PairWithIPAddress(NSError * __autoreleasing * error)
100-
{
90+
NSString * onboardingPayload = [NSString stringWithUTF8String:mOnboardingPayload];
10191
SetUpPairingDelegate();
92+
auto * payload = [MTRSetupPayload setupPayloadWithOnboardingPayload:onboardingPayload error:error];
93+
if (payload == nil) {
94+
return;
95+
}
10296
MTRDeviceController * commissioner = CurrentCommissioner();
103-
[commissioner pairDevice:mNodeId
104-
address:[NSString stringWithUTF8String:ipAddress]
105-
port:mRemotePort
106-
setupPINCode:mSetupPINCode
107-
error:error];
97+
[commissioner setupCommissioningSessionWithPayload:payload newNodeID:@(mNodeId) error:error];
10898
}
10999

110100
void PairingCommandBridge::Unpair()
111101
{
112102
dispatch_queue_t callbackQueue = dispatch_queue_create("com.chip-tool.command", DISPATCH_QUEUE_SERIAL);
113103
MTRDeviceController * commissioner = CurrentCommissioner();
114-
[commissioner
115-
getBaseDevice:mNodeId
116-
queue:callbackQueue
117-
completion:^(MTRBaseDevice * _Nullable device, NSError * _Nullable error) {
118-
CHIP_ERROR err = CHIP_NO_ERROR;
119-
if (error) {
120-
err = MTRErrorToCHIPErrorCode(error);
121-
LogNSError("Error: ", error);
122-
SetCommandExitStatus(err);
123-
} else if (device == nil) {
124-
ChipLogError(chipTool, "Error: %s", chip::ErrorStr(CHIP_ERROR_INTERNAL));
125-
SetCommandExitStatus(CHIP_ERROR_INTERNAL);
126-
} else {
127-
ChipLogProgress(chipTool, "Attempting to unpair device %llu", mNodeId);
128-
MTRBaseClusterOperationalCredentials * opCredsCluster =
129-
[[MTRBaseClusterOperationalCredentials alloc] initWithDevice:device endpoint:@(0) queue:callbackQueue];
130-
[opCredsCluster
131-
readAttributeCurrentFabricIndexWithCompletion:^(NSNumber * _Nullable value, NSError * _Nullable readError) {
132-
if (readError) {
133-
CHIP_ERROR readErr = MTRErrorToCHIPErrorCode(readError);
134-
LogNSError("Failed to get current fabric: ", readError);
135-
SetCommandExitStatus(readErr);
136-
return;
137-
}
138-
MTROperationalCredentialsClusterRemoveFabricParams * params =
139-
[[MTROperationalCredentialsClusterRemoveFabricParams alloc] init];
140-
params.fabricIndex = value;
141-
[opCredsCluster
142-
removeFabricWithParams:params
143-
completion:^(MTROperationalCredentialsClusterNOCResponseParams * _Nullable data,
144-
NSError * _Nullable removeError) {
145-
CHIP_ERROR removeErr = CHIP_NO_ERROR;
146-
if (removeError) {
147-
removeErr = MTRErrorToCHIPErrorCode(removeError);
148-
LogNSError("Failed to remove current fabric: ", removeError);
149-
} else {
150-
ChipLogProgress(chipTool, "Successfully unpaired deviceId %llu", mNodeId);
151-
}
152-
SetCommandExitStatus(removeErr);
153-
}];
154-
}];
155-
}
156-
}];
104+
auto * device = [MTRBaseDevice deviceWithNodeID:@(mNodeId) controller:commissioner];
105+
106+
ChipLogProgress(chipTool, "Attempting to unpair device %llu", mNodeId);
107+
MTRBaseClusterOperationalCredentials * opCredsCluster =
108+
[[MTRBaseClusterOperationalCredentials alloc] initWithDevice:device endpoint:@(0) queue:callbackQueue];
109+
[opCredsCluster readAttributeCurrentFabricIndexWithCompletion:^(NSNumber * _Nullable value, NSError * _Nullable readError) {
110+
if (readError) {
111+
CHIP_ERROR readErr = MTRErrorToCHIPErrorCode(readError);
112+
LogNSError("Failed to get current fabric: ", readError);
113+
SetCommandExitStatus(readErr);
114+
return;
115+
}
116+
MTROperationalCredentialsClusterRemoveFabricParams * params =
117+
[[MTROperationalCredentialsClusterRemoveFabricParams alloc] init];
118+
params.fabricIndex = value;
119+
[opCredsCluster removeFabricWithParams:params
120+
completion:^(MTROperationalCredentialsClusterNOCResponseParams * _Nullable data,
121+
NSError * _Nullable removeError) {
122+
CHIP_ERROR removeErr = CHIP_NO_ERROR;
123+
if (removeError) {
124+
removeErr = MTRErrorToCHIPErrorCode(removeError);
125+
LogNSError("Failed to remove current fabric: ", removeError);
126+
} else {
127+
ChipLogProgress(chipTool, "Successfully unpaired deviceId %llu", mNodeId);
128+
}
129+
SetCommandExitStatus(removeErr);
130+
}];
131+
}];
157132
}

‎examples/darwin-framework-tool/commands/pairing/PairingDelegateBridge.mm

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ - (void)onPairingComplete:(NSError *)error
5454
ChipLogProgress(chipTool, "Pairing Success");
5555
ChipLogProgress(chipTool, "PASE establishment successful");
5656
NSError * commissionError;
57-
[_commissioner commissionDevice:_deviceID commissioningParams:_params error:&commissionError];
57+
[_commissioner commissionNodeWithID:@(_deviceID) commissioningParams:_params error:&commissionError];
5858
if (commissionError != nil) {
5959
_commandBridge->SetCommandExitStatus(commissionError);
6060
return;

‎examples/darwin-framework-tool/commands/tests/TestCommandBridge.h

+14-16
Original file line numberDiff line numberDiff line change
@@ -156,27 +156,19 @@ class TestCommandBridge : public CHIPCommandBridge,
156156

157157
SetIdentity(identity);
158158

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

169-
[controller getBaseDevice:value.nodeId
170-
queue:mCallbackQueue
171-
completion:^(MTRBaseDevice * _Nullable device, NSError * _Nullable error) {
172-
if (error != nil) {
173-
SetCommandExitStatus(error);
174-
return;
175-
}
176-
177-
mConnectedDevices[identity] = device;
178-
NextTest();
179-
}];
170+
mConnectedDevices[identity] = [MTRBaseDevice deviceWithNodeID:@(value.nodeId) controller:controller];
171+
NextTest();
180172
return CHIP_NO_ERROR;
181173
}
182174

@@ -197,7 +189,11 @@ class TestCommandBridge : public CHIPCommandBridge,
197189
length:value.payload.size()
198190
encoding:NSUTF8StringEncoding];
199191
NSError * err;
200-
BOOL ok = [controller pairDevice:value.nodeId onboardingPayload:payloadStr error:&err];
192+
auto * payload = [MTRSetupPayload setupPayloadWithOnboardingPayload:payloadStr error:&err];
193+
if (err != nil) {
194+
return MTRErrorToCHIPErrorCode(err);
195+
}
196+
BOOL ok = [controller setupCommissioningSessionWithPayload:payload newNodeID:@(value.nodeId) error:&err];
201197
if (ok == YES) {
202198
return CHIP_NO_ERROR;
203199
}
@@ -234,7 +230,9 @@ class TestCommandBridge : public CHIPCommandBridge,
234230
VerifyOrReturn(commissioner != nil, Exit("No current commissioner"));
235231

236232
NSError * commissionError = nil;
237-
[commissioner commissionDevice:nodeId commissioningParams:[[MTRCommissioningParameters alloc] init] error:&commissionError];
233+
[commissioner commissionNodeWithID:@(nodeId)
234+
commissioningParams:[[MTRCommissioningParameters alloc] init]
235+
error:&commissionError];
238236
CHIP_ERROR err = MTRErrorToCHIPErrorCode(commissionError);
239237
if (err != CHIP_NO_ERROR) {
240238
Exit("Failed to kick off commissioning", err);

‎examples/darwin-framework-tool/main.mm

+11-9
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,15 @@
3030

3131
int main(int argc, const char * argv[])
3232
{
33-
Commands commands;
34-
registerCommandsPairing(commands);
35-
registerCommandsInteractive(commands);
36-
registerCommandsPayload(commands);
37-
registerClusterOtaSoftwareUpdateProviderInteractive(commands);
38-
registerCommandsStorage(commands);
39-
registerCommandsTests(commands);
40-
registerClusters(commands);
41-
return commands.Run(argc, (char **) argv);
33+
@autoreleasepool {
34+
Commands commands;
35+
registerCommandsPairing(commands);
36+
registerCommandsInteractive(commands);
37+
registerCommandsPayload(commands);
38+
registerClusterOtaSoftwareUpdateProviderInteractive(commands);
39+
registerCommandsStorage(commands);
40+
registerCommandsTests(commands);
41+
registerClusters(commands);
42+
return commands.Run(argc, (char **) argv);
43+
}
4244
}

‎scripts/tests/chiptest/lsan-mac-suppressions.txt

-11
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,5 @@ leak:drbg_bytes
1919
# TODO: OpenSSL ERR_get_state seems to leak.
2020
leak:ERR_get_state
2121

22-
# TODO: BLE initialization allocates some UUIDs and strings that seem to leak.
23-
leak:[BleConnection initWithDiscriminator:]
24-
leak:[CBXpcConnection initWithDelegate:queue:options:sessionType:]
25-
26-
# TODO: Figure out how we are managing to leak NSData while using ARC!
27-
leak:[CHIPToolKeypair signMessageECDSA_RAW:]
28-
29-
# TODO: Figure out how we are managing to leak NSData while using ARC, though
30-
# this may just be a leak deep inside the CFPreferences stuff somewhere.
31-
leak:[CHIPToolPersistentStorageDelegate storageDataForKey:]
32-
3322
# TODO: https://github.com/project-chip/connectedhomeip/issues/22333
3423
leak:[MTRBaseCluster* subscribeAttribute*WithMinInterval:maxInterval:params:subscriptionEstablished:reportHandler:]

‎src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ extern NSString * const kNetworkSSIDDefaultsKey;
2525
extern NSString * const kNetworkPasswordDefaultsKey;
2626
extern NSString * const kFabricIdKey;
2727

28+
typedef void (^DeviceConnectionCallback)(MTRBaseDevice * _Nullable device, NSError * _Nullable error);
29+
2830
MTRDeviceController * _Nullable InitializeMTR(void);
2931
MTRDeviceController * _Nullable MTRRestartController(MTRDeviceController * controller);
3032
id _Nullable MTRGetDomainValueForKey(NSString * domain, NSString * key);
@@ -36,8 +38,8 @@ uint64_t MTRGetLastPairedDeviceId(void);
3638
void MTRSetNextAvailableDeviceID(uint64_t id);
3739
void MTRSetDevicePaired(uint64_t id, BOOL paired);
3840
BOOL MTRIsDevicePaired(uint64_t id);
39-
BOOL MTRGetConnectedDevice(MTRDeviceConnectionCallback completionHandler);
40-
BOOL MTRGetConnectedDeviceWithID(uint64_t deviceId, MTRDeviceConnectionCallback completionHandler);
41+
BOOL MTRGetConnectedDevice(DeviceConnectionCallback completionHandler);
42+
BOOL MTRGetConnectedDeviceWithID(uint64_t deviceId, DeviceConnectionCallback completionHandler);
4143
void MTRUnpairDeviceWithID(uint64_t deviceId);
4244
MTRBaseDevice * _Nullable MTRGetDeviceBeingCommissioned(void);
4345

0 commit comments

Comments
 (0)