Skip to content

Commit

Permalink
Darwin API review fixes for MTRSetupPayload and MTROptionalQRCodeInfo (…
Browse files Browse the repository at this point in the history 
…#22566)

* Rename MTRDiscoveryCapabilitiesNone to MTRDiscoveryCapabilitiesUnknown to
  indicate capabilities unknown (e.g. manual setup code).
* In MTROptionalQRCodeInfo mark integerValue and stringValue as nullable and
  document they are mutually exclusive.
* In MTROptionalQRCodeInfo switch infoType to MTROptionalQRCodeInfoType, not
  NSNumber.
* Change rendezvousInformation to just be a MTRDiscoveryCapabilities value, not
  nullable NSNumber, with MTRDiscoveryCapabilitiesNone meaning unknown. When
  parsing QR code, if the value ends up as 0, reset it to
  MTRDiscoveryCapabilitiesOnNetwork.
* Rename setUpPINCode to setupPasscode.
* Mark serialNumber as nullable.
* Add setupPayloadWithOnboardingPayload class method on MTRSetupPayload.
* Stop exposing the various payload parser APIs from the framework;
  consumers should use setupPayloadWithOnboardingPayload.

Fixes #22543

Fixes #22539

Addresses part of #22420
  • Loading branch information
@bzbarsky-apple
bzbarsky-apple authored Sep 12, 2022
1 parent 4b38a4a commit 9ababd1
Show file tree
Hide file tree
Showing 10 changed files with 155 additions and 129 deletions.
40 changes: 18 additions & 22 deletions examples/darwin-framework-tool/commands/payload/SetupPayloadParseCommand.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
NSString * codeString = [NSString stringWithCString:mCode encoding:NSASCIIStringEncoding];
NSError * error;
MTRSetupPayload * payload;
payload = [MTROnboardingPayloadParser setupPayloadForOnboardingPayload:codeString error:&error];
payload = [MTRSetupPayload setupPayloadWithOnboardingPayload:codeString error:&error];
if (error) {
LogNSError("Error: ", error);
return CHIP_ERROR_INTERNAL;
Expand All @@ -78,37 +78,33 @@
NSLog(@"ProductID: %@", payload.productID);
NSLog(@"Custom flow: %lu (%@)", payload.commissioningFlow, CustomFlowString(payload.commissioningFlow));
{
if (payload.rendezvousInformation == nil) {
if (payload.discoveryCapabilities == MTRDiscoveryCapabilitiesUnknown) {
NSLog(@"Capabilities: UNKNOWN");
} else {
NSMutableString * humanFlags = [[NSMutableString alloc] init];

auto value = [payload.rendezvousInformation unsignedLongValue];
if (value == MTRDiscoveryCapabilitiesNone) {
[humanFlags appendString:@"NONE"];
} else {
if (value & MTRDiscoveryCapabilitiesSoftAP) {
[humanFlags appendString:@"SoftAP"];
}
if (value & MTRDiscoveryCapabilitiesBLE) {
if (!humanFlags) {
[humanFlags appendString:@", "];
}
[humanFlags appendString:@"BLE"];
auto value = payload.discoveryCapabilities;
if (value & MTRDiscoveryCapabilitiesSoftAP) {
[humanFlags appendString:@"SoftAP"];
}
if (value & MTRDiscoveryCapabilitiesBLE) {
if (!humanFlags) {
[humanFlags appendString:@", "];
}
if (value & MTRDiscoveryCapabilitiesOnNetwork) {
if (!humanFlags) {
[humanFlags appendString:@", "];
}
[humanFlags appendString:@"ON NETWORK"];
[humanFlags appendString:@"BLE"];
}
if (value & MTRDiscoveryCapabilitiesOnNetwork) {
if (!humanFlags) {
[humanFlags appendString:@", "];
}
[humanFlags appendString:@"ON NETWORK"];
}

NSLog(@"Capabilities: 0x%02lX (%@)", value, humanFlags);
}
}
NSLog(@"Discriminator: %@", payload.discriminator);
NSLog(@"Passcode: %@", payload.setUpPINCode);
NSLog(@"Passcode: %@", payload.setupPasscode);

if (payload.serialNumber) {
NSLog(@"SerialNumber: %@", payload.serialNumber);
Expand All @@ -120,8 +116,8 @@
return CHIP_ERROR_INTERNAL;
}
for (const MTROptionalQRCodeInfo * info : optionalVendorData) {
bool isTypeString = [info.infoType isEqual:@(MTROptionalQRCodeInfoTypeString)];
bool isTypeInt32 = [info.infoType isEqual:@(MTROptionalQRCodeInfoTypeInt32)];
bool isTypeString = (info.infoType == MTROptionalQRCodeInfoTypeString);
bool isTypeInt32 = (info.infoType == MTROptionalQRCodeInfoTypeInt32);
VerifyOrReturnError(isTypeString || isTypeInt32, CHIP_ERROR_INVALID_ARGUMENT);

if (isTypeString) {
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/CHIPTool/CHIPTool/View Controllers/Enumeration/EnumerateViewController.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ - (void)enumerate
for (NSNumber * endpoint in endpointsInUse) {
MTRBaseClusterDescriptor * descriptorCluster =
[[MTRBaseClusterDescriptor alloc] initWithDevice:device endpoint:endpoint queue:dispatch_get_main_queue()];
[descriptorCluster readAttributeDeviceListWithCompletionHandler:^(
[descriptorCluster readAttributeDeviceTypeListWithCompletionHandler:^(
NSArray * _Nullable value, NSError * _Nullable error) {
if (error) {
NSString * resultLog = [[NSString alloc]
Expand Down
37 changes: 19 additions & 18 deletions src/darwin/CHIPTool/CHIPTool/View Controllers/QRCode/QRCodeViewController.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -706,10 +706,10 @@ - (void)updateUIFields:(MTRSetupPayload *)payload rawPayload:(nullable NSString
} else {
_manualCodeLabel.hidden = YES;
_versionLabel.text = [NSString stringWithFormat:@"%@", payload.version];
if (payload.rendezvousInformation == nil) {
if (payload.discoveryCapabilities == MTRDiscoveryCapabilitiesUnknown) {
_rendezVousInformation.text = NOT_APPLICABLE_STRING;
} else {
_rendezVousInformation.text = [NSString stringWithFormat:@"%lu", [payload.rendezvousInformation unsignedLongValue]];
_rendezVousInformation.text = [NSString stringWithFormat:@"%lu", payload.discoveryCapabilities];
}
if ([payload.serialNumber length] > 0) {
self->_serialNumber.text = payload.serialNumber;
Expand All @@ -719,7 +719,7 @@ - (void)updateUIFields:(MTRSetupPayload *)payload rawPayload:(nullable NSString
}

_discriminatorLabel.text = [NSString stringWithFormat:@"%@", payload.discriminator];
_setupPinCodeLabel.text = [NSString stringWithFormat:@"%@", payload.setUpPINCode];
_setupPinCodeLabel.text = [NSString stringWithFormat:@"%@", payload.setupPasscode];
// TODO: Only display vid and pid if present
_vendorID.text = [NSString stringWithFormat:@"%@", payload.vendorID];
_productID.text = [NSString stringWithFormat:@"%@", payload.productID];
Expand Down Expand Up @@ -747,7 +747,7 @@ - (void)parseOptionalData:(MTRSetupPayload *)payload
continue;
}

BOOL isTypeString = [info.infoType isEqualToNumber:[NSNumber numberWithInt:MTROptionalQRCodeInfoTypeString]];
BOOL isTypeString = (info.infoType == MTROptionalQRCodeInfoTypeString);
if (!isTypeString) {
return;
}
Expand All @@ -767,26 +767,29 @@ - (void)parseOptionalData:(MTRSetupPayload *)payload

- (void)handleRendezVous:(MTRSetupPayload *)payload rawPayload:(NSString *)rawPayload
{
if (payload.rendezvousInformation == nil) {
if (payload.discoveryCapabilities == MTRDiscoveryCapabilitiesUnknown) {
NSLog(@"Rendezvous Default");
[self handleRendezVousDefault:rawPayload];
return;
}

// TODO: This is a pretty broken way to handle a bitmask.
switch ([payload.rendezvousInformation unsignedLongValue]) {
case MTRDiscoveryCapabilitiesNone:
case MTRDiscoveryCapabilitiesOnNetwork:
case MTRDiscoveryCapabilitiesBLE:
case MTRDiscoveryCapabilitiesAllMask:
// Avoid SoftAP if we have other options.
if ((payload.discoveryCapabilities & MTRDiscoveryCapabilitiesOnNetwork)
|| (payload.discoveryCapabilities & MTRDiscoveryCapabilitiesBLE)) {
NSLog(@"Rendezvous Default");
[self handleRendezVousDefault:rawPayload];
break;
case MTRDiscoveryCapabilitiesSoftAP:
return;
}

if (payload.discoveryCapabilities & MTRDiscoveryCapabilitiesSoftAP) {
NSLog(@"Rendezvous Wi-Fi");
[self handleRendezVousWiFi:[self getNetworkName:payload.discriminator]];
break;
return;
}

// Just fall back on the default.
NSLog(@"Rendezvous Default");
[self handleRendezVousDefault:rawPayload];
}

- (NSString *)getNetworkName:(NSNumber *)discriminator
Expand Down Expand Up @@ -879,9 +882,8 @@ - (void)scannedQRCode:(NSString *)qrCode
[self->_captureSession stopRunning];
[self->_session invalidateSession];
});
MTRQRCodeSetupPayloadParser * parser = [[MTRQRCodeSetupPayloadParser alloc] initWithBase38Representation:qrCode];
NSError * error;
_setupPayload = [parser populatePayload:&error];
_setupPayload = [MTRSetupPayload setupPayloadWithOnboardingPayload:qrCode error:&error];
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 1.0 * NSEC_PER_SEC), dispatch_get_main_queue(), ^{
[self postScanningQRCodeState];

Expand Down Expand Up @@ -952,9 +954,8 @@ - (IBAction)enteredManualCode:(id)sender
NSString * decimalString = _manualCodeTextField.text;
[self manualCodeEnteredStartState];

MTRManualSetupPayloadParser * parser = [[MTRManualSetupPayloadParser alloc] initWithDecimalStringRepresentation:decimalString];
NSError * error;
_setupPayload = [parser populatePayload:&error];
_setupPayload = [MTRSetupPayload setupPayloadWithOnboardingPayload:decimalString error:&error];
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, INDICATOR_DELAY), dispatch_get_main_queue(), ^{
[self displayManualCodeInSetupPayloadView:self->_setupPayload decimalString:decimalString withError:error];
});
Expand Down
1 change: 0 additions & 1 deletion src/darwin/Framework/CHIP/MTRDeviceController.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#import <Foundation/Foundation.h>

#import <Matter/MTRNOCChainIssuer.h>
#import <Matter/MTROnboardingPayloadParser.h>

@class MTRBaseDevice;

Expand Down
52 changes: 35 additions & 17 deletions src/darwin/Framework/CHIP/MTRSetupPayload.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

NS_ASSUME_NONNULL_BEGIN

typedef NS_ENUM(NSUInteger, MTRDiscoveryCapabilities) {
MTRDiscoveryCapabilitiesNone = 0, // Device does not support any method for rendezvous
typedef NS_OPTIONS(NSUInteger, MTRDiscoveryCapabilities) {
MTRDiscoveryCapabilitiesUnknown = 0, // Device capabilties are not known (e.g. all we have is a manual pairing code).
MTRDiscoveryCapabilitiesSoftAP = 1 << 0, // Device supports WiFi softAP
MTRDiscoveryCapabilitiesBLE = 1 << 1, // Device supports BLE
MTRDiscoveryCapabilitiesOnNetwork = 1 << 2, // Device supports On Network setup
Expand All @@ -36,44 +36,62 @@ typedef NS_ENUM(NSUInteger, MTRCommissioningFlow) {
MTRCommissioningFlowInvalid = 3,
};

typedef NS_ENUM(NSUInteger, MTROptionalQRCodeInfoType) {
MTROptionalQRCodeInfoTypeUnknown,
MTROptionalQRCodeInfoTypeString,
MTROptionalQRCodeInfoTypeInt32
};
typedef NS_ENUM(NSUInteger, MTROptionalQRCodeInfoType) { MTROptionalQRCodeInfoTypeString, MTROptionalQRCodeInfoTypeInt32 };

/**
* An optional information item present in the QR code the setup payload was
* initialized from.
*/
@interface MTROptionalQRCodeInfo : NSObject
@property (nonatomic, copy) NSNumber * infoType;
@property (nonatomic, assign) MTROptionalQRCodeInfoType infoType;
// The numeric value of the TLV tag for this information item.
@property (nonatomic, copy) NSNumber * tag;
@property (nonatomic, copy) NSNumber * integerValue;
@property (nonatomic, copy) NSString * stringValue;
// Exactly one of integerValue and stringValue will be non-nil, depending on the
// the value of "infoType".
@property (nonatomic, copy, nullable) NSNumber * integerValue;
@property (nonatomic, copy, nullable) NSString * stringValue;
@end

/**
* A setup payload that can be created from a pairing code and serialized to a
* pairing code.
*/
@interface MTRSetupPayload : NSObject <NSSecureCoding>

@property (nonatomic, copy) NSNumber * version;
@property (nonatomic, copy) NSNumber * vendorID;
@property (nonatomic, copy) NSNumber * productID;
@property (nonatomic, assign) MTRCommissioningFlow commissioningFlow;
/**
* rendezvousInformation is nil when the discovery capabilities bitmask is
* unknown.
*
* Otherwise its value is made up of the MTRDiscoveryCapabilities flags.
* The value of discoveryCapabilities is made up of the various MTRDiscoveryCapabilities flags.
*/
@property (nonatomic, copy, nullable) NSNumber * rendezvousInformation;
@property (nonatomic, assign) MTRDiscoveryCapabilities discoveryCapabilities;
@property (nonatomic, copy) NSNumber * discriminator;
/**
* If hasShortDiscriminator is true, the discriminator value contains just the
* high 4 bits of the full discriminator. For example, if
* hasShortDiscriminator is true and discriminator is 0xA, then the full
* discriminator can be anything in the range 0xA00 t0 0xAFF.
*/
@property (nonatomic, assign) BOOL hasShortDiscriminator;
@property (nonatomic, copy) NSNumber * setUpPINCode;
@property (nonatomic, copy) NSNumber * setupPasscode;

@property (nonatomic, copy) NSString * serialNumber;
@property (nonatomic, copy, nullable) NSString * serialNumber;
- (nullable NSArray<MTROptionalQRCodeInfo *> *)getAllOptionalVendorData:(NSError * __autoreleasing *)error;

/**
* Generate a random Matter-valid setup passcode.
*/
+ (NSNumber *)generateRandomSetupPasscode;

/**
* Create an MTRSetupPayload with the given onboarding payload.
*
* Will return nil on errors (e.g. if the onboarding payload cannot be parsed).
*/
+ (MTRSetupPayload * _Nullable)setupPayloadWithOnboardingPayload:(NSString *)onboardingPayload
error:(NSError * __autoreleasing *)error;

/** Get 11 digit manual entry code from the setup payload. */
- (nullable NSString *)manualEntryCode;

Expand Down
Loading

0 comments on commit 9ababd1

Please sign in to comment.