Skip to content

Commit

Permalink
Clearly distinguish between unknown and all-0 discovery capabilities. (
Browse files Browse the repository at this point in the history
…#21696)

SetUpCodePairer was treating all-0 as unknown, which is not really right.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Aug 9, 2023
1 parent 5ad969d commit 1665836
Show file tree
Hide file tree
Showing 25 changed files with 167 additions and 118 deletions.
2 changes: 1 addition & 1 deletion examples/all-clusters-app/nxp/mw320/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ std::string createSetupPayload()
payload.version = 0;
payload.discriminator = discriminator;
payload.setUpPINCode = setupPINCode;
payload.rendezvousInformation = RendezvousInformationFlags(chip::RendezvousInformationFlag::kBLE);
payload.rendezvousInformation.SetValue(chip::RendezvousInformationFlag::kBLE);
payload.vendorID = vendorId;
payload.productID = productId;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,12 @@ CHIP_ERROR SetupPayloadGenerateQRCodeCommand::Run()

if (mRendezvous.HasValue())
{
payload.rendezvousInformation.SetRaw(mRendezvous.Value());
payload.rendezvousInformation.Emplace().SetRaw(mRendezvous.Value());
}
else if (!payload.rendezvousInformation.HasValue())
{
// Default to not having anything in the discovery capabilities.
payload.rendezvousInformation.SetValue(RendezvousInformationFlag::kNone);
}

if (mTLVBytes.HasValue())
Expand Down
50 changes: 29 additions & 21 deletions examples/chip-tool/commands/payload/SetupPayloadParseCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,35 +78,43 @@ CHIP_ERROR SetupPayloadParseCommand::Print(chip::SetupPayload payload)
{
StringBuilder<128> humanFlags;

if (payload.rendezvousInformation.HasAny())
if (!payload.rendezvousInformation.HasValue())
{
if (payload.rendezvousInformation.Has(RendezvousInformationFlag::kSoftAP))
{
humanFlags.Add("Soft-AP");
}
if (payload.rendezvousInformation.Has(RendezvousInformationFlag::kBLE))
ChipLogProgress(SetupPayload, "Discovery Bitmask: UNKNOWN");
}
else
{
if (payload.rendezvousInformation.Value().HasAny())
{
if (!humanFlags.Empty())
if (payload.rendezvousInformation.Value().Has(RendezvousInformationFlag::kSoftAP))
{
humanFlags.Add(", ");
humanFlags.Add("Soft-AP");
}
humanFlags.Add("BLE");
}
if (payload.rendezvousInformation.Has(RendezvousInformationFlag::kOnNetwork))
{
if (!humanFlags.Empty())
if (payload.rendezvousInformation.Value().Has(RendezvousInformationFlag::kBLE))
{
if (!humanFlags.Empty())
{
humanFlags.Add(", ");
}
humanFlags.Add("BLE");
}
if (payload.rendezvousInformation.Value().Has(RendezvousInformationFlag::kOnNetwork))
{
humanFlags.Add(", ");
if (!humanFlags.Empty())
{
humanFlags.Add(", ");
}
humanFlags.Add("On IP network");
}
humanFlags.Add("On IP network");
}
}
else
{
humanFlags.Add("NONE");
}
else
{
humanFlags.Add("NONE");
}

ChipLogProgress(SetupPayload, "Capabilities: 0x%02X (%s)", payload.rendezvousInformation.Raw(), humanFlags.c_str());
ChipLogProgress(SetupPayload, "Discovery Bitmask: 0x%02X (%s)", payload.rendezvousInformation.Value().Raw(),
humanFlags.c_str());
}
}
if (payload.discriminator.IsShortDiscriminator())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,33 +84,34 @@
NSLog(@"ProductID: %@", payload.productID);
NSLog(@"Custom flow: %lu (%@)", payload.commissioningFlow, CustomFlowString(payload.commissioningFlow));
{
NSMutableString * humanFlags = [[NSMutableString alloc] init];
if (payload.rendezvousInformation == nil) {
NSLog(@"Capabilities: UNKNOWN");
} else {
NSMutableString * humanFlags = [[NSMutableString alloc] init];

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

NSLog(@"Capabilities: 0x%02lX (%@)", payload.rendezvousInformation, humanFlags);
NSLog(@"Capabilities: 0x%02lX (%@)", value, humanFlags);
}
}
NSLog(@"Discriminator: %@", payload.discriminator);
NSLog(@"Passcode: %@", payload.setUpPINCode);
Expand Down
4 changes: 2 additions & 2 deletions examples/platform/linux/AppMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,9 @@ int ChipLinuxAppInit(int argc, char * const argv[], OptionSet * customOptions)
LinuxDeviceOptions::GetInstance());
SuccessOrExit(err);

if (LinuxDeviceOptions::GetInstance().payload.rendezvousInformation.HasAny())
if (LinuxDeviceOptions::GetInstance().payload.rendezvousInformation.HasValue())
{
rendezvousFlags = LinuxDeviceOptions::GetInstance().payload.rendezvousInformation;
rendezvousFlags = LinuxDeviceOptions::GetInstance().payload.rendezvousInformation.Value();
}

err = GetPayloadContents(LinuxDeviceOptions::GetInstance().payload, rendezvousFlags);
Expand Down
2 changes: 1 addition & 1 deletion examples/platform/linux/Options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ bool HandleOption(const char * aProgram, OptionSet * aOptions, int aIdentifier,
break;

case kDeviceOption_Capabilities:
LinuxDeviceOptions::GetInstance().payload.rendezvousInformation.SetRaw(static_cast<uint8_t>(atoi(aValue)));
LinuxDeviceOptions::GetInstance().payload.rendezvousInformation.Emplace().SetRaw(static_cast<uint8_t>(atoi(aValue)));
break;

case kDeviceOption_Discriminator: {
Expand Down
6 changes: 3 additions & 3 deletions src/app/server/OnboardingCodesUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ void ShareQRCodeOverNFC(chip::RendezvousInformationFlags aRendezvousFlags)

CHIP_ERROR GetPayloadContents(chip::PayloadContents & aPayload, chip::RendezvousInformationFlags aRendezvousFlags)
{
CHIP_ERROR err = CHIP_NO_ERROR;
aPayload.version = 0;
aPayload.rendezvousInformation = aRendezvousFlags;
CHIP_ERROR err = CHIP_NO_ERROR;
aPayload.version = 0;
aPayload.rendezvousInformation.SetValue(aRendezvousFlags);

err = GetCommissionableDataProvider()->GetSetupPasscode(aPayload.setUpPINCode);
if (err != CHIP_NO_ERROR)
Expand Down
2 changes: 1 addition & 1 deletion src/controller/CommissioningWindowOpener.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(NodeId deviceId, S

mSetupPayload.version = 0;
mSetupPayload.discriminator.SetLongValue(discriminator);
mSetupPayload.rendezvousInformation = RendezvousInformationFlags(RendezvousInformationFlag::kOnNetwork);
mSetupPayload.rendezvousInformation.SetValue(RendezvousInformationFlag::kOnNetwork);

mCommissioningWindowCallback = callback;
mBasicCommissioningWindowCallback = nullptr;
Expand Down
6 changes: 3 additions & 3 deletions src/controller/SetUpCodePairer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ CHIP_ERROR SetUpCodePairer::Connect(SetupPayload & payload)
CHIP_ERROR err = CHIP_NO_ERROR;
bool isRunning = false;

bool searchOverAll = payload.rendezvousInformation == RendezvousInformationFlag::kNone;
if (searchOverAll || payload.rendezvousInformation == RendezvousInformationFlag::kBLE)
bool searchOverAll = !payload.rendezvousInformation.HasValue();
if (searchOverAll || payload.rendezvousInformation.Value().Has(RendezvousInformationFlag::kBLE))
{
if (CHIP_NO_ERROR == (err = StartDiscoverOverBle(payload)))
{
Expand All @@ -81,7 +81,7 @@ CHIP_ERROR SetUpCodePairer::Connect(SetupPayload & payload)
VerifyOrReturnError(searchOverAll || CHIP_NO_ERROR == err || CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE == err, err);
}

if (searchOverAll || payload.rendezvousInformation == RendezvousInformationFlag::kSoftAP)
if (searchOverAll || payload.rendezvousInformation.Value().Has(RendezvousInformationFlag::kSoftAP))
{
if (CHIP_NO_ERROR == (err = StartDiscoverOverSoftAP(payload)))
{
Expand Down
2 changes: 1 addition & 1 deletion src/controller/python/chip/setup_payload/Generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ extern "C" ChipError::StorageType pychip_SetupPayload_PrintOnboardingCodes(uint3
payload.vendorID = vendorId;
payload.productID = productId;
payload.discriminator.SetLongValue(discriminator);
payload.rendezvousInformation = rendezvousFlags.SetRaw(capabilities);
payload.rendezvousInformation.SetValue(rendezvousFlags.SetRaw(capabilities));

switch (customFlow)
{
Expand Down
5 changes: 4 additions & 1 deletion src/controller/python/chip/setup_payload/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ void YieldSetupPayloadAttributes(const SetupPayload & payload, AttributeVisitor
attrVisitor("VendorID", std::to_string(payload.vendorID).c_str());
attrVisitor("ProductID", std::to_string(payload.productID).c_str());
attrVisitor("CommissioningFlow", std::to_string(static_cast<uint8_t>(payload.commissioningFlow)).c_str());
attrVisitor("RendezvousInformation", std::to_string(payload.rendezvousInformation.Raw()).c_str());
if (payload.rendezvousInformation.HasValue())
{
attrVisitor("RendezvousInformation", std::to_string(payload.rendezvousInformation.Value().Raw()).c_str());
}
if (payload.discriminator.IsShortDiscriminator())
{
attrVisitor("Short discriminator", std::to_string(payload.discriminator.GetShortValue()).c_str());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,11 @@ - (void)updateUIFields:(MTRSetupPayload *)payload rawPayload:(nullable NSString
} else {
_manualCodeLabel.hidden = YES;
_versionLabel.text = [NSString stringWithFormat:@"%@", payload.version];
_rendezVousInformation.text = [NSString stringWithFormat:@"%lu", payload.rendezvousInformation];
if (payload.rendezvousInformation == nil) {
_rendezVousInformation.text = NOT_APPLICABLE_STRING;
} else {
_rendezVousInformation.text = [NSString stringWithFormat:@"%lu", [payload.rendezvousInformation unsignedLongValue]];
}
if ([payload.serialNumber length] > 0) {
self->_serialNumber.text = payload.serialNumber;
} else {
Expand Down Expand Up @@ -763,15 +767,22 @@ - (void)parseOptionalData:(MTRSetupPayload *)payload

- (void)handleRendezVous:(MTRSetupPayload *)payload rawPayload:(NSString *)rawPayload
{
switch (payload.rendezvousInformation) {
case MTRRendezvousInformationNone:
case MTRRendezvousInformationOnNetwork:
case MTRRendezvousInformationBLE:
case MTRRendezvousInformationAllMask:
if (payload.rendezvousInformation == nil) {
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:
NSLog(@"Rendezvous Default");
[self handleRendezVousDefault:rawPayload];
break;
case MTRRendezvousInformationSoftAP:
case MTRDiscoveryCapabilitiesSoftAP:
NSLog(@"Rendezvous Wi-Fi");
[self handleRendezVousWiFi:[self getNetworkName:payload.discriminator]];
break;
Expand Down
22 changes: 14 additions & 8 deletions src/darwin/Framework/CHIP/MTRSetupPayload.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@

NS_ASSUME_NONNULL_BEGIN

typedef NS_ENUM(NSUInteger, MTRRendezvousInformationFlags) {
MTRRendezvousInformationNone = 0, // Device does not support any method for rendezvous
MTRRendezvousInformationSoftAP = 1 << 0, // Device supports WiFi softAP
MTRRendezvousInformationBLE = 1 << 1, // Device supports BLE
MTRRendezvousInformationOnNetwork = 1 << 2, // Device supports On Network setup
typedef NS_ENUM(NSUInteger, MTRDiscoveryCapabilities) {
MTRDiscoveryCapabilitiesNone = 0, // Device does not support any method for rendezvous
MTRDiscoveryCapabilitiesSoftAP = 1 << 0, // Device supports WiFi softAP
MTRDiscoveryCapabilitiesBLE = 1 << 1, // Device supports BLE
MTRDiscoveryCapabilitiesOnNetwork = 1 << 2, // Device supports On Network setup

MTRRendezvousInformationAllMask
= MTRRendezvousInformationSoftAP | MTRRendezvousInformationBLE | MTRRendezvousInformationOnNetwork,
MTRDiscoveryCapabilitiesAllMask
= MTRDiscoveryCapabilitiesSoftAP | MTRDiscoveryCapabilitiesBLE | MTRDiscoveryCapabilitiesOnNetwork,
};

typedef NS_ENUM(NSUInteger, MTRCommissioningFlow) {
Expand Down Expand Up @@ -55,7 +55,13 @@ typedef NS_ENUM(NSUInteger, MTROptionalQRCodeInfoType) {
@property (nonatomic, copy) NSNumber * vendorID;
@property (nonatomic, copy) NSNumber * productID;
@property (nonatomic, assign) MTRCommissioningFlow commissioningFlow;
@property (nonatomic, assign) MTRRendezvousInformationFlags rendezvousInformation;
/**
* rendezvousInformation is nil when the discovery capabilities bitmask is
* unknown.
*
* Otherwise its value is made up of the MTRDiscoveryCapabilities flags.
*/
@property (nonatomic, assign, nullable) NSNumber * rendezvousInformation;
@property (nonatomic, copy) NSNumber * discriminator;
@property (nonatomic, assign) BOOL hasShortDiscriminator;
@property (nonatomic, copy) NSNumber * setUpPINCode;
Expand Down
32 changes: 19 additions & 13 deletions src/darwin/Framework/CHIP/MTRSetupPayload.mm
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,23 @@ @implementation MTRSetupPayload {
chip::SetupPayload _chipSetupPayload;
}

- (MTRRendezvousInformationFlags)convertRendezvousFlags:(chip::RendezvousInformationFlags)value
- (NSNumber *)convertRendezvousFlags:(const chip::Optional<chip::RendezvousInformationFlags> &)value
{
if (value.Has(chip::RendezvousInformationFlag::kBLE)) {
return MTRRendezvousInformationBLE;
if (!value.HasValue()) {
return nil;
}
if (value.Has(chip::RendezvousInformationFlag::kSoftAP)) {
return MTRRendezvousInformationSoftAP;

NSUInteger flags = MTRDiscoveryCapabilitiesNone;
if (value.Value().Has(chip::RendezvousInformationFlag::kBLE)) {
flags |= MTRDiscoveryCapabilitiesBLE;
}
if (value.Value().Has(chip::RendezvousInformationFlag::kSoftAP)) {
flags |= MTRDiscoveryCapabilitiesSoftAP;
}
if (value.Has(chip::RendezvousInformationFlag::kOnNetwork)) {
return MTRRendezvousInformationOnNetwork;
if (value.Value().Has(chip::RendezvousInformationFlag::kOnNetwork)) {
flags |= MTRDiscoveryCapabilitiesOnNetwork;
}
return MTRRendezvousInformationNone;
return [NSNumber numberWithUnsignedLong:flags];
}

- (MTRCommissioningFlow)convertCommissioningFlow:(chip::CommissioningFlow)value
Expand Down Expand Up @@ -154,10 +159,10 @@ - (void)encodeWithCoder:(NSCoder *)coder
[coder encodeObject:self.version forKey:MTRSetupPayloadCodingKeyVersion];
[coder encodeObject:self.vendorID forKey:MTRSetupPayloadCodingKeyVendorID];
[coder encodeObject:self.productID forKey:MTRSetupPayloadCodingKeyProductID];
// Casts are safe because commissioning flow, rendezvous information, and
// hasShortDiscriminator values are all pretty small and non-negative.
// Casts are safe because commissioning flow and hasShortDiscriminator
// values are both pretty small and non-negative.
[coder encodeInteger:static_cast<NSInteger>(self.commissioningFlow) forKey:MTRSetupPayloadCodingKeyCommissioningFlow];
[coder encodeInteger:static_cast<NSInteger>(self.rendezvousInformation) forKey:MTRSetupPayloadCodingKeyRendezvousFlags];
[coder encodeObject:self.rendezvousInformation forKey:MTRSetupPayloadCodingKeyRendezvousFlags];
[coder encodeInteger:static_cast<NSInteger>(self.hasShortDiscriminator) forKey:MTRSetupPayloadCodingKeyHasShortDiscriminator];
[coder encodeObject:self.discriminator forKey:MTRSetupPayloadCodingKeyDiscriminator];
[coder encodeObject:self.setUpPINCode forKey:MTRSetupPayloadCodingKeySetupPINCode];
Expand All @@ -170,7 +175,8 @@ - (nullable instancetype)initWithCoder:(NSCoder *)decoder
NSNumber * vendorID = [decoder decodeObjectOfClass:[NSNumber class] forKey:MTRSetupPayloadCodingKeyVendorID];
NSNumber * productID = [decoder decodeObjectOfClass:[NSNumber class] forKey:MTRSetupPayloadCodingKeyProductID];
NSInteger commissioningFlow = [decoder decodeIntegerForKey:MTRSetupPayloadCodingKeyCommissioningFlow];
NSInteger rendezvousInformation = [decoder decodeIntegerForKey:MTRSetupPayloadCodingKeyRendezvousFlags];
NSNumber * rendezvousInformation = [decoder decodeObjectOfClass:[NSNumber class]
forKey:MTRSetupPayloadCodingKeyRendezvousFlags];
NSInteger hasShortDiscriminator = [decoder decodeIntegerForKey:MTRSetupPayloadCodingKeyHasShortDiscriminator];
NSNumber * discriminator = [decoder decodeObjectOfClass:[NSNumber class] forKey:MTRSetupPayloadCodingKeyDiscriminator];
NSNumber * setUpPINCode = [decoder decodeObjectOfClass:[NSNumber class] forKey:MTRSetupPayloadCodingKeySetupPINCode];
Expand All @@ -181,7 +187,7 @@ - (nullable instancetype)initWithCoder:(NSCoder *)decoder
payload.vendorID = vendorID;
payload.productID = productID;
payload.commissioningFlow = static_cast<MTRCommissioningFlow>(commissioningFlow);
payload.rendezvousInformation = static_cast<MTRRendezvousInformationFlags>(rendezvousInformation);
payload.rendezvousInformation = rendezvousInformation;
payload.hasShortDiscriminator = static_cast<BOOL>(hasShortDiscriminator);
payload.discriminator = discriminator;
payload.setUpPINCode = setUpPINCode;
Expand Down
3 changes: 2 additions & 1 deletion src/darwin/Framework/CHIP/MTRSetupPayload_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@
#import "MTRSetupPayload.h"

#ifdef __cplusplus
#import <lib/core/Optional.h>
#import <setup_payload/SetupPayload.h>
#endif

@interface MTRSetupPayload () <NSSecureCoding>

#ifdef __cplusplus
- (id)initWithSetupPayload:(chip::SetupPayload)setupPayload;
- (MTRRendezvousInformationFlags)convertRendezvousFlags:(chip::RendezvousInformationFlags)value;
- (NSNumber *)convertRendezvousFlags:(const chip::Optional<chip::RendezvousInformationFlags> &)value;
- (MTRCommissioningFlow)convertCommissioningFlow:(chip::CommissioningFlow)value;
#endif

Expand Down
Loading

0 comments on commit 1665836

Please sign in to comment.