NFC support for SetUpCodePairer and AutoCommissioner (#40783)
* NFC support for SetUpCodePairer and AutoCommissioner
- Support for NFC discovery in SetUpCodePairer
- AutoCommissioner support for two phase stage set for NFC based
commissioning
- Added a new kUnpoweredStageComplete to mark end phase1 of
commissioning. This allows Commissioner to remember this for the
device and use the new API (ContinueCommissioningAfterUnpoweredPhaseComplete)
to continue commissioning post powerup of the device.
* Code review comments
* Restyled by clang-format
* Code review fixes
* Restyled by clang-format
* Renamed API to support existing pattern
- AutoCommissioner has SkipCommissioningComplete flags. Renamed
API to stage name to allow using it if flag is a set as well
* Fixed tracing string to match API name
---------
Co-authored-by: Restyled.io <commits@restyled.io>
diff --git a/src/controller/AutoCommissioner.cpp b/src/controller/AutoCommissioner.cpp
index 77b964b..360e67e 100644
--- a/src/controller/AutoCommissioner.cpp
+++ b/src/controller/AutoCommissioner.cpp
@@ -487,7 +487,25 @@
}
return CommissioningStage::kEvictPreviousCaseSessions;
case CommissioningStage::kEvictPreviousCaseSessions:
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ {
+ // If there is no secure session, means commissioning has been continued after the unpowered phase was completed
+ // In such case, move on setup the CASE session over operational network
+ if (!mCommissioneeDeviceProxy->GetSecureSession().HasValue())
+ {
+ return CommissioningStage::kFindOperationalForStayActive;
+ }
+
+ // If the transport is NFC, move to unpowered phase complete to end the first phase of commissioning
+ auto transportType =
+ mCommissioneeDeviceProxy->GetSecureSession().Value()->AsSecureSession()->GetPeerAddress().GetTransportType();
+ return (transportType == Transport::Type::kNfc && mDeviceCommissioningInfo.general.isCommissioningWithoutPower)
+ ? CommissioningStage::kUnpoweredPhaseComplete
+ : CommissioningStage::kFindOperationalForStayActive;
+ }
+#else
return CommissioningStage::kFindOperationalForStayActive;
+#endif
case CommissioningStage::kPrimaryOperationalNetworkFailed:
if (mDeviceCommissioningInfo.network.wifi.endpoint == kRootEndpointId)
{
@@ -508,6 +526,10 @@
return CommissioningStage::kSendComplete;
case CommissioningStage::kSendComplete:
return CommissioningStage::kCleanup;
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ case CommissioningStage::kUnpoweredPhaseComplete:
+ return CommissioningStage::kCleanup; // End commissioning (phase 1)
+#endif
// Neither of these have a next stage so return kError;
case CommissioningStage::kCleanup:
@@ -578,7 +600,14 @@
return CHIP_ERROR_INVALID_ARGUMENT;
}
- if (proxy == nullptr || !proxy->GetSecureSession().HasValue())
+ // Proxy is expected to have a valid secure session before starting to commission. However, in case of continuing
+ // commissioning post unpowered phase, allow proceeding if the stage is evict secure CASE sessions
+ if (proxy == nullptr ||
+ (!proxy->GetSecureSession().HasValue()
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ && commissioner->GetCommissioningStage() != CommissioningStage::kEvictPreviousCaseSessions
+#endif
+ ))
{
ChipLogError(Controller, "Device proxy secure session error");
return CHIP_ERROR_INVALID_ARGUMENT;
@@ -587,8 +616,15 @@
mCommissioner = commissioner;
mCommissioneeDeviceProxy = proxy;
- auto transportType =
- mCommissioneeDeviceProxy->GetSecureSession().Value()->AsSecureSession()->GetPeerAddress().GetTransportType();
+ // When commissioning is started after unpowered phase, there will be no secure session since the CASE session first needs to be
+ // setup over operational network. Hence assume transport is UDP in such case.
+ auto transportType = Transport::Type::kUdp;
+ if (mCommissioneeDeviceProxy->GetSecureSession().HasValue())
+ {
+ transportType =
+ mCommissioneeDeviceProxy->GetSecureSession().Value()->AsSecureSession()->GetPeerAddress().GetTransportType();
+ }
+
mNeedsNetworkSetup = (transportType == Transport::Type::kBle);
#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
mNeedsNetworkSetup = mNeedsNetworkSetup || (transportType == Transport::Type::kNfc);
@@ -597,7 +633,7 @@
mNeedsNetworkSetup = mNeedsNetworkSetup || (transportType == Transport::Type::kWiFiPAF);
#endif
CHIP_ERROR err = CHIP_NO_ERROR;
- CommissioningStage nextStage = GetNextCommissioningStage(CommissioningStage::kSecurePairing, err);
+ CommissioningStage nextStage = GetNextCommissioningStage(commissioner->GetCommissioningStage(), err);
mCommissioner->PerformCommissioningStep(mCommissioneeDeviceProxy, nextStage, mParams, this, GetEndpoint(nextStage),
GetCommandTimeout(mCommissioneeDeviceProxy, nextStage));
diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp
index da0ae9e..a2aac86 100644
--- a/src/controller/CHIPDeviceController.cpp
+++ b/src/controller/CHIPDeviceController.cpp
@@ -70,6 +70,10 @@
#include <transport/raw/WiFiPAF.h>
#endif
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+#include <platform/internal/NFCCommissioningManager.h>
+#endif
+
#include <algorithm>
#include <array>
#include <errno.h>
@@ -632,6 +636,16 @@
mSystemState->BleLayer()->CloseAllBleConnections();
}
#endif
+
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ Nfc::NFCReaderTransport * readerTransport = DeviceLayer::Internal::NFCCommissioningMgr().GetNFCReaderTransport();
+ if (readerTransport)
+ {
+ ChipLogProgress(Controller, "Stopping discovery of all NFC tags");
+ readerTransport->StopDiscoveringTags();
+ }
+#endif
+
// Make sure that there will be no dangling pointer
if (mDeviceInPASEEstablishment == device)
{
@@ -1071,6 +1085,49 @@
return CHIP_NO_ERROR;
}
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+CHIP_ERROR DeviceCommissioner::ContinueCommissioningAfterConnectNetworkRequest(NodeId remoteDeviceId)
+{
+ MATTER_TRACE_SCOPE("continueCommissioningAfterConnectNetworkRequest", "DeviceCommissioner");
+
+ if (mDefaultCommissioner == nullptr)
+ {
+ ChipLogError(Controller, "No default commissioner is specified");
+ return CHIP_ERROR_INCORRECT_STATE;
+ }
+
+ // Move to kEvictPreviousCaseSessions stage since the next stage will be to find the device
+ // on the operational network
+ mCommissioningStage = CommissioningStage::kEvictPreviousCaseSessions;
+
+ // Setup device being commissioned
+ CommissioneeDeviceProxy * device = nullptr;
+ if (!mDeviceBeingCommissioned)
+ {
+ device = mCommissioneeDevicePool.CreateObject();
+ if (!device)
+ return CHIP_ERROR_NO_MEMORY;
+
+ Transport::PeerAddress peerAddress = Transport::PeerAddress::UDP(Inet::IPAddress::Any);
+ device->Init(GetControllerDeviceInitParams(), remoteDeviceId, peerAddress);
+ mDeviceBeingCommissioned = device;
+ }
+
+ mDefaultCommissioner->SetOperationalCredentialsDelegate(mOperationalCredentialsDelegate);
+
+ ChipLogProgress(Controller, "Continuing commissioning after connect to network complete for device ID 0x" ChipLogFormatX64,
+ ChipLogValueX64(remoteDeviceId));
+
+ MATTER_LOG_METRIC_BEGIN(kMetricDeviceCommissioningOperationalSetup);
+ CHIP_ERROR err = mDefaultCommissioner->StartCommissioning(this, device);
+ if (err != CHIP_NO_ERROR)
+ {
+ MATTER_LOG_METRIC_END(kMetricDeviceCommissioningOperationalSetup, err);
+ }
+ return err;
+}
+#endif
+
CHIP_ERROR DeviceCommissioner::StopPairing(NodeId remoteDeviceId)
{
VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE);
@@ -2302,6 +2359,8 @@
Clusters::GeneralCommissioning::Attributes::RegulatoryConfig::Id));
VerifyOrReturn(builder.AddAttributePath(kRootEndpointId, Clusters::GeneralCommissioning::Id,
Clusters::GeneralCommissioning::Attributes::LocationCapability::Id));
+ VerifyOrReturn(builder.AddAttributePath(kRootEndpointId, Clusters::GeneralCommissioning::Id,
+ Clusters::GeneralCommissioning::Attributes::IsCommissioningWithoutPower::Id));
// Basic Information: VID and PID for device attestation purposes
VerifyOrReturn(builder.AddAttributePath(kRootEndpointId, Clusters::BasicInformation::Id,
@@ -2458,6 +2517,13 @@
info.supportsConcurrentConnection = true; // default to true (concurrent), not a fatal error
}
+ err = mAttributeCache->Get<IsCommissioningWithoutPower::TypeInfo>(kRootEndpointId, info.general.isCommissioningWithoutPower);
+ if (err != CHIP_NO_ERROR)
+ {
+ ChipLogError(Controller, "Ignoring failure to read IsCommissioningWithoutPower: %" CHIP_ERROR_FORMAT, err.Format());
+ info.general.isCommissioningWithoutPower = false; // default to false, not a fatal error
+ }
+
return return_err;
}
@@ -3761,6 +3827,12 @@
}
}
break;
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ case CommissioningStage::kUnpoweredPhaseComplete:
+ ChipLogProgress(Controller, "Completed unpowered commissioning phase, marking commissioning as complete");
+ CommissioningStageComplete(CHIP_NO_ERROR);
+ break;
+#endif
case CommissioningStage::kCleanup:
CleanupCommissioning(proxy, proxy->GetDeviceId(), params.GetCompletionStatus());
break;
diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h
index 40f0096..8060031 100644
--- a/src/controller/CHIPDeviceController.h
+++ b/src/controller/CHIPDeviceController.h
@@ -619,6 +619,17 @@
CHIP_ERROR
ContinueCommissioningAfterDeviceAttestation(DeviceProxy * device, Credentials::AttestationVerificationResult attestationResult);
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ /**
+ * @brief
+ * This method instructs the commissioner to proceed to the commissioning complete stage for a device
+ * that had previously being commissioned until request to connect to network.
+ *
+ * @param[in] remoteDeviceId The remote device Id.
+ */
+ CHIP_ERROR ContinueCommissioningAfterConnectNetworkRequest(NodeId remoteDeviceId);
+#endif
+
CHIP_ERROR GetDeviceBeingCommissioned(NodeId deviceId, CommissioneeDeviceProxy ** device);
/**
diff --git a/src/controller/CommissioningDelegate.cpp b/src/controller/CommissioningDelegate.cpp
index ecc608f..7251644 100644
--- a/src/controller/CommissioningDelegate.cpp
+++ b/src/controller/CommissioningDelegate.cpp
@@ -148,6 +148,11 @@
case kRemoveThreadNetworkConfig:
return "RemoveThreadNetworkConfig";
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ case kUnpoweredPhaseComplete:
+ return "UnpoweredPhaseComplete";
+#endif
+
default:
return "???";
}
@@ -263,6 +268,11 @@
case kNeedsNetworkCreds:
return "core_commissioning_stage_need_network_creds";
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ case kUnpoweredPhaseComplete:
+ return "core_commissioning_stage_unpowered_phase";
+#endif
+
default:
return "core_commissioning_stage_unknown";
}
diff --git a/src/controller/CommissioningDelegate.h b/src/controller/CommissioningDelegate.h
index aae437a..f92b5fd 100644
--- a/src/controller/CommissioningDelegate.h
+++ b/src/controller/CommissioningDelegate.h
@@ -89,6 +89,9 @@
kRemoveThreadNetworkConfig, ///< Remove Thread network config.
kConfigureTCAcknowledgments, ///< Send SetTCAcknowledgements (0x30:6) command to the device
kCleanup, ///< Call delegates with status, free memory, clear timers and state/
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ kUnpoweredPhaseComplete, ///< Commissioning completed until connect network for unpowered commissioning (NFC)
+#endif
};
enum class ICDRegistrationStrategy : uint8_t
@@ -768,7 +771,7 @@
app::Clusters::GeneralCommissioning::RegulatoryLocationTypeEnum::kIndoorOutdoor;
app::Clusters::GeneralCommissioning::RegulatoryLocationTypeEnum locationCapability =
app::Clusters::GeneralCommissioning::RegulatoryLocationTypeEnum::kIndoorOutdoor;
- ;
+ bool isCommissioningWithoutPower = false;
};
// ICDManagementClusterInfo is populated when the controller reads information from
diff --git a/src/controller/SetUpCodePairer.cpp b/src/controller/SetUpCodePairer.cpp
index da3ef3d..e86a2fa 100644
--- a/src/controller/SetUpCodePairer.cpp
+++ b/src/controller/SetUpCodePairer.cpp
@@ -30,6 +30,7 @@
#include <lib/dnssd/Resolver.h>
#include <lib/support/CodeUtils.h>
#include <memory>
+#include <platform/internal/NFCCommissioningManager.h>
#include <system/SystemClock.h>
#include <tracing/metric_event.h>
#include <vector>
@@ -129,6 +130,20 @@
err.Format());
}
}
+ if (ShouldDiscoverUsing(RendezvousInformationFlag::kNFC))
+ {
+ CHIP_ERROR err = StartDiscoveryOverNFC();
+ if ((CHIP_ERROR_NOT_IMPLEMENTED == err) || (CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE == err))
+ {
+ ChipLogProgress(Controller,
+ "Skipping commissionable node discovery over NFC since not supported by the controller!");
+ }
+ else if (err != CHIP_NO_ERROR)
+ {
+ ChipLogError(Controller, "Failed to start commissionable node discovery over NFC: %" CHIP_ERROR_FORMAT,
+ err.Format());
+ }
+ }
}
// We always want to search on network because any node that has already been commissioned will use on-network regardless of the
@@ -300,6 +315,66 @@
return CHIP_NO_ERROR;
}
+CHIP_ERROR SetUpCodePairer::StartDiscoveryOverNFC()
+{
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ if (mSetupPayloads.size() != 1)
+ {
+ ChipLogError(Controller, "NFC commissioning does not support concatenated QR codes yet.");
+ return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+ }
+
+ auto & payload = mSetupPayloads[0];
+
+ ChipLogProgress(Controller, "Starting commissionable node discovery over NFC");
+ VerifyOrReturnError(mCommissioner != nullptr, CHIP_ERROR_INCORRECT_STATE);
+
+ const SetupDiscriminator connDiscriminator(payload.discriminator);
+ VerifyOrReturnValue(!connDiscriminator.IsShortDiscriminator(), CHIP_ERROR_INVALID_ARGUMENT,
+ ChipLogError(Controller, "Error, Long discriminator is required"));
+ chip::Nfc::NFCTag::Identifier identifier = { .discriminator = payload.discriminator.GetLongValue() };
+ Nfc::NFCReaderTransport * readerTransport = DeviceLayer::Internal::NFCCommissioningMgr().GetNFCReaderTransport();
+ if (!readerTransport)
+ {
+ ChipLogError(Controller, "Commissionable node discovery over NFC since there is no valid NFC reader transport");
+ return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+ }
+
+ readerTransport->SetDelegate(this);
+ CHIP_ERROR err = readerTransport->StartDiscoveringTagMatchingAddress(identifier);
+ if (err != CHIP_NO_ERROR)
+ {
+ ChipLogError(Controller, "Commissionable node discovery over NFC failed, err = %" CHIP_ERROR_FORMAT, err.Format());
+ }
+ else
+ {
+ mWaitingForDiscovery[kNFCTransport] = true;
+ }
+ return err;
+#else
+ return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+#endif // CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+}
+
+CHIP_ERROR SetUpCodePairer::StopDiscoveryOverNFC()
+{
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ mWaitingForDiscovery[kNFCTransport] = false;
+
+ Nfc::NFCReaderTransport * readerTransport = DeviceLayer::Internal::NFCCommissioningMgr().GetNFCReaderTransport();
+ if (!readerTransport)
+ {
+ ChipLogError(Controller,
+ "Failed to stop commissionable node discovery over NFC since there is no valid NFC reader transport");
+ return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+ }
+
+ ChipLogProgress(Controller, "Stopping commissionable node discovery over NFC by removing delegate");
+ readerTransport->SetDelegate(nullptr);
+#endif
+ return CHIP_NO_ERROR;
+}
+
bool SetUpCodePairer::ConnectToDiscoveredDevice()
{
if (mWaitingForPASE)
@@ -478,6 +553,29 @@
}
#endif
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+void SetUpCodePairer::OnTagDiscovered(const chip::Nfc::NFCTag::Identifier & identifier)
+{
+ ChipLogProgress(Controller, "Discovered device to be commissioned over NFC, Identifier: %u", identifier.discriminator);
+
+ mWaitingForDiscovery[kNFCTransport] = false;
+ auto param = SetUpCodePairerParameters();
+ param.SetPeerAddress(Transport::PeerAddress(Transport::PeerAddress::NFC(identifier.discriminator)));
+ // TODO: This needs to support concatenated QR codes and set the relevant
+ // long discriminator on param.
+ //
+ // See https://github.com/project-chip/connectedhomeip/issues/39134
+ mDiscoveredParameters.emplace_back(param);
+ ConnectToDiscoveredDevice();
+}
+
+void SetUpCodePairer::OnTagDiscoveryFailed(CHIP_ERROR error)
+{
+ ChipLogError(Controller, "Commissionable node discovery over NFC failed: %" CHIP_ERROR_FORMAT, error.Format());
+ mWaitingForDiscovery[kNFCTransport] = false;
+}
+#endif
+
bool SetUpCodePairer::IdIsPresent(uint16_t vendorOrProductID)
{
return vendorOrProductID != kNotAvailable;
@@ -614,6 +712,7 @@
LogErrorOnFailure(StopDiscoveryOverBLE());
LogErrorOnFailure(StopDiscoveryOverDNSSD());
LogErrorOnFailure(StopDiscoveryOverWiFiPAF());
+ LogErrorOnFailure(StopDiscoveryOverNFC());
// Just in case any of those failed to reset the waiting state properly.
for (auto & waiting : mWaitingForDiscovery)
diff --git a/src/controller/SetUpCodePairer.h b/src/controller/SetUpCodePairer.h
index 1be67a7..fb109b3 100644
--- a/src/controller/SetUpCodePairer.h
+++ b/src/controller/SetUpCodePairer.h
@@ -39,6 +39,10 @@
#include <ble/Ble.h>
#endif // CONFIG_NETWORK_BLE
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+#include <nfc/NFC.h>
+#endif // CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+
#include <controller/DeviceDiscoveryDelegate.h>
#include <deque>
@@ -89,6 +93,10 @@
};
class DLL_EXPORT SetUpCodePairer : public DevicePairingDelegate
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ ,
+ public Nfc::NFCReaderTransportDelegate
+#endif
{
public:
SetUpCodePairer(DeviceCommissioner * commissioner) : mCommissioner(commissioner) {}
@@ -119,6 +127,12 @@
void OnPairingDeleted(CHIP_ERROR error) override;
void OnCommissioningComplete(NodeId deviceId, CHIP_ERROR error) override;
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ // Nfc::NFCReaderTransportDelegate implementation
+ void OnTagDiscovered(const chip::Nfc::NFCTag::Identifier & identifer) override;
+ void OnTagDiscoveryFailed(CHIP_ERROR error) override;
+#endif
+
CHIP_ERROR Connect();
CHIP_ERROR StartDiscoveryOverBLE();
CHIP_ERROR StopDiscoveryOverBLE();
@@ -126,6 +140,8 @@
CHIP_ERROR StopDiscoveryOverDNSSD();
CHIP_ERROR StartDiscoveryOverWiFiPAF();
CHIP_ERROR StopDiscoveryOverWiFiPAF();
+ CHIP_ERROR StartDiscoveryOverNFC();
+ CHIP_ERROR StopDiscoveryOverNFC();
// Returns whether we have kicked off a new connection attempt.
bool ConnectToDiscoveredDevice();
@@ -169,6 +185,9 @@
kBLETransport = 0,
kIPTransport,
kWiFiPAFTransport,
+#if CHIP_DEVICE_CONFIG_ENABLE_NFC_BASED_COMMISSIONING
+ kNFCTransport,
+#endif
kTransportTypeCount,
};
