diff --git a/src/controller/AutoCommissioner.cpp b/src/controller/AutoCommissioner.cpp index 02df9def4ba537..1d633716fa9e11 100644 --- a/src/controller/AutoCommissioner.cpp +++ b/src/controller/AutoCommissioner.cpp @@ -631,11 +631,25 @@ 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().nodeId; - if (nodeId != kUndefinedNodeId) + case CommissioningStage::kReadCommissioningInfo2: { + ReadCommissioningInfo2 commissioningInfo = report.Get(); + + if (mParams.GetCheckForMatchingFabric()) + { + chip::NodeId nodeId = commissioningInfo.nodeId; + if (nodeId != kUndefinedNodeId) + { + mParams.SetRemoteNodeId(nodeId); + } + } + + if (mParams.GetICDRegistrationStrategy() != ICDRegistrationStrategy::kIgnore) { - mParams.SetRemoteNodeId(nodeId); + if (commissioningInfo.isIcd) + { + mNeedIcdRegistraion = true; + ChipLogDetail(Controller, "AutoCommissioner: Device is ICD"); + } } break; } @@ -701,15 +715,6 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio case CommissioningStage::kFindOperational: mOperationalDeviceProxy = report.Get().operationalProxy; break; - case CommissioningStage::kICDIdentification: { - IcdInfo icdInfo = report.Get(); - if (icdInfo.isIcd) - { - mNeedIcdRegistraion = true; - ChipLogDetail(Controller, "AutoCommissioner: Device is ICD"); - } - break; - } case CommissioningStage::kCleanup: ReleasePAI(); ReleaseDAC(); diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 379139798b7e00..7296abe29a1a65 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -1878,14 +1878,7 @@ void DeviceCommissioner::OnDone(app::ReadClient *) ParseCommissioningInfo(); break; case CommissioningStage::kReadCommissioningInfo2: - if (param.GetCheckForMatchingFabric()) - { - ParseFabrics(); - } - if (param.GetICDRegistrationStrategy() != ICDRegistrationStrategy::kIgnore) - { - ParseICDInfo(); - } + ParseCommissioningInfo2(); break; default: // We're not trying to read anything here, just exit @@ -2110,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(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) { @@ -2177,15 +2187,12 @@ void DeviceCommissioner::ParseFabrics() mPairingDelegate->OnFabricCheck(info.nodeId); } - CommissioningDelegate::CommissioningReport report; - report.Set(info); - CommissioningStageComplete(return_err, report); + return return_err; } -void DeviceCommissioner::ParseICDInfo() +CHIP_ERROR DeviceCommissioner::ParseICDInfo(ReadCommissioningInfo2 & info) { CHIP_ERROR err; - IcdInfo info; IcdManagement::Attributes::FeatureMap::TypeInfo::DecodableType featureMap; err = mAttributeCache->Get(kRootEndpointId, featureMap); @@ -2194,6 +2201,10 @@ void DeviceCommissioner::ParseICDInfo() 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; @@ -2212,12 +2223,7 @@ void DeviceCommissioner::ParseICDInfo() } } - CommissioningDelegate::CommissioningReport report; - if (err == CHIP_NO_ERROR) - { - report.Set(info); - } - CommissioningStageComplete(err, report); + return err; } void DeviceCommissioner::OnArmFailSafe(void * context, @@ -2482,7 +2488,7 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio 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); } @@ -2842,14 +2848,6 @@ 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. - readPaths[0] = app::AttributePathParams(endpoint, app::Clusters::IcdManagement::Id, - app::Clusters::IcdManagement::Attributes::FeatureMap::Id); - SendCommissioningReadRequest(proxy, timeout, readPaths, 1); - } - break; case CommissioningStage::kSendComplete: { GeneralCommissioning::Commands::CommissioningComplete::Type request; SendCommand(proxy, request, OnCommissioningCompleteResponse, OnBasicFailure, endpoint, timeout); diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 75fde0c92b38ed..9d6a9d6f5a897c 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -935,10 +935,12 @@ 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); - void ParseICDInfo(); #endif // CHIP_CONFIG_ENABLE_READ_CLIENT static CHIP_ERROR diff --git a/src/controller/CommissioningDelegate.h b/src/controller/CommissioningDelegate.h index 2c34ae71f64425..e0e7924db495e5 100644 --- a/src/controller/CommissioningDelegate.h +++ b/src/controller/CommissioningDelegate.h @@ -648,17 +648,13 @@ struct ReadCommissioningInfo uint8_t maxDSTSize = 1; }; -struct IcdInfo +struct ReadCommissioningInfo2 { + NodeId nodeId = kUndefinedNodeId; bool isIcd = false; bool checkInProtocolSupport = false; }; -struct MatchingFabricInfo -{ - NodeId nodeId = kUndefinedNodeId; -}; - struct TimeZoneResponseInfo { bool requiresDSTOffsets; @@ -690,7 +686,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 @@ -715,8 +711,8 @@ class CommissioningDelegate * kCleanup: none */ struct CommissioningReport : Variant + ReadCommissioningInfo, ReadCommissioningInfo2, AttestationErrorInfo, + CommissioningErrorInfo, NetworkCommissioningStatusInfo, TimeZoneResponseInfo> { CommissioningReport() : stageCompleted(CommissioningStage::kError) {} CommissioningStage stageCompleted; diff --git a/src/controller/python/OpCredsBinding.cpp b/src/controller/python/OpCredsBinding.cpp index f78b0135e1f2c4..03f4ca0606975f 100644 --- a/src/controller/python/OpCredsBinding.cpp +++ b/src/controller/python/OpCredsBinding.cpp @@ -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: