Skip to content

Commit

Permalink
add isShortDiscriminator to SetupPayload (#13671)
Browse files Browse the repository at this point in the history
The SetupPayload (Onboarding Payload in spec terminology) should be
fully self describing, but isn't because it does not record whether the
discriminator is the short or long version.

This commit makes the necessary additions to expose that information.
This improvement is then used to simplify the code in SetUpCodePairer,
which was passing around its own isShort bool.  Instead, it now just
uses the SetupPayload struct in all interfaces.
  • Loading branch information
msandstedt authored Jan 21, 2022
1 parent c24c34b commit cf1b254
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 20 deletions.
31 changes: 16 additions & 15 deletions src/controller/SetUpCodePairer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,27 +44,27 @@ CHIP_ERROR SetUpCodePairer::PairDevice(NodeId remoteId, const char * setUpCode)
mRemoteId = remoteId;
mSetUpPINCode = payload.setUpPINCode;

return Connect(payload.rendezvousInformation, payload.discriminator, !isQRCode);
return Connect(payload);
}

CHIP_ERROR SetUpCodePairer::Connect(RendezvousInformationFlag rendezvousInformation, uint16_t discriminator, bool isShort)
CHIP_ERROR SetUpCodePairer::Connect(SetupPayload & payload)
{
CHIP_ERROR err = CHIP_NO_ERROR;
bool isRunning = false;

bool searchOverAll = rendezvousInformation == RendezvousInformationFlag::kNone;
if (searchOverAll || rendezvousInformation == RendezvousInformationFlag::kBLE)
bool searchOverAll = payload.rendezvousInformation == RendezvousInformationFlag::kNone;
if (searchOverAll || payload.rendezvousInformation == RendezvousInformationFlag::kBLE)
{
if (CHIP_NO_ERROR == (err = StartDiscoverOverBle(discriminator, isShort)))
if (CHIP_NO_ERROR == (err = StartDiscoverOverBle(payload)))
{
isRunning = true;
}
VerifyOrReturnError(searchOverAll || CHIP_NO_ERROR == err, err);
}

if (searchOverAll || rendezvousInformation == RendezvousInformationFlag::kSoftAP)
if (searchOverAll || payload.rendezvousInformation == RendezvousInformationFlag::kSoftAP)
{
if (CHIP_NO_ERROR == (err = StartDiscoverOverSoftAP(discriminator, isShort)))
if (CHIP_NO_ERROR == (err = StartDiscoverOverSoftAP(payload)))
{
isRunning = true;
}
Expand All @@ -73,8 +73,7 @@ CHIP_ERROR SetUpCodePairer::Connect(RendezvousInformationFlag rendezvousInformat

// We always want to search on network because any node that has already been commissioned will use on-network regardless of the
// QR code flag.
if (CHIP_NO_ERROR ==
(err = StartDiscoverOverIP(isShort ? static_cast<uint16_t>((discriminator >> 8) & 0x0F) : discriminator, isShort)))
if (CHIP_NO_ERROR == (err = StartDiscoverOverIP(payload)))
{
isRunning = true;
}
Expand All @@ -83,11 +82,11 @@ CHIP_ERROR SetUpCodePairer::Connect(RendezvousInformationFlag rendezvousInformat
return isRunning ? CHIP_NO_ERROR : CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
}

CHIP_ERROR SetUpCodePairer::StartDiscoverOverBle(uint16_t discriminator, bool isShort)
CHIP_ERROR SetUpCodePairer::StartDiscoverOverBle(SetupPayload & payload)
{
#if CONFIG_NETWORK_LAYER_BLE
VerifyOrReturnError(mBleLayer != nullptr, CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE);
return mBleLayer->NewBleConnectionByDiscriminator(discriminator, this, OnDiscoveredDeviceOverBleSuccess,
return mBleLayer->NewBleConnectionByDiscriminator(payload.discriminator, this, OnDiscoveredDeviceOverBleSuccess,
OnDiscoveredDeviceOverBleError);
#else
return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
Expand All @@ -104,11 +103,13 @@ CHIP_ERROR SetUpCodePairer::StopConnectOverBle()
#endif // CONFIG_NETWORK_LAYER_BLE
}

CHIP_ERROR SetUpCodePairer::StartDiscoverOverIP(uint16_t discriminator, bool isShort)
CHIP_ERROR SetUpCodePairer::StartDiscoverOverIP(SetupPayload & payload)
{
#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
currentFilter.type = isShort ? Dnssd::DiscoveryFilterType::kShortDiscriminator : Dnssd::DiscoveryFilterType::kLongDiscriminator;
currentFilter.code = discriminator;
currentFilter.type = payload.isShortDiscriminator ? Dnssd::DiscoveryFilterType::kShortDiscriminator
: Dnssd::DiscoveryFilterType::kLongDiscriminator;
currentFilter.code =
payload.isShortDiscriminator ? static_cast<uint16_t>((payload.discriminator >> 8) & 0x0F) : payload.discriminator;
return mCommissioner->DiscoverCommissionableNodes(currentFilter);
#else
return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
Expand All @@ -123,7 +124,7 @@ CHIP_ERROR SetUpCodePairer::StopConnectOverIP()
return CHIP_NO_ERROR;
}

CHIP_ERROR SetUpCodePairer::StartDiscoverOverSoftAP(uint16_t discriminator, bool isShort)
CHIP_ERROR SetUpCodePairer::StartDiscoverOverSoftAP(SetupPayload & payload)
{
return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
}
Expand Down
8 changes: 4 additions & 4 deletions src/controller/SetUpCodePairer.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ class DLL_EXPORT SetUpCodePairer
#endif // CONFIG_NETWORK_LAYER_BLE

private:
CHIP_ERROR Connect(RendezvousInformationFlag rendezvousInformation, uint16_t discriminator, bool isShort);
CHIP_ERROR StartDiscoverOverBle(uint16_t discriminator, bool isShort);
CHIP_ERROR Connect(SetupPayload & paload);
CHIP_ERROR StartDiscoverOverBle(SetupPayload & payload);
CHIP_ERROR StopConnectOverBle();
CHIP_ERROR StartDiscoverOverIP(uint16_t discriminator, bool isShort);
CHIP_ERROR StartDiscoverOverIP(SetupPayload & payload);
CHIP_ERROR StopConnectOverIP();
CHIP_ERROR StartDiscoverOverSoftAP(uint16_t discriminator, bool isShort);
CHIP_ERROR StartDiscoverOverSoftAP(SetupPayload & payload);
CHIP_ERROR StopConnectOverSoftAP();

void OnDeviceDiscovered(RendezvousParameters & params);
Expand Down
3 changes: 2 additions & 1 deletion src/setup_payload/ManualSetupPayloadParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ CHIP_ERROR ManualSetupPayloadParser::populatePayload(SetupPayload & outPayload)
static_assert(kSetupPINCodeFieldLengthInBits <= 32, "Won't fit in uint32_t");
outPayload.setUpPINCode = static_cast<uint32_t>(setUpPINCode);
static_assert(kManualSetupDiscriminatorFieldLengthInBits <= 16, "Won't fit in uint16_t");
outPayload.discriminator = static_cast<uint16_t>(discriminator);
outPayload.discriminator = static_cast<uint16_t>(discriminator);
outPayload.isShortDiscriminator = true;

return result;
}
Expand Down
1 change: 1 addition & 0 deletions src/setup_payload/SetupPayload.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ struct PayloadContents

bool isValidQRCodePayload() const;
bool isValidManualCode() const;
bool isShortDiscriminator = false;
bool operator==(PayloadContents & input) const;
};

Expand Down

0 comments on commit cf1b254

Please sign in to comment.