Skip to content

Commit

Permalink
Fixes related to PASE-only commissioning on Android (project-chip#22140)
Browse files Browse the repository at this point in the history
* Draft: fixes related to PASE-only commissioning on Android

* fix build

* avoid null pointer in cleanup
  • Loading branch information
chrisdecenzo authored and isiu-apple committed Sep 16, 2022
1 parent 0015df3 commit a4f60a9
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 7 deletions.
18 changes: 18 additions & 0 deletions src/controller/AutoCommissioner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam
}
mParams.SetCSRNonce(ByteSpan(mCSRNonce, sizeof(mCSRNonce)));

if (params.GetSkipCommissioningComplete().HasValue())
{
ChipLogProgress(Controller, "Setting PASE-only commissioning from parameters");
mParams.SetSkipCommissioningComplete(params.GetSkipCommissioningComplete().Value());
}

return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -252,6 +258,10 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
}
else
{
if (mParams.GetSkipCommissioningComplete().ValueOr(false))
{
return CommissioningStage::kCleanup;
}
return CommissioningStage::kFindOperational;
}
case CommissioningStage::kWiFiNetworkSetup:
Expand Down Expand Up @@ -280,11 +290,19 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
{
return CommissioningStage::kThreadNetworkEnable;
}
else if (mParams.GetSkipCommissioningComplete().ValueOr(false))
{
return CommissioningStage::kCleanup;
}
else
{
return CommissioningStage::kFindOperational;
}
case CommissioningStage::kThreadNetworkEnable:
if (mParams.GetSkipCommissioningComplete().ValueOr(false))
{
return CommissioningStage::kCleanup;
}
return CommissioningStage::kFindOperational;
case CommissioningStage::kFindOperational:
return CommissioningStage::kSendComplete;
Expand Down
1 change: 0 additions & 1 deletion src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1572,7 +1572,6 @@ void DeviceCommissioner::CommissioningStageComplete(CHIP_ERROR err, Commissionin
{
// Once this stage is complete, reset mDeviceBeingCommissioned - this will be reset when the delegate calls the next step.
MATTER_TRACE_EVENT_SCOPE("CommissioningStageComplete", "DeviceCommissioner");

if (mDeviceBeingCommissioned == nullptr)
{
// We are getting a stray callback (e.g. due to un-cancellable
Expand Down
4 changes: 4 additions & 0 deletions src/controller/CommissioningDelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ const char * StageToString(CommissioningStage stage)
return "Cleanup";
break;

case kNeedsNetworkCreds:
return "NeedsNetworkCreds";
break;

default:
return "???";
break;
Expand Down
10 changes: 10 additions & 0 deletions src/controller/CommissioningDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,15 @@ class CommissioningParameters
return *this;
}

// Only perform the PASE steps of commissioning.
// Commissioning will be completed by another admin on the network.
Optional<bool> GetSkipCommissioningComplete() const { return mSkipCommissioningComplete; }
CommissioningParameters & SetSkipCommissioningComplete(bool skipCommissioningComplete)
{
mSkipCommissioningComplete = MakeOptional(skipCommissioningComplete);
return *this;
}

private:
// Items that can be set by the commissioner
Optional<uint16_t> mFailsafeTimerSeconds;
Expand Down Expand Up @@ -407,6 +416,7 @@ class CommissioningParameters
nullptr; // Delegate to handle device attestation failures during commissioning
Optional<bool> mAttemptWiFiNetworkScan;
Optional<bool> mAttemptThreadNetworkScan; // This automatically gets set to false when a ThreadOperationalDataset is set
Optional<bool> mSkipCommissioningComplete;
};

struct RequestedCertificate
Expand Down
3 changes: 2 additions & 1 deletion src/controller/java/AndroidDeviceControllerWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew(
chip::Inet::EndPointManager<Inet::UDPEndPoint> * udpEndPointManager, AndroidOperationalCredentialsIssuerPtr opCredsIssuerPtr,
jobject keypairDelegate, jbyteArray rootCertificate, jbyteArray intermediateCertificate, jbyteArray nodeOperationalCertificate,
jbyteArray ipkEpochKey, uint16_t listenPort, uint16_t controllerVendorId, uint16_t failsafeTimerSeconds,
bool attemptNetworkScanWiFi, bool attemptNetworkScanThread, CHIP_ERROR * errInfoOnFailure)
bool attemptNetworkScanWiFi, bool attemptNetworkScanThread, bool skipCommissioningComplete, CHIP_ERROR * errInfoOnFailure)
{
if (errInfoOnFailure == nullptr)
{
Expand Down Expand Up @@ -156,6 +156,7 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew(
params.SetFailsafeTimerSeconds(failsafeTimerSeconds);
params.SetAttemptWiFiNetworkScan(attemptNetworkScanWiFi);
params.SetAttemptThreadNetworkScan(attemptNetworkScanThread);
params.SetSkipCommissioningComplete(skipCommissioningComplete);
wrapper->UpdateCommissioningParameters(params);

CHIP_ERROR err = wrapper->mGroupDataProvider.Init();
Expand Down
3 changes: 2 additions & 1 deletion src/controller/java/AndroidDeviceControllerWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ class AndroidDeviceControllerWrapper : public chip::Controller::DevicePairingDel
* @param[in] failsafeTimerSeconds the failsafe timer in seconds
* @param[in] attemptNetworkScanWiFi whether to attempt a network scan when configuring the network for a WiFi device
* @param[in] attemptNetworkScanThread whether to attempt a network scan when configuring the network for a Thread device
* @param[in] skipCommissioningComplete whether to skip the CASE commissioningComplete command during commissioning
* @param[out] errInfoOnFailure a pointer to a CHIP_ERROR that will be populated if this method returns nullptr
*/
static AndroidDeviceControllerWrapper *
Expand All @@ -148,7 +149,7 @@ class AndroidDeviceControllerWrapper : public chip::Controller::DevicePairingDel
AndroidOperationalCredentialsIssuerPtr opCredsIssuer, jobject keypairDelegate, jbyteArray rootCertificate,
jbyteArray intermediateCertificate, jbyteArray nodeOperationalCertificate, jbyteArray ipkEpochKey,
uint16_t listenPort, uint16_t controllerVendorId, uint16_t failsafeTimerSeconds, bool attemptNetworkScanWiFi,
bool attemptNetworkScanThread, CHIP_ERROR * errInfoOnFailure);
bool attemptNetworkScanThread, bool skipCommissioningComplete, CHIP_ERROR * errInfoOnFailure);

chip::Controller::AndroidOperationalCredentialsIssuer * GetAndroidOperationalCredentialsIssuer()
{
Expand Down
15 changes: 11 additions & 4 deletions src/controller/java/CHIPDeviceController-JNI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,11 @@ JNI_METHOD(jlong, newDeviceController)(JNIEnv * env, jobject self, jobject contr
&getAttemptNetworkScanThread);
SuccessOrExit(err);

jmethodID getSkipCommissioningComplete;
err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getSkipCommissioningComplete", "()Z",
&getSkipCommissioningComplete);
SuccessOrExit(err);

jmethodID getKeypairDelegate;
err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getKeypairDelegate",
"()Lchip/devicecontroller/KeypairDelegate;", &getKeypairDelegate);
Expand Down Expand Up @@ -324,6 +329,7 @@ JNI_METHOD(jlong, newDeviceController)(JNIEnv * env, jobject self, jobject contr
uint16_t failsafeTimerSeconds = env->CallIntMethod(controllerParams, getFailsafeTimerSeconds);
bool attemptNetworkScanWiFi = env->CallBooleanMethod(controllerParams, getAttemptNetworkScanWiFi);
bool attemptNetworkScanThread = env->CallBooleanMethod(controllerParams, getAttemptNetworkScanThread);
bool skipCommissioningComplete = env->CallBooleanMethod(controllerParams, getSkipCommissioningComplete);
uint64_t adminSubject = env->CallLongMethod(controllerParams, getAdminSubject);

std::unique_ptr<chip::Controller::AndroidOperationalCredentialsIssuer> opCredsIssuer(
Expand All @@ -332,7 +338,7 @@ JNI_METHOD(jlong, newDeviceController)(JNIEnv * env, jobject self, jobject contr
sJVM, self, kLocalDeviceId, fabricId, chip::kUndefinedCATs, &DeviceLayer::SystemLayer(),
DeviceLayer::TCPEndPointManager(), DeviceLayer::UDPEndPointManager(), std::move(opCredsIssuer), keypairDelegate,
rootCertificate, intermediateCertificate, operationalCertificate, ipk, listenPort, controllerVendorId,
failsafeTimerSeconds, attemptNetworkScanWiFi, attemptNetworkScanThread, &err);
failsafeTimerSeconds, attemptNetworkScanWiFi, attemptNetworkScanThread, skipCommissioningComplete, &err);
SuccessOrExit(err);

if (adminSubject != kUndefinedNodeId)
Expand Down Expand Up @@ -384,7 +390,7 @@ JNI_METHOD(void, commissionDevice)

ChipLogProgress(Controller, "commissionDevice() called");

CommissioningParameters commissioningParams = CommissioningParameters();
CommissioningParameters commissioningParams = wrapper->GetCommissioningParameters();
if (networkCredentials != nullptr)
{
err = wrapper->ApplyNetworkCredentials(commissioningParams, networkCredentials);
Expand Down Expand Up @@ -422,7 +428,7 @@ JNI_METHOD(void, pairDevice)
#endif
.SetPeerAddress(Transport::PeerAddress::BLE());

CommissioningParameters commissioningParams = CommissioningParameters();
CommissioningParameters commissioningParams = wrapper->GetCommissioningParameters();
wrapper->ApplyNetworkCredentials(commissioningParams, networkCredentials);

if (csrNonce != nullptr)
Expand Down Expand Up @@ -456,7 +462,8 @@ JNI_METHOD(void, pairDeviceWithAddress)
.SetDiscriminator(discriminator)
.SetSetupPINCode(pinCode)
.SetPeerAddress(Transport::PeerAddress::UDP(const_cast<char *>(addrJniString.c_str()), port));
CommissioningParameters commissioningParams = CommissioningParameters();

CommissioningParameters commissioningParams = wrapper->GetCommissioningParameters();
if (csrNonce != nullptr)
{
JniByteArray jniCsrNonce(env, csrNonce);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public final class ControllerParams {
private final int failsafeTimerSeconds;
private final boolean attemptNetworkScanWiFi;
private final boolean attemptNetworkScanThread;
private final boolean skipCommissioningComplete;
@Nullable private final KeypairDelegate keypairDelegate;
@Nullable private final byte[] rootCertificate;
@Nullable private final byte[] intermediateCertificate;
Expand All @@ -28,6 +29,7 @@ private ControllerParams(Builder builder) {
this.failsafeTimerSeconds = builder.failsafeTimerSeconds;
this.attemptNetworkScanWiFi = builder.attemptNetworkScanWiFi;
this.attemptNetworkScanThread = builder.attemptNetworkScanThread;
this.skipCommissioningComplete = builder.skipCommissioningComplete;
this.keypairDelegate = builder.keypairDelegate;
this.rootCertificate = builder.rootCertificate;
this.intermediateCertificate = builder.intermediateCertificate;
Expand Down Expand Up @@ -61,6 +63,10 @@ public boolean getAttemptNetworkScanThread() {
return attemptNetworkScanThread;
}

public boolean getSkipCommissioningComplete() {
return skipCommissioningComplete;
}

public KeypairDelegate getKeypairDelegate() {
return keypairDelegate;
}
Expand Down Expand Up @@ -112,6 +118,7 @@ public static class Builder {
private int failsafeTimerSeconds = 30;
private boolean attemptNetworkScanWiFi = false;
private boolean attemptNetworkScanThread = false;
private boolean skipCommissioningComplete = false;
@Nullable private KeypairDelegate keypairDelegate = null;
@Nullable private byte[] rootCertificate = null;
@Nullable private byte[] intermediateCertificate = null;
Expand Down Expand Up @@ -197,6 +204,24 @@ public Builder setAttemptNetworkScanThread(boolean attemptNetworkScanThread) {
return this;
}

/**
* Disable the CASE phase of commissioning when the CommissioningComplete command is sent by
* this ChipDeviceCommissioner.
*
* <p>Specifically, this sets SkipCommissioningComplete in the CommissioningParameters passed to
* the CommissioningDelegate.
*
* <p>A controller will set this to true when the CASE phase of commissioning is done by a
* separate process, for example, by a Hub on the network.
*
* @param skipCommissioningComplete
* @return
*/
public Builder setSkipCommissioningComplete(boolean skipCommissioningComplete) {
this.skipCommissioningComplete = skipCommissioningComplete;
return this;
}

public Builder setKeypairDelegate(KeypairDelegate keypairDelegate) {
this.keypairDelegate = keypairDelegate;
return this;
Expand Down

0 comments on commit a4f60a9

Please sign in to comment.