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.

Sign up for GitHub

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

[controller] merge ReadCommissioningInfo2 into ReadCommissioningInfo #30978

Merged
merged 7 commits into from
Dec 14, 2023
Merged

[controller] merge ReadCommissioningInfo2 into ReadCommissioningInfo #30978

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7bd3176
[controller] merge ReadCommissioningInfo2 into ReadCommissioningInfo
erjiaqing Dec 13, 2023
232e283
Update src/controller/CHIPDeviceController.cpp
yunhanw-google Dec 13, 2023
35c84b6
Update src/controller/CHIPDeviceController.cpp
yunhanw-google Dec 13, 2023
d8a592d
Update src/controller/CHIPDeviceController.cpp
yunhanw-google Dec 13, 2023
f9a40db
Update src/controller/CHIPDeviceController.cpp
yunhanw-google Dec 13, 2023
d65e55b
address comment
yunhanw-google Dec 13, 2023
9af75bf
Fix
erjiaqing Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions src/controller/AutoCommissioner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,15 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
switch (report.stageCompleted)
{
case CommissioningStage::kReadCommissioningInfo:
break;
case CommissioningStage::kReadCommissioningInfo2: {
if (!report.Is<ReadCommissioningInfo>())
{
ChipLogError(Controller,
"[BUG] Should read commissioning info, but report is not ReadCommissioningInfo. THIS IS A BUG.");
}
ReadCommissioningInfo commissioningInfo = report.Get<ReadCommissioningInfo>();

mDeviceCommissioningInfo = report.Get<ReadCommissioningInfo>();
if (!mParams.GetFailsafeTimerSeconds().HasValue() && mDeviceCommissioningInfo.general.recommendedFailsafe > 0)
{
Expand All @@ -736,22 +745,12 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
.SetLocationCapability(mDeviceCommissioningInfo.general.locationCapability);
// Don't send DST unless the device says it needs it
mNeedsDST = false;
break;
case CommissioningStage::kReadCommissioningInfo2: {

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>();
mParams.SetSupportsConcurrentConnection(commissioningInfo.supportsConcurrentConnection);

if (mParams.GetCheckForMatchingFabric())
{
chip::NodeId nodeId = commissioningInfo.nodeId;
chip::NodeId nodeId = commissioningInfo.remoteNodeId;
if (nodeId != kUndefinedNodeId)
{
mParams.SetRemoteNodeId(nodeId);
Expand All @@ -760,7 +759,7 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio

if (mParams.GetICDRegistrationStrategy() != ICDRegistrationStrategy::kIgnore)
{
if (commissioningInfo.isIcd && commissioningInfo.checkInProtocolSupport)
if (commissioningInfo.isLIT && commissioningInfo.checkInProtocolSupport)
{
mNeedIcdRegistration = true;
ChipLogDetail(Controller, "AutoCommissioner: ICD supports the check-in protocol.");
Expand Down
89 changes: 55 additions & 34 deletions src/controller/CHIPDeviceController.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1922,25 +1922,50 @@ void DeviceCommissioner::OnDeviceConnectionRetryFn(void * context, const ScopedN
// ClusterStateCache::Callback impl
void DeviceCommissioner::OnDone(app::ReadClient *)
{
mReadClient = nullptr;
switch (mCommissioningStage)
{
case CommissioningStage::kReadCommissioningInfo:
ParseCommissioningInfo();
// Silently complete the stage, data will be saved in attribute cache and
// will be parsed after all ReadCommissioningInfo stages are completed.
CommissioningStageComplete(CHIP_NO_ERROR);
break;
case CommissioningStage::kReadCommissioningInfo2:
ParseCommissioningInfo2();
// Note: Only parse commissioning info in the last ReadCommissioningInfo stage.
ParseCommissioningInfo();
break;
default:
// We're not trying to read anything here, just exit
break;
}
}

void DeviceCommissioner::ParseCommissioningInfo()
{
CHIP_ERROR err = CHIP_NO_ERROR;
ReadCommissioningInfo info;

err = ParseCommissioningInfo1(info);
if (err == CHIP_NO_ERROR)
{
err = ParseCommissioningInfo2(info);
}

mAttributeCache = nullptr;

if (mPairingDelegate != nullptr)
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
{
mPairingDelegate->OnReadCommissioningInfo(info);
}

CommissioningDelegate::CommissioningReport report;
report.Set<ReadCommissioningInfo>(info);
CommissioningStageComplete(err, report);
}

CHIP_ERROR DeviceCommissioner::ParseCommissioningInfo1(ReadCommissioningInfo & info)
{
CHIP_ERROR err;
CHIP_ERROR return_err = CHIP_NO_ERROR;
ReadCommissioningInfo info;

// Try to parse as much as we can here before returning, even if attributes
// are missing or cannot be decoded.
Expand Down Expand Up @@ -2080,17 +2105,8 @@ void DeviceCommissioner::ParseCommissioningInfo()
{
ChipLogError(Controller, "Error parsing commissioning information");
}
mAttributeCache = nullptr;
mReadClient = nullptr;

if (mPairingDelegate != nullptr)
{
mPairingDelegate->OnReadCommissioningInfo(info);
}

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

void DeviceCommissioner::ParseTimeSyncInfo(ReadCommissioningInfo & info)
Expand Down Expand Up @@ -2153,34 +2169,31 @@ void DeviceCommissioner::ParseTimeSyncInfo(ReadCommissioningInfo & info)
}
}

void DeviceCommissioner::ParseCommissioningInfo2()
CHIP_ERROR DeviceCommissioner::ParseCommissioningInfo2(ReadCommissioningInfo & info)
{
ReadCommissioningInfo2 info;
CHIP_ERROR return_err = CHIP_NO_ERROR;
CHIP_ERROR err = CHIP_NO_ERROR;

using namespace chip::app::Clusters::GeneralCommissioning::Attributes;
CHIP_ERROR err =
mAttributeCache->Get<SupportsConcurrentConnection::TypeInfo>(kRootEndpointId, info.supportsConcurrentConnection);
if (err != CHIP_NO_ERROR)

if (mAttributeCache->Get<SupportsConcurrentConnection::TypeInfo>(kRootEndpointId, info.supportsConcurrentConnection) !=
CHIP_NO_ERROR)
{
// May not be present so don't return the error code, non fatal, default concurrent
ChipLogError(Controller, "Failed to read SupportsConcurrentConnection: %" CHIP_ERROR_FORMAT, err.Format());
info.supportsConcurrentConnection = true;
}

return_err = ParseFabrics(info);
err = ParseFabrics(info);

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

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

CHIP_ERROR DeviceCommissioner::ParseFabrics(ReadCommissioningInfo2 & info)
CHIP_ERROR DeviceCommissioner::ParseFabrics(ReadCommissioningInfo & info)
{
CHIP_ERROR err;
CHIP_ERROR return_err = CHIP_NO_ERROR;
Expand Down Expand Up @@ -2230,7 +2243,7 @@ CHIP_ERROR DeviceCommissioner::ParseFabrics(ReadCommissioningInfo2 & info)
else if (commissionerRootPublicKey.Matches(deviceRootPublicKey))
{
ChipLogProgress(Controller, "DeviceCommissioner::OnDone - fabric root keys match");
info.nodeId = fabricDescriptor.nodeID;
info.remoteNodeId = fabricDescriptor.nodeID;
}
}
}
Expand All @@ -2244,28 +2257,28 @@ CHIP_ERROR DeviceCommissioner::ParseFabrics(ReadCommissioningInfo2 & info)

if (mPairingDelegate != nullptr)
{
mPairingDelegate->OnFabricCheck(info.nodeId);
mPairingDelegate->OnFabricCheck(info.remoteNodeId);
}

return return_err;
}

CHIP_ERROR DeviceCommissioner::ParseICDInfo(ReadCommissioningInfo2 & info)
CHIP_ERROR DeviceCommissioner::ParseICDInfo(ReadCommissioningInfo & 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.isLIT = true;
info.checkInProtocolSupport = !!(featureMap & to_underlying(IcdManagement::Feature::kCheckInProtocolSupport));
}
else if (err == CHIP_ERROR_KEY_NOT_FOUND)
{
// This key is optional so not an error
err = CHIP_NO_ERROR;
info.isIcd = false;
info.isLIT = false;
err = CHIP_NO_ERROR;
}
else if (err == CHIP_ERROR_IM_STATUS_CODE_RECEIVED)
Expand All @@ -2277,7 +2290,7 @@ CHIP_ERROR DeviceCommissioner::ParseICDInfo(ReadCommissioningInfo2 & info)
{
if (statusIB.mStatus == Protocols::InteractionModel::Status::UnsupportedCluster)
{
info.isIcd = false;
info.isLIT = false;
}
else
{
Expand Down Expand Up @@ -2458,7 +2471,8 @@ void DeviceCommissioner::SendCommissioningReadRequest(DeviceProxy * proxy, Optio
readParams.mpAttributePathParamsList = readPaths;
readParams.mAttributePathParamsListSize = readPathsSize;

auto attributeCache = Platform::MakeUnique<app::ClusterStateCache>(*this);
// Take ownership of the attribute cache, so it can be released when SendRequest fails.
auto attributeCache = std::move(mAttributeCache);
auto readClient = chip::Platform::MakeUnique<app::ReadClient>(
engine, proxy->GetExchangeManager(), attributeCache->GetBufferedCallback(), app::ReadClient::InteractionType::Read);
CHIP_ERROR err = readClient->SendRequest(readParams);
Expand Down Expand Up @@ -2509,6 +2523,13 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
break;
case CommissioningStage::kReadCommissioningInfo: {
ChipLogProgress(Controller, "Sending read request for commissioning information");
// Allocate a new ClusterStateCache when starting reading the first batch of attributes.
// The cache will be released in:
// - SendCommissioningReadRequest when failing to send a read request.
// - ParseCommissioningInfo when the last ReadCommissioningInfo stage is completed.
// Currently, we have two ReadCommissioningInfo* stages.
mAttributeCache = Platform::MakeUnique<app::ClusterStateCache>(*this);

// NOTE: this array cannot have more than 9 entries, since the spec mandates that server only needs to support 9
// See R1.1, 2.11.2 Interaction Model Limits
app::AttributePathParams readPaths[9];
Expand Down
10 changes: 6 additions & 4 deletions src/controller/CHIPDeviceController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -952,12 +952,14 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
void SendCommissioningReadRequest(DeviceProxy * proxy, Optional<System::Clock::Timeout> timeout,
app::AttributePathParams * readPaths, size_t readPathsSize);
#if CHIP_CONFIG_ENABLE_READ_CLIENT
// Parsers for the two different read clients
void ParseCommissioningInfo();
void ParseCommissioningInfo2();
// Parsing attributes read in kReadCommissioningInfo stage.
CHIP_ERROR ParseCommissioningInfo1(ReadCommissioningInfo & info);
// Parsing attributes read in kReadCommissioningInfo2 stage.
CHIP_ERROR ParseCommissioningInfo2(ReadCommissioningInfo & info);
// Called by ParseCommissioningInfo2
CHIP_ERROR ParseFabrics(ReadCommissioningInfo2 & info);
CHIP_ERROR ParseICDInfo(ReadCommissioningInfo2 & info);
CHIP_ERROR ParseFabrics(ReadCommissioningInfo & info);
CHIP_ERROR ParseICDInfo(ReadCommissioningInfo & info);
// Called by ParseCommissioningInfo
void ParseTimeSyncInfo(ReadCommissioningInfo & info);
#endif // CHIP_CONFIG_ENABLE_READ_CLIENT
Expand Down
30 changes: 13 additions & 17 deletions src/controller/CommissioningDelegate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -684,18 +684,14 @@ struct ReadCommissioningInfo
NetworkClusters network;
BasicClusterInfo basic;
GeneralCommissioningInfo general;
bool requiresUTC = false;
bool requiresTimeZone = false;
bool requiresDefaultNTP = false;
bool requiresTrustedTimeSource = false;
uint8_t maxTimeZoneSize = 1;
uint8_t maxDSTSize = 1;
};

struct ReadCommissioningInfo2
{
NodeId nodeId = kUndefinedNodeId;
bool isIcd = false;
bool requiresUTC = false;
bool requiresTimeZone = false;
bool requiresDefaultNTP = false;
bool requiresTrustedTimeSource = false;
uint8_t maxTimeZoneSize = 1;
uint8_t maxDSTSize = 1;
NodeId remoteNodeId = kUndefinedNodeId;
bool isLIT = false;
bool checkInProtocolSupport = false;
bool supportsConcurrentConnection = true;
};
Expand Down Expand Up @@ -730,8 +726,8 @@ class CommissioningDelegate
public:
virtual ~CommissioningDelegate(){};
/* CommissioningReport is returned after each commissioning step is completed. The reports for each step are:
* kReadCommissioningInfo: ReadCommissioningInfo
* kReadCommissioningInfo2: ReadCommissioningInfo2
* kReadCommissioningInfo: Reported together with ReadCommissioningInfo2
* kReadCommissioningInfo2: ReadCommissioningInfo
* kArmFailsafe: CommissioningErrorInfo if there is an error
* kConfigRegulatory: CommissioningErrorInfo if there is an error
* kConfigureUTCTime: None
Expand All @@ -755,9 +751,9 @@ class CommissioningDelegate
* kSendComplete: CommissioningErrorInfo if there is an error
* kCleanup: none
*/
struct CommissioningReport : Variant<RequestedCertificate, AttestationResponse, CSRResponse, NocChain, OperationalNodeFoundData,
ReadCommissioningInfo, ReadCommissioningInfo2, AttestationErrorInfo,
CommissioningErrorInfo, NetworkCommissioningStatusInfo, TimeZoneResponseInfo>
struct CommissioningReport
: Variant<RequestedCertificate, AttestationResponse, CSRResponse, NocChain, OperationalNodeFoundData, ReadCommissioningInfo,
AttestationErrorInfo, CommissioningErrorInfo, NetworkCommissioningStatusInfo, TimeZoneResponseInfo>
{
CommissioningReport() : stageCompleted(CommissioningStage::kError) {}
CommissioningStage stageCompleted;
Expand Down
Loading