Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[icd] Add steps for checking ICD during commissioning #29427

Merged
16 changes: 16 additions & 0 deletions src/controller/AutoCommissioner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,13 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
}
return CommissioningStage::kFindOperational;
case CommissioningStage::kFindOperational:
if (mParams.GetICDRegistrationStrategy() != ICDRegistrationStrategy::kIgnore)
{
return CommissioningStage::kICDIdentification;
}
return CommissioningStage::kSendComplete;
erjiaqing marked this conversation as resolved.
Show resolved Hide resolved
case CommissioningStage::kICDIdentification:
// TODO(#29385): Register to the ICD.
return CommissioningStage::kSendComplete;
case CommissioningStage::kSendComplete:
return CommissioningStage::kCleanup;
Expand Down Expand Up @@ -705,6 +712,15 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
case CommissioningStage::kFindOperational:
mOperationalDeviceProxy = report.Get<OperationalNodeFoundData>().operationalProxy;
break;
case CommissioningStage::kICDIdentification: {
IcdInfo icdInfo = report.Get<IcdInfo>();
if (icdInfo.isIcd)
{
mNeedIcdRegistraion = true;
ChipLogDetail(Controller, "AutoCommissioner: Device is ICD");
}
break;
}
case CommissioningStage::kCleanup:
ReleasePAI();
ReleaseDAC();
Expand Down
2 changes: 2 additions & 0 deletions src/controller/AutoCommissioner.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ class AutoCommissioner : public CommissioningDelegate
ReadCommissioningInfo mDeviceCommissioningInfo;
bool mNeedsDST = false;

bool mNeedIcdRegistraion = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please name this mNeedICDRegistration.


// TODO: Why were the nonces statically allocated, but the certs dynamically allocated?
uint8_t * mDAC = nullptr;
uint16_t mDACLen = 0;
Expand Down
50 changes: 50 additions & 0 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1880,6 +1880,9 @@ void DeviceCommissioner::OnDone(app::ReadClient *)
case CommissioningStage::kCheckForMatchingFabric:
ParseFabrics();
break;
case CommissioningStage::kICDIdentification:
ParseICDInfo();
break;
default:
// We're not trying to read anything here, just exit
break;
Expand Down Expand Up @@ -2175,6 +2178,45 @@ void DeviceCommissioner::ParseFabrics()
CommissioningStageComplete(return_err, report);
}

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

// TODO(#29382): We probably want to read "ActiveMode" attribute (to be implemented) for ICD.
erjiaqing marked this conversation as resolved.
Show resolved Hide resolved
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));
mkardous-silabs marked this conversation as resolved.
Show resolved Hide resolved
}
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();
}
}
}

CommissioningDelegate::CommissioningReport report;
if (err == CHIP_NO_ERROR)
{
report.Set<IcdInfo>(info);
}
CommissioningStageComplete(err, report);
}

void DeviceCommissioner::OnArmFailSafe(void * context,
const GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType & data)
{
Expand Down Expand Up @@ -2782,6 +2824,14 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
);
}
break;
case CommissioningStage::kICDIdentification: {
app::AttributePathParams readPaths[1];
// TODO(#29382): We probably want to read "ActiveMode" attribute (to be implemented) for ICD.
erjiaqing marked this conversation as resolved.
Show resolved Hide resolved
readPaths[0] = app::AttributePathParams(endpoint, app::Clusters::IcdManagement::Id,
app::Clusters::IcdManagement::Attributes::FeatureMap::Id);
SendCommissioningReadRequest(proxy, timeout, readPaths, 1);
erjiaqing marked this conversation as resolved.
Show resolved Hide resolved
}
break;
case CommissioningStage::kSendComplete: {
GeneralCommissioning::Commands::CommissioningComplete::Type request;
SendCommand(proxy, request, OnCommissioningCompleteResponse, OnBasicFailure, endpoint, timeout);
Expand Down
1 change: 1 addition & 0 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
void ParseFabrics();
// Called by ParseCommissioningInfo
void ParseTimeSyncInfo(ReadCommissioningInfo & info);
void ParseICDInfo();
#endif // CHIP_CONFIG_ENABLE_READ_CLIENT

static CHIP_ERROR
Expand Down
4 changes: 4 additions & 0 deletions src/controller/CommissioningDelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ const char * StageToString(CommissioningStage stage)
return "FindOperational";
break;

case kICDIdentification:
return "ICDIdentification";
break;

case kSendComplete:
return "SendComplete";
break;
Expand Down
34 changes: 30 additions & 4 deletions src/controller/CommissioningDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,12 @@ enum CommissioningStage : uint8_t
kWiFiNetworkEnable, ///< Send ConnectNetwork (0x31:6) command to the device for the WiFi network
kThreadNetworkEnable, ///< Send ConnectNetwork (0x31:6) command to the device for the Thread network
kFindOperational, ///< Perform operational discovery and establish a CASE session with the device
kSendComplete, ///< Send CommissioningComplete (0x30:4) command to the device
kCleanup, ///< Call delegates with status, free memory, clear timers and state
/// Optional steps for ICD
kICDIdentification, ///< Check whether the device is an ICD
/// TODO(#29384): Finish ICD registration implementation in commissioner
/// End of optional steps for ICD
kSendComplete, ///< Send CommissioningComplete (0x30:4) command to the device
kCleanup, ///< Call delegates with status, free memory, clear timers and state
/// Send ScanNetworks (0x31:0) command to the device.
/// ScanNetworks can happen anytime after kArmFailsafe.
/// However, the cirque tests fail if it is earlier in the list
Expand All @@ -71,6 +75,13 @@ enum CommissioningStage : uint8_t
kNeedsNetworkCreds,
};

enum ICDRegistrationStrategy
erjiaqing marked this conversation as resolved.
Show resolved Hide resolved
{
kIgnore, ///< Do not check whether the device is an ICD during commissioning
kBeforeComplete, ///< Do commissioner self-registration or external controller registration,
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
///< Controller should provide a ICDKey manager for generating symmetric key
};

const char * StageToString(CommissioningStage stage);

struct WiFiCredentials
Expand Down Expand Up @@ -493,6 +504,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 @@ -550,7 +568,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::kBeforeComplete;
erjiaqing marked this conversation as resolved.
Show resolved Hide resolved
bool mCheckForMatchingFabric = false;
};

struct RequestedCertificate
Expand Down Expand Up @@ -632,6 +651,13 @@ struct ReadCommissioningInfo
uint8_t maxTimeZoneSize = 1;
uint8_t maxDSTSize = 1;
};

struct IcdInfo
{
bool isIcd = false;
bool checkInProtocolSupport = false;
};

struct MatchingFabricInfo
{
NodeId nodeId = kUndefinedNodeId;
Expand Down Expand Up @@ -694,7 +720,7 @@ class CommissioningDelegate
*/
struct CommissioningReport : Variant<RequestedCertificate, AttestationResponse, CSRResponse, NocChain, OperationalNodeFoundData,
ReadCommissioningInfo, AttestationErrorInfo, CommissioningErrorInfo,
NetworkCommissioningStatusInfo, MatchingFabricInfo, TimeZoneResponseInfo>
NetworkCommissioningStatusInfo, MatchingFabricInfo, TimeZoneResponseInfo, IcdInfo>
{
CommissioningReport() : stageCompleted(CommissioningStage::kError) {}
CommissioningStage stageCompleted;
Expand Down
Loading