Skip to content

Commit

Permalink
Fixes for open pairing window on chip-tool (#9806)
Browse files Browse the repository at this point in the history 
It looks like the previous version would generate a random nodeid
and then attempt to open the pairing window on that. Issue #9550
indicates a general failure to send commands after provisioning,
but it actually looks like the open pairing window command is the
only one affected. Issue speculates that PersistDevice was not
called, but confirmed that PersistDevice IS called. However,
the data for a randomly generated node id would not be present
so the symptoms may match.

We also need to connect to the device before sending the open
pairing windo command as it is a standard cluster command. Added
calls to connect to the device before opening the pairing window.

Test:
./chip-tool pairing onnetwork 0 20202021 3840 fc00::a 5540
./chip-tool onoff on 1
./chip-tool pairing open-commissioning-window 0 5000 1 3840
./chip-tool pairing onnetwork 0 20202021 3840 fc00::a 5540
./chip-tool onoff on 1
  • Loading branch information
@cecille @pull
cecille authored and pull[bot] committed Sep 22, 2021
1 parent f1e77be commit 5333473
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 22 deletions.
60 changes: 41 additions & 19 deletions examples/chip-tool/commands/pairing/PairingCommand.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,24 +39,27 @@ CHIP_ERROR PairingCommand::Run()
GetExecContext()->commissioner->RegisterDeviceAddressUpdateDelegate(this);
GetExecContext()->commissioner->RegisterPairingDelegate(this);

if (mPairingMode != PairingMode::OpenCommissioningWindow)
{
#if CONFIG_PAIR_WITH_RANDOM_ID
// Generate a random remote id so we don't end up reusing the same node id
// for different nodes.
//
// TODO: Ideally we'd just ask for an operational cert for the commissionnee
// and get the node from that, but the APIs are not set up that way yet.
NodeId randomId;
ReturnErrorOnFailure(Controller::ExampleOperationalCredentialsIssuer::GetRandomOperationalNodeId(&randomId));

ChipLogProgress(Controller, "Generated random node id: 0x" ChipLogFormatX64, ChipLogValueX64(randomId));

ReturnErrorOnFailure(GetExecContext()->storage->SetRemoteNodeId(randomId));
GetExecContext()->remoteId = randomId;
// Generate a random remote id so we don't end up reusing the same node id
// for different nodes.
//
// TODO: Ideally we'd just ask for an operational cert for the commissionnee
// and get the node from that, but the APIs are not set up that way yet.
NodeId randomId;
ReturnErrorOnFailure(Controller::ExampleOperationalCredentialsIssuer::GetRandomOperationalNodeId(&randomId));

ChipLogProgress(Controller, "Generated random node id: 0x" ChipLogFormatX64, ChipLogValueX64(randomId));

ReturnErrorOnFailure(GetExecContext()->storage->SetRemoteNodeId(randomId));
GetExecContext()->remoteId = randomId;
#else // CONFIG_PAIR_WITH_RANDOM_ID
// Use the default id, not whatever happens to be in our storage, since this
// is a new pairing.
GetExecContext()->remoteId = kTestDeviceNodeId;
// Use the default id, not whatever happens to be in our storage, since this
// is a new pairing.
GetExecContext()->remoteId = kTestDeviceNodeId;
#endif // CONFIG_PAIR_WITH_RANDOM_ID
}

err = RunInternal(GetExecContext()->remoteId);
VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Init Failure! PairDevice: %s", ErrorStr(err)));
Expand Down Expand Up @@ -98,13 +101,32 @@ CHIP_ERROR PairingCommand::RunInternal(NodeId remoteId)
err = Pair(remoteId, PeerAddress::UDP(mRemoteAddr.address, mRemotePort));
break;
case PairingMode::OpenCommissioningWindow:
err = OpenCommissioningWindow(remoteId, mTimeout, mIteration, mDiscriminator, mCommissioningWindowOption);
err = GetExecContext()->commissioner->GetConnectedDevice(GetExecContext()->remoteId, &mOnDeviceConnectedCallback,
&mOnDeviceConnectionFailureCallback);
if (err != CHIP_NO_ERROR)
{
ChipLogError(chipTool, "Failed in initiating connection to the device: %" PRIu64 ", error %" CHIP_ERROR_FORMAT,
GetExecContext()->remoteId, err.Format());
}

break;
}

return err;
}

void PairingCommand::OnDeviceConnectedFn(void * context, chip::Controller::Device * device)
{
PairingCommand * command = reinterpret_cast<PairingCommand *>(context);
command->OpenCommissioningWindow();
}
void PairingCommand::OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error)
{
PairingCommand * command = reinterpret_cast<PairingCommand *>(context);
ChipLogError(chipTool, "Failed in connecting to the device %" PRIu64 ". Error %" CHIP_ERROR_FORMAT, deviceId, error.Format());
command->SetCommandExitStatus(error);
}

CHIP_ERROR PairingCommand::PairWithQRCode(NodeId remoteId)
{
SetupPayload payload;
Expand Down Expand Up @@ -154,10 +176,10 @@ CHIP_ERROR PairingCommand::Unpair(NodeId remoteId)
return err;
}

CHIP_ERROR PairingCommand::OpenCommissioningWindow(NodeId remoteId, uint16_t timeout, uint16_t iteration, uint16_t discriminator,
uint8_t option)
CHIP_ERROR PairingCommand::OpenCommissioningWindow()
{
CHIP_ERROR err = GetExecContext()->commissioner->OpenCommissioningWindow(remoteId, timeout, iteration, discriminator, option);
CHIP_ERROR err = GetExecContext()->commissioner->OpenCommissioningWindow(GetExecContext()->remoteId, mTimeout, mIteration,
mDiscriminator, mCommissioningWindowOption);
SetCommandExitStatus(err);
return err;
}
Expand Down
12 changes: 9 additions & 3 deletions examples/chip-tool/commands/pairing/PairingCommand.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ class PairingCommand : public Command,
{
public:
PairingCommand(const char * commandName, PairingMode mode, PairingNetworkType networkType) :
Command(commandName), mPairingMode(mode), mNetworkType(networkType), mRemoteAddr{ IPAddress::Any, INET_NULL_INTERFACEID }
Command(commandName), mPairingMode(mode), mNetworkType(networkType), mRemoteAddr{ IPAddress::Any, INET_NULL_INTERFACEID },
mOnDeviceConnectedCallback(OnDeviceConnectedFn, this), mOnDeviceConnectionFailureCallback(OnDeviceConnectionFailureFn, this)
{
switch (networkType)
{
Expand Down Expand Up @@ -139,8 +140,7 @@ class PairingCommand : public Command,
CHIP_ERROR PairWithCode(NodeId remoteId, chip::SetupPayload payload);
CHIP_ERROR PairWithoutSecurity(NodeId remoteId, PeerAddress address);
CHIP_ERROR Unpair(NodeId remoteId);
CHIP_ERROR OpenCommissioningWindow(NodeId remoteId, uint16_t timeout, uint16_t iteration, uint16_t discriminator,
uint8_t option);
CHIP_ERROR OpenCommissioningWindow();

void InitCallbacks();
CHIP_ERROR SetupNetwork();
Expand Down Expand Up @@ -177,4 +177,10 @@ class PairingCommand : public Command,
chip::Controller::NetworkCommissioningCluster mCluster;
chip::EndpointId mEndpointId = 0;
chip::Controller::ExampleOperationalCredentialsIssuer mOpCredsIssuer;

static void OnDeviceConnectedFn(void * context, chip::Controller::Device * device);
static void OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error);

chip::Callback::Callback<chip::Controller::OnDeviceConnected> mOnDeviceConnectedCallback;
chip::Callback::Callback<chip::Controller::OnDeviceConnectionFailure> mOnDeviceConnectionFailureCallback;
};

0 comments on commit 5333473

Please sign in to comment.