Skip to content

Commit

Permalink
Fix discriminator handling in chip-tool pairing commands.
Browse files Browse the repository at this point in the history
Not all commands initialize the discriminator value, and the ones that don't should not use it.

Fixes #34096
  • Loading branch information
bzbarsky-apple committed Jun 27, 2024
1 parent dd0c49b commit 27723d5
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 12 deletions.
17 changes: 14 additions & 3 deletions examples/chip-tool/commands/pairing/PairingCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,12 @@ CHIP_ERROR PairingCommand::PairWithCode(NodeId remoteId)

CHIP_ERROR PairingCommand::Pair(NodeId remoteId, PeerAddress address)
{
auto params = RendezvousParameters().SetSetupPINCode(mSetupPINCode).SetDiscriminator(mDiscriminator).SetPeerAddress(address);
VerifyOrDieWithMsg(mSetupPINCode.has_value(), chipTool, "Using mSetupPINCode in a mode when we have not gotten one");
auto params = RendezvousParameters().SetSetupPINCode(mSetupPINCode.value()).SetPeerAddress(address);
if (mDiscriminator.has_value())
{
params.SetDiscriminator(mDiscriminator.value());
}

CHIP_ERROR err = CHIP_NO_ERROR;
if (mPaseOnly.ValueOr(false))
Expand All @@ -232,9 +237,11 @@ CHIP_ERROR PairingCommand::PairWithMdnsOrBleByIndex(NodeId remoteId, uint16_t in
#if CHIP_DEVICE_LAYER_TARGET_DARWIN
VerifyOrReturnError(IsInteractive(), CHIP_ERROR_INCORRECT_STATE);

VerifyOrDieWithMsg(mSetupPINCode.has_value(), chipTool, "Using mSetupPINCode in a mode when we have not gotten one");

RendezvousParameters params;
ReturnErrorOnFailure(GetDeviceScanner().Get(index, params));
params.SetSetupPINCode(mSetupPINCode);
params.SetSetupPINCode(mSetupPINCode.value());

CHIP_ERROR err = CHIP_NO_ERROR;
if (mPaseOnly.ValueOr(false))
Expand All @@ -254,6 +261,10 @@ CHIP_ERROR PairingCommand::PairWithMdnsOrBleByIndex(NodeId remoteId, uint16_t in

CHIP_ERROR PairingCommand::PairWithMdnsOrBleByIndexWithCode(NodeId remoteId, uint16_t index)
{
// We might or might not have a setup code. We don't know yet, but if we
// do, we'll emplace it at that point.
mSetupPINCode.reset();

#if CHIP_DEVICE_LAYER_TARGET_DARWIN
VerifyOrReturnError(IsInteractive(), CHIP_ERROR_INCORRECT_STATE);

Expand All @@ -277,7 +288,7 @@ CHIP_ERROR PairingCommand::PairWithMdnsOrBleByIndexWithCode(NodeId remoteId, uin
VerifyOrReturnError(payload.isValidManualCode(), CHIP_ERROR_INVALID_ARGUMENT);
}

mSetupPINCode = payload.setUpPINCode;
mSetupPINCode.emplace(payload.setUpPINCode);
return PairWithMdnsOrBleByIndex(remoteId, index);
}

Expand Down
26 changes: 17 additions & 9 deletions examples/chip-tool/commands/pairing/PairingCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#include <lib/support/Span.h>
#include <lib/support/ThreadOperationalDataset.h>

#include <optional>

enum class PairingMode
{
None,
Expand Down Expand Up @@ -101,32 +103,32 @@ class PairingCommand : public CHIPCommand,
break;
case PairingMode::Ble:
AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
AddArgument("discriminator", 0, 4096, &mDiscriminator);
AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode.emplace());
AddArgument("discriminator", 0, 4096, &mDiscriminator.emplace());
break;
case PairingMode::OnNetwork:
AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode.emplace());
AddArgument("pase-only", 0, 1, &mPaseOnly);
break;
case PairingMode::SoftAP:
AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
AddArgument("discriminator", 0, 4096, &mDiscriminator);
AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode.emplace());
AddArgument("discriminator", 0, 4096, &mDiscriminator.emplace());
AddArgument("device-remote-ip", &mRemoteAddr);
AddArgument("device-remote-port", 0, UINT16_MAX, &mRemotePort);
AddArgument("pase-only", 0, 1, &mPaseOnly);
break;
case PairingMode::AlreadyDiscovered:
AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode.emplace());
AddArgument("device-remote-ip", &mRemoteAddr);
AddArgument("device-remote-port", 0, UINT16_MAX, &mRemotePort);
AddArgument("pase-only", 0, 1, &mPaseOnly);
break;
case PairingMode::AlreadyDiscoveredByIndex:
AddArgument("skip-commissioning-complete", 0, 1, &mSkipCommissioningComplete);
AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode);
AddArgument("setup-pin-code", 0, 134217727, &mSetupPINCode.emplace());
AddArgument("index", 0, UINT16_MAX, &mIndex);
AddArgument("pase-only", 0, 1, &mPaseOnly);
break;
Expand Down Expand Up @@ -246,8 +248,14 @@ class PairingCommand : public CHIPCommand,
mComplex_DSTOffsets;

uint16_t mRemotePort;
uint16_t mDiscriminator;
uint32_t mSetupPINCode;
// mDiscriminator is only used for some situations, but in those situations
// it's mandatory. Track whether we're actually using it; the cases that do
// will emplace this optional.
std::optional<uint16_t> mDiscriminator;
// mSetupPINCode is only used for some situations, but in those situations
// it's mandatory. Track whether we're actually using it; the cases that do
// will emplace this optional.
std::optional<uint32_t> mSetupPINCode;
uint16_t mIndex;
chip::ByteSpan mOperationalDataset;
chip::ByteSpan mSSID;
Expand Down

0 comments on commit 27723d5

Please sign in to comment.