Skip to content

Commit

Permalink
Update code PairDevice calls to work with network params. (#14429)
Browse files Browse the repository at this point in the history 
* Update code PairDevice calls to work with network params.

* Better way to do commissioning parameters

That whole "Add" thing was terrible.

* Add pase only option to setup code pairer

* New pairing commands for pase only

I broke the test assumptions.

* Update src/controller/CHIPDeviceController.h

Co-authored-by: Boris Zbarsky <[email protected]>

* Update src/controller/CHIPDeviceController.h

Co-authored-by: Boris Zbarsky <[email protected]>

* Bootstrap the nonces in the auto commissioner.

* Fix initialization

Something somewhere got messed becuase the initialization happens
in the destructor which is not correct for obvious reasons.

* Fix auto documentation.

Functions order was confusing the documentation.

Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
@cecille @bzbarsky-apple @pull
2 people authored and pull[bot] committed Sep 28, 2023
1 parent c8d98c3 commit c47996d
Show file tree
Hide file tree
Showing 9 changed files with 143 additions and 29 deletions.
54 changes: 54 additions & 0 deletions examples/chip-tool/commands/pairing/Commands.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,30 @@ class PairQRCode : public PairingCommand
{}
};

class PairQRCodePase : public PairingCommand
{
public:
PairQRCodePase(CredentialIssuerCommands * credsIssuerConfig) :
PairingCommand("qrcode-paseonly", PairingMode::QRCodePaseOnly, PairingNetworkType::None, credsIssuerConfig)
{}
};

class PairQRCodeWifi : public PairingCommand
{
public:
PairQRCodeWifi(CredentialIssuerCommands * credsIssuerConfig) :
PairingCommand("qrcode-wifi", PairingMode::QRCode, PairingNetworkType::WiFi, credsIssuerConfig)
{}
};

class PairQRCodeThread : public PairingCommand
{
public:
PairQRCodeThread(CredentialIssuerCommands * credsIssuerConfig) :
PairingCommand("qrcode-thread", PairingMode::QRCode, PairingNetworkType::Thread, credsIssuerConfig)
{}
};

class PairManualCode : public PairingCommand
{
public:
Expand All @@ -50,6 +74,30 @@ class PairManualCode : public PairingCommand
{}
};

class PairManualCodePase : public PairingCommand
{
public:
PairManualCodePase(CredentialIssuerCommands * credsIssuerConfig) :
PairingCommand("manualcode-paseonly", PairingMode::ManualCodePaseOnly, PairingNetworkType::None, credsIssuerConfig)
{}
};

class PairManualCodeWifi : public PairingCommand
{
public:
PairManualCodeWifi(CredentialIssuerCommands * credsIssuerConfig) :
PairingCommand("manualcode-wifi", PairingMode::ManualCode, PairingNetworkType::WiFi, credsIssuerConfig)
{}
};

class PairManualCodeThread : public PairingCommand
{
public:
PairManualCodeThread(CredentialIssuerCommands * credsIssuerConfig) :
PairingCommand("manualcode-thread", PairingMode::ManualCode, PairingNetworkType::Thread, credsIssuerConfig)
{}
};

class PairOnNetwork : public PairingCommand
{
public:
Expand Down Expand Up @@ -182,7 +230,13 @@ void registerCommandsPairing(Commands & commands, CredentialIssuerCommands * cre
commands_list clusterCommands = {
make_unique<Unpair>(credsIssuerConfig),
make_unique<PairQRCode>(credsIssuerConfig),
make_unique<PairQRCodePase>(credsIssuerConfig),
make_unique<PairQRCodeWifi>(credsIssuerConfig),
make_unique<PairQRCodeThread>(credsIssuerConfig),
make_unique<PairManualCode>(credsIssuerConfig),
make_unique<PairManualCodePase>(credsIssuerConfig),
make_unique<PairManualCodeWifi>(credsIssuerConfig),
make_unique<PairManualCodeThread>(credsIssuerConfig),
make_unique<PairBleWiFi>(credsIssuerConfig),
make_unique<PairBleThread>(credsIssuerConfig),
make_unique<PairSoftAP>(credsIssuerConfig),
Expand Down
21 changes: 15 additions & 6 deletions examples/chip-tool/commands/pairing/PairingCommand.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,14 @@ CHIP_ERROR PairingCommand::RunInternal(NodeId remoteId)
err = Unpair(remoteId);
break;
case PairingMode::QRCode:
err = PairWithQRCode(remoteId);
err = PairWithCode(remoteId);
break;
case PairingMode::ManualCode:
err = PairWithManualCode(remoteId);
err = PairWithCode(remoteId);
break;
case PairingMode::QRCodePaseOnly:
case PairingMode::ManualCodePaseOnly:
err = PaseWithCode(remoteId);
break;
case PairingMode::Ble:
err = Pair(remoteId, PeerAddress::BLE());
Expand Down Expand Up @@ -83,14 +87,15 @@ CommissioningParameters PairingCommand::GetCommissioningParameters()
return CommissioningParameters();
}

CHIP_ERROR PairingCommand::PairWithQRCode(NodeId remoteId)
CHIP_ERROR PairingCommand::PaseWithCode(NodeId remoteId)
{
return CurrentCommissioner().PairDevice(remoteId, mOnboardingPayload);
return CurrentCommissioner().EstablishPASEConnection(remoteId, mOnboardingPayload);
}

CHIP_ERROR PairingCommand::PairWithManualCode(NodeId remoteId)
CHIP_ERROR PairingCommand::PairWithCode(NodeId remoteId)
{
return CurrentCommissioner().PairDevice(remoteId, mOnboardingPayload);
CommissioningParameters commissioningParams = GetCommissioningParameters();
return CurrentCommissioner().PairDevice(remoteId, mOnboardingPayload, commissioningParams);
}

CHIP_ERROR PairingCommand::Pair(NodeId remoteId, PeerAddress address)
Expand Down Expand Up @@ -155,6 +160,10 @@ void PairingCommand::OnPairingComplete(CHIP_ERROR err)
if (err == CHIP_NO_ERROR)
{
ChipLogProgress(chipTool, "Pairing Success");
if (mPairingMode == PairingMode::QRCodePaseOnly || mPairingMode == PairingMode::ManualCodePaseOnly)
{
SetCommandExitStatus(err);
}
}
else
{
Expand Down
12 changes: 6 additions & 6 deletions examples/chip-tool/commands/pairing/PairingCommand.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ enum class PairingMode
None,
QRCode,
ManualCode,
QRCodePaseOnly,
ManualCodePaseOnly,
Ble,
SoftAP,
Ethernet,
Expand Down Expand Up @@ -79,6 +81,8 @@ class PairingCommand : public CHIPCommand,
break;
case PairingMode::QRCode:
case PairingMode::ManualCode:
case PairingMode::QRCodePaseOnly:
case PairingMode::ManualCodePaseOnly:
AddArgument("payload", &mOnboardingPayload);
break;
case PairingMode::Ble:
Expand Down Expand Up @@ -147,9 +151,8 @@ class PairingCommand : public CHIPCommand,
CHIP_ERROR RunInternal(NodeId remoteId);
CHIP_ERROR Pair(NodeId remoteId, PeerAddress address);
CHIP_ERROR PairWithMdns(NodeId remoteId);
CHIP_ERROR PairWithQRCode(NodeId remoteId);
CHIP_ERROR PairWithManualCode(NodeId remoteId);
CHIP_ERROR PairWithCode(NodeId remoteId, chip::SetupPayload payload);
CHIP_ERROR PairWithCode(NodeId remoteId);
CHIP_ERROR PaseWithCode(NodeId remoteId);
CHIP_ERROR Unpair(NodeId remoteId);
chip::Controller::CommissioningParameters GetCommissioningParameters();

Expand All @@ -167,7 +170,4 @@ class PairingCommand : public CHIPCommand,
char * mOnboardingPayload;
uint64_t mDiscoveryFilterCode;
char * mDiscoveryFilterInstanceName;

chip::CommissioneeDeviceProxy * mDevice;
chip::EndpointId mEndpointId = 0;
};
5 changes: 5 additions & 0 deletions src/controller/AutoCommissioner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
namespace chip {
namespace Controller {

AutoCommissioner::AutoCommissioner()
{
SetCommissioningParameters(CommissioningParameters());
}

AutoCommissioner::~AutoCommissioner()
{
ReleaseDAC();
Expand Down
2 changes: 1 addition & 1 deletion src/controller/AutoCommissioner.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class DeviceCommissioner;
class AutoCommissioner : public CommissioningDelegate
{
public:
AutoCommissioner() {}
AutoCommissioner();
virtual ~AutoCommissioner();
CHIP_ERROR SetCommissioningParameters(const CommissioningParameters & params) override;
void SetOperationalCredentialsDelegate(OperationalCredentialsDelegate * operationalCredentialsDelegate) override;
Expand Down
29 changes: 20 additions & 9 deletions src/controller/CHIPDeviceController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -725,15 +725,21 @@ CHIP_ERROR DeviceCommissioner::GetConnectedDevice(NodeId deviceId, chip::Callbac
return DeviceController::GetConnectedDevice(deviceId, onConnection, onFailure);
}

CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, const char * setUpCode, const CommissioningParameters & params)
{
ReturnErrorOnFailure(mAutoCommissioner.SetCommissioningParameters(params));
return mSetUpCodePairer.PairDevice(remoteDeviceId, setUpCode, SetupCodePairerBehaviour::kCommission);
}

CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, const char * setUpCode)
{
return mSetUpCodePairer.PairDevice(remoteDeviceId, setUpCode);
return mSetUpCodePairer.PairDevice(remoteDeviceId, setUpCode, SetupCodePairerBehaviour::kCommission);
}

CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParameters & params)
{
CommissioningParameters commissioningParams;
return PairDevice(remoteDeviceId, params, commissioningParams);
ReturnErrorOnFailure(EstablishPASEConnection(remoteDeviceId, params));
return Commission(remoteDeviceId);
}

CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParameters & rendezvousParams,
Expand All @@ -743,6 +749,11 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam
return Commission(remoteDeviceId, commissioningParams);
}

CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, const char * setUpCode)
{
return mSetUpCodePairer.PairDevice(remoteDeviceId, setUpCode, SetupCodePairerBehaviour::kPaseOnly);
}

CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, RendezvousParameters & params)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Expand Down Expand Up @@ -853,6 +864,12 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
}

CHIP_ERROR DeviceCommissioner::Commission(NodeId remoteDeviceId, CommissioningParameters & params)
{
ReturnErrorOnFailure(mAutoCommissioner.SetCommissioningParameters(params));
return Commission(remoteDeviceId);
}

CHIP_ERROR DeviceCommissioner::Commission(NodeId remoteDeviceId)
{
// TODO(cecille): Can we get rid of mDeviceBeingCommissioned and use the remote id instead? Would require storing the
// commissioning stage in the device.
Expand All @@ -869,18 +886,12 @@ CHIP_ERROR DeviceCommissioner::Commission(NodeId remoteDeviceId, CommissioningPa
ChipLogError(Controller, "Commissioning already in progress - not restarting");
return CHIP_ERROR_INCORRECT_STATE;
}
if (!params.GetWiFiCredentials().HasValue() && !params.GetThreadOperationalDataset().HasValue() && !mIsIPRendezvous)
{
ChipLogError(Controller, "Network commissioning parameters are required for BLE auto commissioning.");
return CHIP_ERROR_INVALID_ARGUMENT;
}
ChipLogProgress(Controller, "Commission called for node ID 0x" ChipLogFormatX64, ChipLogValueX64(remoteDeviceId));

mSystemState->SystemLayer()->StartTimer(chip::System::Clock::Milliseconds32(kSessionEstablishmentTimeout),
OnSessionEstablishmentTimeoutCallback, this);

mDefaultCommissioner->SetOperationalCredentialsDelegate(mOperationalCredentialsDelegate);
ReturnErrorOnFailure(mDefaultCommissioner->SetCommissioningParameters(params));
if (device->IsSecureConnected())
{
mDefaultCommissioner->StartCommissioning(this, device);
Expand Down
22 changes: 22 additions & 0 deletions src/controller/CHIPDeviceController.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
* @param[in] setUpCode The setup code for connecting to the device
*/
CHIP_ERROR PairDevice(NodeId remoteDeviceId, const char * setUpCode);
CHIP_ERROR PairDevice(NodeId remoteDeviceId, const char * setUpCode, const CommissioningParameters & CommissioningParameters);

/**
* @brief
Expand Down Expand Up @@ -510,6 +511,26 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
*/
CHIP_ERROR EstablishPASEConnection(NodeId remoteDeviceId, RendezvousParameters & params);

/**
* @brief
* Start establishing a PASE connection with a node for the purposes of commissioning.
* Commissioners that wish to use the auto-commissioning functions should use the
* supplied "PairDevice" functions above to automatically establish a connection then
* perform commissioning. This function is intended to be used by commissioners that
* are not using the supplied auto-commissioner.
*
* This function is non-blocking. PASE is established once the DevicePairingDelegate
* receives the OnPairingComplete call.
*
* PASE connections can only be established with nodes that have their commissioning
* window open. The PASE connection will fail if this window is not open and in that case
* OnPairingComplete will be called with an error.
*
* @param[in] remoteDeviceId The remote device Id.
* @param[in] setUpCode The setup code for connecting to the device
*/
CHIP_ERROR EstablishPASEConnection(NodeId remoteDeviceId, const char * setUpCode);

/**
* @brief
* Start the auto-commissioning process on a node after establishing a PASE connection.
Expand All @@ -523,6 +544,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
* @param[in] params The commissioning parameters
*/
CHIP_ERROR Commission(NodeId remoteDeviceId, CommissioningParameters & params);
CHIP_ERROR Commission(NodeId remoteDeviceId);

CHIP_ERROR GetDeviceBeingCommissioned(NodeId deviceId, CommissioneeDeviceProxy ** device);

Expand Down
16 changes: 11 additions & 5 deletions src/controller/SetUpCodePairer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@
namespace chip {
namespace Controller {

CHIP_ERROR SetUpCodePairer::PairDevice(NodeId remoteId, const char * setUpCode)
CHIP_ERROR SetUpCodePairer::PairDevice(NodeId remoteId, const char * setUpCode, SetupCodePairerBehaviour commission)
{
SetupPayload payload;
mConnectionType = commission;

bool isQRCode = strncmp(setUpCode, kQRCodePrefix, strlen(kQRCodePrefix)) == 0;
ReturnErrorOnFailure(isQRCode ? QRCodeSetupPayloadParser(setUpCode).populatePayload(payload)
Expand Down Expand Up @@ -134,7 +135,14 @@ CHIP_ERROR SetUpCodePairer::StopConnectOverSoftAP()

void SetUpCodePairer::OnDeviceDiscovered(RendezvousParameters & params)
{
LogErrorOnFailure(mCommissioner->PairDevice(mRemoteId, params.SetSetupPINCode(mSetUpPINCode)));
if (mConnectionType == SetupCodePairerBehaviour::kCommission)
{
LogErrorOnFailure(mCommissioner->PairDevice(mRemoteId, params.SetSetupPINCode(mSetUpPINCode)));
}
else
{
LogErrorOnFailure(mCommissioner->EstablishPASEConnection(mRemoteId, params.SetSetupPINCode(mSetUpPINCode)));
}
}

#if CONFIG_NETWORK_LAYER_BLE
Expand All @@ -145,9 +153,7 @@ void SetUpCodePairer::OnDiscoveredDeviceOverBle(BLE_CONNECTION_OBJECT connObj)

Transport::PeerAddress peerAddress = Transport::PeerAddress::BLE();
RendezvousParameters params = RendezvousParameters().SetPeerAddress(peerAddress).SetConnectionObject(connObj);
// We don't have network credentials, so can't do the entire pairing flow. Just establish a PASE session to the
// device and let our consumer deal with the rest.
LogErrorOnFailure(mCommissioner->EstablishPASEConnection(mRemoteId, params.SetSetupPINCode(mSetUpPINCode)));
OnDeviceDiscovered(params);
}

void SetUpCodePairer::OnDiscoveredDeviceOverBleSuccess(void * appState, BLE_CONNECTION_OBJECT connObj)
Expand Down
11 changes: 9 additions & 2 deletions src/controller/SetUpCodePairer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,19 @@ namespace Controller {

class DeviceCommissioner;

enum class SetupCodePairerBehaviour : uint8_t
{
kCommission,
kPaseOnly,
};
class DLL_EXPORT SetUpCodePairer
{
public:
SetUpCodePairer(DeviceCommissioner * commissioner) : mCommissioner(commissioner) {}
virtual ~SetUpCodePairer() {}

CHIP_ERROR PairDevice(chip::NodeId remoteId, const char * setUpCode);
CHIP_ERROR PairDevice(chip::NodeId remoteId, const char * setUpCode,
SetupCodePairerBehaviour connectionType = SetupCodePairerBehaviour::kCommission);

// Called by the DeviceCommissioner to notify that we have discovered a new device.
void NotifyCommissionableDeviceDiscovered(const chip::Dnssd::DiscoveredNodeData & nodeData);
Expand Down Expand Up @@ -84,7 +90,8 @@ class DLL_EXPORT SetUpCodePairer

DeviceCommissioner * mCommissioner = nullptr;
chip::NodeId mRemoteId;
uint32_t mSetUpPINCode = 0;
uint32_t mSetUpPINCode = 0;
SetupCodePairerBehaviour mConnectionType = SetupCodePairerBehaviour::kCommission;
};

} // namespace Controller
Expand Down

0 comments on commit c47996d

Please sign in to comment.