From cf1b25478bbcb93164b09f9546865430a3b7d8e7 Mon Sep 17 00:00:00 2001 From: Michael Sandstedt Date: Thu, 20 Jan 2022 19:24:10 -0600 Subject: [PATCH] add isShortDiscriminator to SetupPayload (#13671) 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. --- src/controller/SetUpCodePairer.cpp | 31 ++++++++++--------- src/controller/SetUpCodePairer.h | 8 ++--- .../ManualSetupPayloadParser.cpp | 3 +- src/setup_payload/SetupPayload.h | 1 + 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/controller/SetUpCodePairer.cpp b/src/controller/SetUpCodePairer.cpp index ddb460051b40b9..16e0e04d14900d 100644 --- a/src/controller/SetUpCodePairer.cpp +++ b/src/controller/SetUpCodePairer.cpp @@ -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; } @@ -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((discriminator >> 8) & 0x0F) : discriminator, isShort))) + if (CHIP_NO_ERROR == (err = StartDiscoverOverIP(payload))) { isRunning = true; } @@ -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; @@ -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((payload.discriminator >> 8) & 0x0F) : payload.discriminator; return mCommissioner->DiscoverCommissionableNodes(currentFilter); #else return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; @@ -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; } diff --git a/src/controller/SetUpCodePairer.h b/src/controller/SetUpCodePairer.h index 3b72831a331164..0ecd41c96b0597 100644 --- a/src/controller/SetUpCodePairer.h +++ b/src/controller/SetUpCodePairer.h @@ -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); diff --git a/src/setup_payload/ManualSetupPayloadParser.cpp b/src/setup_payload/ManualSetupPayloadParser.cpp index 0b59ffb53d2c16..6ef1f35ef636c6 100644 --- a/src/setup_payload/ManualSetupPayloadParser.cpp +++ b/src/setup_payload/ManualSetupPayloadParser.cpp @@ -193,7 +193,8 @@ CHIP_ERROR ManualSetupPayloadParser::populatePayload(SetupPayload & outPayload) static_assert(kSetupPINCodeFieldLengthInBits <= 32, "Won't fit in uint32_t"); outPayload.setUpPINCode = static_cast(setUpPINCode); static_assert(kManualSetupDiscriminatorFieldLengthInBits <= 16, "Won't fit in uint16_t"); - outPayload.discriminator = static_cast(discriminator); + outPayload.discriminator = static_cast(discriminator); + outPayload.isShortDiscriminator = true; return result; } diff --git a/src/setup_payload/SetupPayload.h b/src/setup_payload/SetupPayload.h index b6e000c2f0e131..298dd54ea12a69 100644 --- a/src/setup_payload/SetupPayload.h +++ b/src/setup_payload/SetupPayload.h @@ -119,6 +119,7 @@ struct PayloadContents bool isValidQRCodePayload() const; bool isValidManualCode() const; + bool isShortDiscriminator = false; bool operator==(PayloadContents & input) const; };