Skip to content

Commit

Permalink
Fix discriminator handling in chip-tool pairing commands. (project-ch…
Browse files Browse the repository at this point in the history
…ip#34115)

Not all commands initialize the discriminator value, and the ones that don't should not use it.

Fixes project-chip#34096
  • Loading branch information
bzbarsky-apple authored Jun 27, 2024
1 parent 3fc8d67 commit 03ae36e
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 @@ -216,7 +216,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 @@ -236,9 +241,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 @@ -258,6 +265,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 @@ -281,7 +292,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 @@ -107,32 +109,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 @@ -252,8 +254,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 03ae36e

Please sign in to comment.