Skip to content

Commit

Permalink
[icd] Add steps for checking ICD during commissioning (#29427)
Browse files Browse the repository at this point in the history 
* [icd] Add steps for checking ICD during commissioning

* Add TODO for attribute used & rename stage to IcdIdentification

* Update

* Fix -- missing endpoint

* Update

* Update

* Update

* Fix

* Fix

* ReadCommissioningInfo2 may complete with enpty report
  • Loading branch information
@erjiaqing @pull
erjiaqing authored and pull[bot] committed Mar 18, 2024
1 parent d2a04c6 commit 1090306
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 37 deletions.
42 changes: 32 additions & 10 deletions src/controller/AutoCommissioner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,12 +273,8 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
// Per the spec, we restart from after adding the NOC.
return GetNextCommissioningStage(CommissioningStage::kSendNOC, lastErr);
}
if (mParams.GetCheckForMatchingFabric())
{
return CommissioningStage::kCheckForMatchingFabric;
}
return CommissioningStage::kArmFailsafe;
case CommissioningStage::kCheckForMatchingFabric:
return CommissioningStage::kReadCommissioningInfo2;
case CommissioningStage::kReadCommissioningInfo2:
return CommissioningStage::kArmFailsafe;
case CommissioningStage::kArmFailsafe:
return CommissioningStage::kConfigRegulatory;
Expand Down Expand Up @@ -666,11 +662,37 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
// Don't send DST unless the device says it needs it
mNeedsDST = false;
break;
case CommissioningStage::kCheckForMatchingFabric: {
chip::NodeId nodeId = report.Get<MatchingFabricInfo>().nodeId;
if (nodeId != kUndefinedNodeId)
case CommissioningStage::kReadCommissioningInfo2: {
bool shouldReadCommissioningInfo2 =
mParams.GetCheckForMatchingFabric() || (mParams.GetICDRegistrationStrategy() != ICDRegistrationStrategy::kIgnore);
if (shouldReadCommissioningInfo2)
{
mParams.SetRemoteNodeId(nodeId);
if (!report.Is<ReadCommissioningInfo2>())
{
ChipLogError(
Controller,
"[BUG] Should read commissioning info (part 2), but report is not ReadCommissioningInfo2. THIS IS A BUG.");
}

ReadCommissioningInfo2 commissioningInfo = report.Get<ReadCommissioningInfo2>();

if (mParams.GetCheckForMatchingFabric())
{
chip::NodeId nodeId = commissioningInfo.nodeId;
if (nodeId != kUndefinedNodeId)
{
mParams.SetRemoteNodeId(nodeId);
}
}

if (mParams.GetICDRegistrationStrategy() != ICDRegistrationStrategy::kIgnore)
{
if (commissioningInfo.isIcd)
{
mNeedIcdRegistraion = true;
ChipLogDetail(Controller, "AutoCommissioner: Device is ICD");
}
}
}
break;
}
Expand Down
2 changes: 2 additions & 0 deletions src/controller/AutoCommissioner.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ class AutoCommissioner : public CommissioningDelegate
ReadCommissioningInfo mDeviceCommissioningInfo;
bool mNeedsDST = false;

bool mNeedIcdRegistraion = false;

// TODO: Why were the nonces statically allocated, but the certs dynamically allocated?
uint8_t * mDAC = nullptr;
uint16_t mDACLen = 0;
Expand Down
101 changes: 86 additions & 15 deletions src/controller/CHIPDeviceController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1877,8 +1877,8 @@ void DeviceCommissioner::OnDone(app::ReadClient *)
case CommissioningStage::kReadCommissioningInfo:
ParseCommissioningInfo();
break;
case CommissioningStage::kCheckForMatchingFabric:
ParseFabrics();
case CommissioningStage::kReadCommissioningInfo2:
ParseCommissioningInfo2();
break;
default:
// We're not trying to read anything here, just exit
Expand Down Expand Up @@ -2103,11 +2103,28 @@ void DeviceCommissioner::ParseTimeSyncInfo(ReadCommissioningInfo & info)
}
}

void DeviceCommissioner::ParseFabrics()
void DeviceCommissioner::ParseCommissioningInfo2()
{
ReadCommissioningInfo2 info;
CHIP_ERROR return_err = CHIP_NO_ERROR;

return_err = ParseFabrics(info);

if (return_err == CHIP_NO_ERROR)
{
return_err = ParseICDInfo(info);
}

CommissioningDelegate::CommissioningReport report;
report.Set<ReadCommissioningInfo2>(info);
CommissioningStageComplete(return_err, report);
}

CHIP_ERROR DeviceCommissioner::ParseFabrics(ReadCommissioningInfo2 & info)
{
CHIP_ERROR err;
CHIP_ERROR return_err = CHIP_NO_ERROR;
MatchingFabricInfo info;

// We might not have requested a Fabrics attribute at all, so not having a
// value for it is not an error.
err = mAttributeCache->ForEachAttribute(OperationalCredentials::Id, [this, &info](const app::ConcreteAttributePath & path) {
Expand Down Expand Up @@ -2170,9 +2187,43 @@ void DeviceCommissioner::ParseFabrics()
mPairingDelegate->OnFabricCheck(info.nodeId);
}

CommissioningDelegate::CommissioningReport report;
report.Set<MatchingFabricInfo>(info);
CommissioningStageComplete(return_err, report);
return return_err;
}

CHIP_ERROR DeviceCommissioner::ParseICDInfo(ReadCommissioningInfo2 & info)
{
CHIP_ERROR err;
IcdManagement::Attributes::FeatureMap::TypeInfo::DecodableType featureMap;

err = mAttributeCache->Get<IcdManagement::Attributes::FeatureMap::TypeInfo>(kRootEndpointId, featureMap);
if (err == CHIP_NO_ERROR)
{
info.isIcd = true;
info.checkInProtocolSupport = !!(featureMap & to_underlying(IcdManagement::Feature::kCheckInProtocolSupport));
}
else if (err == CHIP_ERROR_KEY_NOT_FOUND)
{
info.isIcd = false;
}
else if (err == CHIP_ERROR_IM_STATUS_CODE_RECEIVED)
{
app::StatusIB statusIB;
err = mAttributeCache->GetStatus(
app::ConcreteAttributePath(kRootEndpointId, IcdManagement::Id, IcdManagement::Attributes::FeatureMap::Id), statusIB);
if (err == CHIP_NO_ERROR)
{
if (statusIB.mStatus == Protocols::InteractionModel::Status::UnsupportedCluster)
{
info.isIcd = false;
}
else
{
err = statusIB.ToChipError();
}
}
}

return err;
}

void DeviceCommissioner::OnArmFailSafe(void * context,
Expand Down Expand Up @@ -2414,19 +2465,39 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
SendCommissioningReadRequest(proxy, timeout, readPaths, 9);
}
break;
case CommissioningStage::kCheckForMatchingFabric: {
case CommissioningStage::kReadCommissioningInfo2: {
ChipLogProgress(Controller, "Sending request for commissioning information -- Part 2");

size_t numberOfAttributes = 0;
// This is done in a separate step since we've already used up all the available read paths in the previous read step
app::AttributePathParams readPaths[9];

// Read the current fabrics
if (!params.GetCheckForMatchingFabric())
if (params.GetCheckForMatchingFabric())
{
readPaths[numberOfAttributes++] =
app::AttributePathParams(OperationalCredentials::Id, OperationalCredentials::Attributes::Fabrics::Id);
}

if (params.GetICDRegistrationStrategy() != ICDRegistrationStrategy::kIgnore)
{
readPaths[numberOfAttributes++] =
app::AttributePathParams(endpoint, IcdManagement::Id, IcdManagement::Attributes::FeatureMap::Id);
}

// Current implementation makes sense when we only have a few attributes to read with conditions. Should revisit this if we
// are adding more attributes here.

if (numberOfAttributes == 0)
{
// We don't actually want to do this step, so just bypass it
ChipLogProgress(Controller, "kCheckForMatchingFabric step called without parameter set, skipping");
ChipLogProgress(Controller, "kReadCommissioningInfo2 step called without parameter set, skipping");
CommissioningStageComplete(CHIP_NO_ERROR);
}

// This is done in a separate step since we've already used up all the available read paths in the previous read step
app::AttributePathParams readPaths[1];
readPaths[0] = app::AttributePathParams(OperationalCredentials::Id, OperationalCredentials::Attributes::Fabrics::Id);
SendCommissioningReadRequest(proxy, timeout, readPaths, 1);
else
{
SendCommissioningReadRequest(proxy, timeout, readPaths, numberOfAttributes);
}
}
break;
case CommissioningStage::kConfigureUTCTime: {
Expand Down
5 changes: 4 additions & 1 deletion src/controller/CHIPDeviceController.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,10 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
#if CHIP_CONFIG_ENABLE_READ_CLIENT
// Parsers for the two different read clients
void ParseCommissioningInfo();
void ParseFabrics();
void ParseCommissioningInfo2();
// Called by ParseCommissioningInfo2
CHIP_ERROR ParseFabrics(ReadCommissioningInfo2 & info);
CHIP_ERROR ParseICDInfo(ReadCommissioningInfo2 & info);
// Called by ParseCommissioningInfo
void ParseTimeSyncInfo(ReadCommissioningInfo & info);
#endif // CHIP_CONFIG_ENABLE_READ_CLIENT
Expand Down
4 changes: 2 additions & 2 deletions src/controller/CommissioningDelegate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ const char * StageToString(CommissioningStage stage)
return "ReadCommissioningInfo";
break;

case kCheckForMatchingFabric:
return "CheckForMatchingFabric";
case kReadCommissioningInfo2:
return "ReadCommissioningInfo2";
break;

case kArmFailsafe:
Expand Down
32 changes: 25 additions & 7 deletions src/controller/CommissioningDelegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ enum CommissioningStage : uint8_t
kError,
kSecurePairing, ///< Establish a PASE session with the device
kReadCommissioningInfo, ///< Query General Commissioning Attributes, Network Features and Time Synchronization Cluster
kCheckForMatchingFabric, ///< Read the current fabrics on the commissionee
kReadCommissioningInfo2, ///< Query ICD state, check for matching fabric
kArmFailsafe, ///< Send ArmFailSafe (0x30:0) command to the device
kConfigRegulatory, ///< Send SetRegulatoryConfig (0x30:2) command to the device
kConfigureUTCTime, ///< SetUTCTime if the DUT has a time cluster
Expand Down Expand Up @@ -71,6 +71,13 @@ enum CommissioningStage : uint8_t
kNeedsNetworkCreds,
};

enum ICDRegistrationStrategy : uint8_t
{
kIgnore, ///< Do not check whether the device is an ICD during commissioning
kBeforeComplete, ///< Do commissioner self-registration or external controller registration,
///< Controller should provide a ICDKey manager for generating symmetric key
};

const char * StageToString(CommissioningStage stage);

struct WiFiCredentials
Expand Down Expand Up @@ -493,6 +500,13 @@ class CommissioningParameters
return *this;
}

ICDRegistrationStrategy GetICDRegistrationStrategy() const { return mICDRegistrationStrategy; }
CommissioningParameters & SetICDRegistrationStrategy(ICDRegistrationStrategy icdRegistrationStrategy)
{
mICDRegistrationStrategy = icdRegistrationStrategy;
return *this;
}

// Clear all members that depend on some sort of external buffer. Can be
// used to make sure that we are not holding any dangling pointers.
void ClearExternalBufferDependentValues()
Expand Down Expand Up @@ -552,7 +566,8 @@ class CommissioningParameters
Optional<bool> mAttemptWiFiNetworkScan;
Optional<bool> mAttemptThreadNetworkScan; // This automatically gets set to false when a ThreadOperationalDataset is set
Optional<bool> mSkipCommissioningComplete;
bool mCheckForMatchingFabric = false;
ICDRegistrationStrategy mICDRegistrationStrategy = ICDRegistrationStrategy::kIgnore;
bool mCheckForMatchingFabric = false;
};

struct RequestedCertificate
Expand Down Expand Up @@ -634,9 +649,12 @@ struct ReadCommissioningInfo
uint8_t maxTimeZoneSize = 1;
uint8_t maxDSTSize = 1;
};
struct MatchingFabricInfo

struct ReadCommissioningInfo2
{
NodeId nodeId = kUndefinedNodeId;
NodeId nodeId = kUndefinedNodeId;
bool isIcd = false;
bool checkInProtocolSupport = false;
};

struct TimeZoneResponseInfo
Expand Down Expand Up @@ -670,7 +688,7 @@ class CommissioningDelegate
virtual ~CommissioningDelegate(){};
/* CommissioningReport is returned after each commissioning step is completed. The reports for each step are:
* kReadCommissioningInfo - ReadCommissioningInfo
* kCheckForMatchingFabric = MatchingFabricInfo
* kReadCommissioningInfo2: ReadCommissioningInfo2
* kArmFailsafe: CommissioningErrorInfo if there is an error
* kConfigRegulatory: CommissioningErrorInfo if there is an error
* kConfigureUTCTime: None
Expand All @@ -695,8 +713,8 @@ class CommissioningDelegate
* kCleanup: none
*/
struct CommissioningReport : Variant<RequestedCertificate, AttestationResponse, CSRResponse, NocChain, OperationalNodeFoundData,
ReadCommissioningInfo, AttestationErrorInfo, CommissioningErrorInfo,
NetworkCommissioningStatusInfo, MatchingFabricInfo, TimeZoneResponseInfo>
ReadCommissioningInfo, ReadCommissioningInfo2, AttestationErrorInfo,
CommissioningErrorInfo, NetworkCommissioningStatusInfo, TimeZoneResponseInfo>
{
CommissioningReport() : stageCompleted(CommissioningStage::kError) {}
CommissioningStage stageCompleted;
Expand Down
2 changes: 0 additions & 2 deletions src/controller/python/OpCredsBinding.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,6 @@ class TestCommissioner : public chip::Controller::AutoCommissioner
{
switch (stage)
{
case chip::Controller::CommissioningStage::kCheckForMatchingFabric:
return mParams.GetCheckForMatchingFabric();
case chip::Controller::CommissioningStage::kWiFiNetworkEnable:
case chip::Controller::CommissioningStage::kFailsafeBeforeWiFiEnable:
case chip::Controller::CommissioningStage::kWiFiNetworkSetup:
Expand Down

0 comments on commit 1090306

Please sign in to comment.