Skip to content

Commit

Permalink
[ICD] Invoke stayActive in the end of commissioning flow (#32496)
Browse files Browse the repository at this point in the history
* Invoke stayActive in the end of commissioning flow

* address comment

* Restyled by clang-format

* address comments

* Restyled by whitespace

* Restyled by clang-format

* address comments

* Restyled by clang-format

* Send ICDStayActive over case

* Add another 3 state machine to handle

-- To release all previous stale case sssion
-- To handle case session for ICD stay active
-- To handle case session for commision complete

* address comment

---------

Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
yunhanw-google and restyled-commits authored Mar 26, 2024
1 parent eed2a1a commit 813181a
Show file tree
Hide file tree
Showing 9 changed files with 147 additions and 86 deletions.
10 changes: 10 additions & 0 deletions examples/chip-tool/commands/pairing/PairingCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ CommissioningParameters PairingCommand::GetCommissioningParameters()
// These Optionals must have values now.
// The commissioner will verify these values.
params.SetICDSymmetricKey(mICDSymmetricKey.Value());
if (mICDStayActiveDurationMsec.HasValue())
{
params.SetICDStayActiveDurationMsec(mICDStayActiveDurationMsec.Value());
}
params.SetICDCheckInNodeId(mICDCheckInNodeId.Value());
params.SetICDMonitoredSubject(mICDMonitoredSubject.Value());
}
Expand Down Expand Up @@ -465,6 +469,12 @@ void PairingCommand::OnICDRegistrationComplete(NodeId nodeId, uint32_t icdCounte
ChipLogValueX64(mICDMonitoredSubject.Value()), icdSymmetricKeyHex);
}

void PairingCommand::OnICDStayActiveComplete(NodeId deviceId, uint32_t promisedActiveDuration)
{
ChipLogProgress(chipTool, "ICD Stay Active Complete for device " ChipLogFormatX64 " / promisedActiveDuration: %u",
ChipLogValueX64(deviceId), promisedActiveDuration);
}

void PairingCommand::OnDiscoveredDevice(const chip::Dnssd::DiscoveredNodeData & nodeData)
{
// Ignore nodes with closed commissioning window
Expand Down
5 changes: 4 additions & 1 deletion examples/chip-tool/commands/pairing/PairingCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ class PairingCommand : public CHIPCommand,
AddArgument("icd-monitored-subject", 0, UINT64_MAX, &mICDMonitoredSubject,
"The monitored subject of the ICD, default: The node id used for icd-check-in-nodeid.");
AddArgument("icd-symmetric-key", &mICDSymmetricKey, "The 16 bytes ICD symmetric key, default: randomly generated.");

AddArgument("icd-stay-active-duration", 0, UINT32_MAX, &mICDStayActiveDurationMsec,
"If set, a LIT ICD that is commissioned will be requested to stay active for this many milliseconds");
switch (networkType)
{
case PairingNetworkType::None:
Expand Down Expand Up @@ -197,6 +198,7 @@ class PairingCommand : public CHIPCommand,
void OnReadCommissioningInfo(const chip::Controller::ReadCommissioningInfo & info) override;
void OnCommissioningComplete(NodeId deviceId, CHIP_ERROR error) override;
void OnICDRegistrationComplete(NodeId deviceId, uint32_t icdCounter) override;
void OnICDStayActiveComplete(NodeId deviceId, uint32_t promisedActiveDuration) override;

/////////// DeviceDiscoveryDelegate Interface /////////
void OnDiscoveredDevice(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
Expand Down Expand Up @@ -235,6 +237,7 @@ class PairingCommand : public CHIPCommand,
chip::Optional<NodeId> mICDCheckInNodeId;
chip::Optional<chip::ByteSpan> mICDSymmetricKey;
chip::Optional<uint64_t> mICDMonitoredSubject;
chip::Optional<uint32_t> mICDStayActiveDurationMsec;
chip::app::DataModel::List<chip::app::Clusters::TimeSynchronization::Structs::TimeZoneStruct::Type> mTimeZoneList;
TypedComplexArgument<chip::app::DataModel::List<chip::app::Clusters::TimeSynchronization::Structs::TimeZoneStruct::Type>>
mComplex_TimeZones;
Expand Down
34 changes: 20 additions & 14 deletions src/controller/AutoCommissioner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -416,13 +416,10 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
}
return CommissioningStage::kICDGetRegistrationInfo;
}
return GetNextCommissioningStageInternal(CommissioningStage::kICDSendStayActive, lastErr);
return GetNextCommissioningStageInternal(CommissioningStage::kICDRegistration, lastErr);
case CommissioningStage::kICDGetRegistrationInfo:
return CommissioningStage::kICDRegistration;
case CommissioningStage::kICDRegistration:
// TODO(#24259): StayActiveRequest is not supported by server. We may want to SendStayActive after OpDiscovery.
return CommissioningStage::kICDSendStayActive;
case CommissioningStage::kICDSendStayActive:
// TODO(cecille): device attestation casues operational cert provisioning to happen, This should be a separate stage.
// For thread and wifi, this should go to network setup then enable. For on-network we can skip right to finding the
// operational network because the provisioning of certificates will trigger the device to start operational advertising.
Expand All @@ -446,7 +443,7 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
{
return CommissioningStage::kCleanup;
}
return CommissioningStage::kFindOperational;
return CommissioningStage::kEvictPreviousCaseSessions;
}
case CommissioningStage::kScanNetworks:
return CommissioningStage::kNeedsNetworkCreds;
Expand Down Expand Up @@ -489,16 +486,22 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
else
{
SetCASEFailsafeTimerIfNeeded();
return CommissioningStage::kFindOperational;
return CommissioningStage::kEvictPreviousCaseSessions;
}
case CommissioningStage::kThreadNetworkEnable:
SetCASEFailsafeTimerIfNeeded();
if (mParams.GetSkipCommissioningComplete().ValueOr(false))
{
return CommissioningStage::kCleanup;
}
return CommissioningStage::kFindOperational;
case CommissioningStage::kFindOperational:
return CommissioningStage::kEvictPreviousCaseSessions;
case CommissioningStage::kEvictPreviousCaseSessions:
return CommissioningStage::kFindOperationalForStayActive;
case CommissioningStage::kFindOperationalForStayActive:
return CommissioningStage::kICDSendStayActive;
case CommissioningStage::kICDSendStayActive:
return CommissioningStage::kFindOperationalForCommissioningComplete;
case CommissioningStage::kFindOperationalForCommissioningComplete:
return CommissioningStage::kSendComplete;
case CommissioningStage::kSendComplete:
return CommissioningStage::kCleanup;
Expand Down Expand Up @@ -539,7 +542,7 @@ void AutoCommissioner::SetCASEFailsafeTimerIfNeeded()
//
// A false return from ExtendArmFailSafe is fine; we don't want to make
// the fail-safe shorter here.
mCommissioner->ExtendArmFailSafe(mCommissioneeDeviceProxy, CommissioningStage::kFindOperational,
mCommissioner->ExtendArmFailSafe(mCommissioneeDeviceProxy, mCommissioner->GetCommissioningStage(),
mParams.GetCASEFailsafeTimerSeconds().Value(),
GetCommandTimeout(mCommissioneeDeviceProxy, CommissioningStage::kArmFailsafe),
OnExtendFailsafeSuccessForCASE, OnFailsafeFailureForCASE);
Expand Down Expand Up @@ -754,6 +757,11 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
mNeedIcdRegistration = true;
ChipLogDetail(Controller, "AutoCommissioner: ICD supports the check-in protocol.");
}
else if (mParams.GetICDStayActiveDurationMsec().HasValue())
{
ChipLogDetail(Controller, "AutoCommissioner: Clear ICD StayActiveDurationMsec");
mParams.ClearICDStayActiveDurationMsec();
}
}
break;
}
Expand Down Expand Up @@ -822,10 +830,8 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
case CommissioningStage::kICDRegistration:
// Noting to do. DevicePairingDelegate will handle this.
break;
case CommissioningStage::kICDSendStayActive:
// Nothing to do.
break;
case CommissioningStage::kFindOperational:
case CommissioningStage::kFindOperationalForStayActive:
case CommissioningStage::kFindOperationalForCommissioningComplete:
mOperationalDeviceProxy = report.Get<OperationalNodeFoundData>().operationalProxy;
break;
case CommissioningStage::kCleanup:
Expand Down Expand Up @@ -861,7 +867,7 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio

DeviceProxy * AutoCommissioner::GetDeviceProxyForStep(CommissioningStage nextStage)
{
if (nextStage == CommissioningStage::kSendComplete ||
if (nextStage == CommissioningStage::kSendComplete || nextStage == CommissioningStage::kICDSendStayActive ||
(nextStage == CommissioningStage::kCleanup && mOperationalDeviceProxy.GetDeviceId() != kUndefinedNodeId))
{
return &mOperationalDeviceProxy;
Expand Down
1 change: 0 additions & 1 deletion src/controller/AutoCommissioner.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ class AutoCommissioner : public CommissioningDelegate
bool mNeedsDST = false;

bool mNeedIcdRegistration = false;

// TODO: Why were the nonces statically allocated, but the certs dynamically allocated?
uint8_t * mDAC = nullptr;
uint16_t mDACLen = 0;
Expand Down
84 changes: 63 additions & 21 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1201,24 +1201,39 @@ void DeviceCommissioner::OnFailedToExtendedArmFailSafeDeviceAttestation(void * c
void DeviceCommissioner::OnICDManagementRegisterClientResponse(
void * context, const app::Clusters::IcdManagement::Commands::RegisterClientResponse::DecodableType & data)
{
CHIP_ERROR err = CHIP_NO_ERROR;
DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
VerifyOrReturn(commissioner != nullptr, ChipLogProgress(Controller, "Command response callback with null context. Ignoring"));
VerifyOrExit(commissioner != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrExit(commissioner->mCommissioningStage == CommissioningStage::kICDRegistration, err = CHIP_ERROR_INCORRECT_STATE);
VerifyOrExit(commissioner->mDeviceBeingCommissioned != nullptr, err = CHIP_ERROR_INCORRECT_STATE);

if (commissioner->mCommissioningStage != CommissioningStage::kICDRegistration)
if (commissioner->mPairingDelegate != nullptr)
{
return;
commissioner->mPairingDelegate->OnICDRegistrationComplete(commissioner->mDeviceBeingCommissioned->GetDeviceId(),
data.ICDCounter);
}

if (commissioner->mDeviceBeingCommissioned == nullptr)
{
return;
}
exit:
CommissioningDelegate::CommissioningReport report;
commissioner->CommissioningStageComplete(err, report);
}

void DeviceCommissioner::OnICDManagementStayActiveResponse(
void * context, const app::Clusters::IcdManagement::Commands::StayActiveResponse::DecodableType & data)
{
CHIP_ERROR err = CHIP_NO_ERROR;
DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
VerifyOrExit(commissioner != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrExit(commissioner->mCommissioningStage == CommissioningStage::kICDSendStayActive, err = CHIP_ERROR_INCORRECT_STATE);
VerifyOrExit(commissioner->mDeviceBeingCommissioned != nullptr, err = CHIP_ERROR_INCORRECT_STATE);

if (commissioner->mPairingDelegate != nullptr)
{
commissioner->mPairingDelegate->OnICDRegistrationComplete(commissioner->mDeviceBeingCommissioned->GetDeviceId(),
data.ICDCounter);
commissioner->mPairingDelegate->OnICDStayActiveComplete(commissioner->mDeviceBeingCommissioned->GetDeviceId(),
data.promisedActiveDuration);
}

exit:
CommissioningDelegate::CommissioningReport report;
commissioner->CommissioningStageComplete(CHIP_NO_ERROR, report);
}
Expand Down Expand Up @@ -1854,7 +1869,8 @@ void DeviceCommissioner::OnDeviceConnectedFn(void * context, Messaging::Exchange
{
// CASE session established.
DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
VerifyOrDie(commissioner->mCommissioningStage == CommissioningStage::kFindOperational);
VerifyOrDie(commissioner->mCommissioningStage == CommissioningStage::kFindOperationalForStayActive ||
commissioner->mCommissioningStage == CommissioningStage::kFindOperationalForCommissioningComplete);
VerifyOrDie(commissioner->mDeviceBeingCommissioned->GetDeviceId() == sessionHandle->GetPeer().GetNodeId());
commissioner->CancelCASECallbacks(); // ensure all CASE callbacks are unregistered

Expand All @@ -1867,7 +1883,8 @@ void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, const Scope
{
// CASE session establishment failed.
DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
VerifyOrDie(commissioner->mCommissioningStage == CommissioningStage::kFindOperational);
VerifyOrDie(commissioner->mCommissioningStage == CommissioningStage::kFindOperationalForStayActive ||
commissioner->mCommissioningStage == CommissioningStage::kFindOperationalForCommissioningComplete);
VerifyOrDie(commissioner->mDeviceBeingCommissioned->GetDeviceId() == peerId.GetNodeId());
commissioner->CancelCASECallbacks(); // ensure all CASE callbacks are unregistered

Expand Down Expand Up @@ -1908,7 +1925,8 @@ void DeviceCommissioner::OnDeviceConnectionRetryFn(void * context, const ScopedN
ChipLogValueScopedNodeId(peerId), error.Format(), retryTimeout.count());

auto self = static_cast<DeviceCommissioner *>(context);
VerifyOrDie(self->mCommissioningStage == CommissioningStage::kFindOperational);
VerifyOrDie(self->GetCommissioningStage() == CommissioningStage::kFindOperationalForStayActive ||
self->GetCommissioningStage() == CommissioningStage::kFindOperationalForCommissioningComplete);
VerifyOrDie(self->mDeviceBeingCommissioned->GetDeviceId() == peerId.GetNodeId());

// We need to do the fail-safe arming over the PASE session.
Expand Down Expand Up @@ -1936,7 +1954,7 @@ void DeviceCommissioner::OnDeviceConnectionRetryFn(void * context, const ScopedN
}

// A false return is fine; we don't want to make the fail-safe shorter here.
self->ExtendArmFailSafeInternal(commissioneeDevice, CommissioningStage::kFindOperational, failsafeTimeout,
self->ExtendArmFailSafeInternal(commissioneeDevice, self->GetCommissioningStage(), failsafeTimeout,
MakeOptional(kMinimumCommissioningStepTimeout), OnExtendFailsafeForCASERetrySuccess,
OnExtendFailsafeForCASERetryFailure, /* fireAndForget = */ true);
}
Expand Down Expand Up @@ -3180,13 +3198,7 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
}
}
break;
case CommissioningStage::kICDSendStayActive: {
// TODO(#24259): Send StayActiveRequest once server supports this.
CommissioningStageComplete(CHIP_NO_ERROR);
}
break;
case CommissioningStage::kFindOperational: {
// If there is an error, CommissioningStageComplete will be called from OnDeviceConnectionFailureFn.
case CommissioningStage::kEvictPreviousCaseSessions: {
auto scopedPeerId = GetPeerScopedId(proxy->GetDeviceId());

// If we ever had a commissioned device with this node ID before, we may
Expand All @@ -3196,7 +3208,13 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
// clearing the ones associated with our fabric index is good enough and
// we don't need to worry about ExpireAllSessionsOnLogicalFabric.
mSystemState->SessionMgr()->ExpireAllSessions(scopedPeerId);

CommissioningStageComplete(CHIP_NO_ERROR);
return;
}
case CommissioningStage::kFindOperationalForStayActive:
case CommissioningStage::kFindOperationalForCommissioningComplete: {
// If there is an error, CommissioningStageComplete will be called from OnDeviceConnectionFailureFn.
auto scopedPeerId = GetPeerScopedId(proxy->GetDeviceId());
mSystemState->CASESessionMgr()->FindOrEstablishSession(scopedPeerId, &mOnDeviceConnectedCallback,
&mOnDeviceConnectionFailureCallback
#if CHIP_DEVICE_CONFIG_ENABLE_AUTOMATIC_CASE_RETRIES
Expand All @@ -3206,7 +3224,31 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
);
}
break;
case CommissioningStage::kICDSendStayActive: {
if (!(params.GetICDStayActiveDurationMsec().HasValue()))
{
ChipLogProgress(Controller, "Skipping kICDSendStayActive");
CommissioningStageComplete(CHIP_NO_ERROR);
return;
}

// StayActive Command happens over CASE Connection
IcdManagement::Commands::StayActiveRequest::Type request;
request.stayActiveDuration = params.GetICDStayActiveDurationMsec().Value();
ChipLogError(Controller, "Send ICD StayActive with Duration %u", request.stayActiveDuration);
CHIP_ERROR err =
SendCommissioningCommand(proxy, request, OnICDManagementStayActiveResponse, OnBasicFailure, endpoint, timeout);
if (err != CHIP_NO_ERROR)
{
// We won't get any async callbacks here, so just complete our stage.
ChipLogError(Controller, "Failed to send IcdManagement.StayActive command: %" CHIP_ERROR_FORMAT, err.Format());
CommissioningStageComplete(err);
return;
}
}
break;
case CommissioningStage::kSendComplete: {
// CommissioningComplete command happens over the CASE connection.
GeneralCommissioning::Commands::CommissioningComplete::Type request;
CHIP_ERROR err =
SendCommissioningCommand(proxy, request, OnCommissioningCompleteResponse, OnBasicFailure, endpoint, timeout);
Expand Down
4 changes: 4 additions & 0 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,10 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
static void OnICDManagementRegisterClientResponse(
void * context, const app::Clusters::IcdManagement::Commands::RegisterClientResponse::DecodableType & data);

static void
OnICDManagementStayActiveResponse(void * context,
const app::Clusters::IcdManagement::Commands::StayActiveResponse::DecodableType & data);

/**
* @brief
* This function processes the CSR sent by the device.
Expand Down
Loading

0 comments on commit 813181a

Please sign in to comment.