Skip to content

Commit 813181a

Browse files
[ICD] Invoke stayActive in the end of commissioning flow (#32496)
* 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 <commits@restyled.io>
1 parent eed2a1a commit 813181a

9 files changed

+147
-86
lines changed

examples/chip-tool/commands/pairing/PairingCommand.cpp

+10
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,10 @@ CommissioningParameters PairingCommand::GetCommissioningParameters()
154154
// These Optionals must have values now.
155155
// The commissioner will verify these values.
156156
params.SetICDSymmetricKey(mICDSymmetricKey.Value());
157+
if (mICDStayActiveDurationMsec.HasValue())
158+
{
159+
params.SetICDStayActiveDurationMsec(mICDStayActiveDurationMsec.Value());
160+
}
157161
params.SetICDCheckInNodeId(mICDCheckInNodeId.Value());
158162
params.SetICDMonitoredSubject(mICDMonitoredSubject.Value());
159163
}
@@ -465,6 +469,12 @@ void PairingCommand::OnICDRegistrationComplete(NodeId nodeId, uint32_t icdCounte
465469
ChipLogValueX64(mICDMonitoredSubject.Value()), icdSymmetricKeyHex);
466470
}
467471

472+
void PairingCommand::OnICDStayActiveComplete(NodeId deviceId, uint32_t promisedActiveDuration)
473+
{
474+
ChipLogProgress(chipTool, "ICD Stay Active Complete for device " ChipLogFormatX64 " / promisedActiveDuration: %u",
475+
ChipLogValueX64(deviceId), promisedActiveDuration);
476+
}
477+
468478
void PairingCommand::OnDiscoveredDevice(const chip::Dnssd::DiscoveredNodeData & nodeData)
469479
{
470480
// Ignore nodes with closed commissioning window

examples/chip-tool/commands/pairing/PairingCommand.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ class PairingCommand : public CHIPCommand,
7272
AddArgument("icd-monitored-subject", 0, UINT64_MAX, &mICDMonitoredSubject,
7373
"The monitored subject of the ICD, default: The node id used for icd-check-in-nodeid.");
7474
AddArgument("icd-symmetric-key", &mICDSymmetricKey, "The 16 bytes ICD symmetric key, default: randomly generated.");
75-
75+
AddArgument("icd-stay-active-duration", 0, UINT32_MAX, &mICDStayActiveDurationMsec,
76+
"If set, a LIT ICD that is commissioned will be requested to stay active for this many milliseconds");
7677
switch (networkType)
7778
{
7879
case PairingNetworkType::None:
@@ -197,6 +198,7 @@ class PairingCommand : public CHIPCommand,
197198
void OnReadCommissioningInfo(const chip::Controller::ReadCommissioningInfo & info) override;
198199
void OnCommissioningComplete(NodeId deviceId, CHIP_ERROR error) override;
199200
void OnICDRegistrationComplete(NodeId deviceId, uint32_t icdCounter) override;
201+
void OnICDStayActiveComplete(NodeId deviceId, uint32_t promisedActiveDuration) override;
200202

201203
/////////// DeviceDiscoveryDelegate Interface /////////
202204
void OnDiscoveredDevice(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
@@ -235,6 +237,7 @@ class PairingCommand : public CHIPCommand,
235237
chip::Optional<NodeId> mICDCheckInNodeId;
236238
chip::Optional<chip::ByteSpan> mICDSymmetricKey;
237239
chip::Optional<uint64_t> mICDMonitoredSubject;
240+
chip::Optional<uint32_t> mICDStayActiveDurationMsec;
238241
chip::app::DataModel::List<chip::app::Clusters::TimeSynchronization::Structs::TimeZoneStruct::Type> mTimeZoneList;
239242
TypedComplexArgument<chip::app::DataModel::List<chip::app::Clusters::TimeSynchronization::Structs::TimeZoneStruct::Type>>
240243
mComplex_TimeZones;

src/controller/AutoCommissioner.cpp

+20-14
Original file line numberDiff line numberDiff line change
@@ -416,13 +416,10 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
416416
}
417417
return CommissioningStage::kICDGetRegistrationInfo;
418418
}
419-
return GetNextCommissioningStageInternal(CommissioningStage::kICDSendStayActive, lastErr);
419+
return GetNextCommissioningStageInternal(CommissioningStage::kICDRegistration, lastErr);
420420
case CommissioningStage::kICDGetRegistrationInfo:
421421
return CommissioningStage::kICDRegistration;
422422
case CommissioningStage::kICDRegistration:
423-
// TODO(#24259): StayActiveRequest is not supported by server. We may want to SendStayActive after OpDiscovery.
424-
return CommissioningStage::kICDSendStayActive;
425-
case CommissioningStage::kICDSendStayActive:
426423
// TODO(cecille): device attestation casues operational cert provisioning to happen, This should be a separate stage.
427424
// For thread and wifi, this should go to network setup then enable. For on-network we can skip right to finding the
428425
// operational network because the provisioning of certificates will trigger the device to start operational advertising.
@@ -446,7 +443,7 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
446443
{
447444
return CommissioningStage::kCleanup;
448445
}
449-
return CommissioningStage::kFindOperational;
446+
return CommissioningStage::kEvictPreviousCaseSessions;
450447
}
451448
case CommissioningStage::kScanNetworks:
452449
return CommissioningStage::kNeedsNetworkCreds;
@@ -489,16 +486,22 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
489486
else
490487
{
491488
SetCASEFailsafeTimerIfNeeded();
492-
return CommissioningStage::kFindOperational;
489+
return CommissioningStage::kEvictPreviousCaseSessions;
493490
}
494491
case CommissioningStage::kThreadNetworkEnable:
495492
SetCASEFailsafeTimerIfNeeded();
496493
if (mParams.GetSkipCommissioningComplete().ValueOr(false))
497494
{
498495
return CommissioningStage::kCleanup;
499496
}
500-
return CommissioningStage::kFindOperational;
501-
case CommissioningStage::kFindOperational:
497+
return CommissioningStage::kEvictPreviousCaseSessions;
498+
case CommissioningStage::kEvictPreviousCaseSessions:
499+
return CommissioningStage::kFindOperationalForStayActive;
500+
case CommissioningStage::kFindOperationalForStayActive:
501+
return CommissioningStage::kICDSendStayActive;
502+
case CommissioningStage::kICDSendStayActive:
503+
return CommissioningStage::kFindOperationalForCommissioningComplete;
504+
case CommissioningStage::kFindOperationalForCommissioningComplete:
502505
return CommissioningStage::kSendComplete;
503506
case CommissioningStage::kSendComplete:
504507
return CommissioningStage::kCleanup;
@@ -539,7 +542,7 @@ void AutoCommissioner::SetCASEFailsafeTimerIfNeeded()
539542
//
540543
// A false return from ExtendArmFailSafe is fine; we don't want to make
541544
// the fail-safe shorter here.
542-
mCommissioner->ExtendArmFailSafe(mCommissioneeDeviceProxy, CommissioningStage::kFindOperational,
545+
mCommissioner->ExtendArmFailSafe(mCommissioneeDeviceProxy, mCommissioner->GetCommissioningStage(),
543546
mParams.GetCASEFailsafeTimerSeconds().Value(),
544547
GetCommandTimeout(mCommissioneeDeviceProxy, CommissioningStage::kArmFailsafe),
545548
OnExtendFailsafeSuccessForCASE, OnFailsafeFailureForCASE);
@@ -754,6 +757,11 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
754757
mNeedIcdRegistration = true;
755758
ChipLogDetail(Controller, "AutoCommissioner: ICD supports the check-in protocol.");
756759
}
760+
else if (mParams.GetICDStayActiveDurationMsec().HasValue())
761+
{
762+
ChipLogDetail(Controller, "AutoCommissioner: Clear ICD StayActiveDurationMsec");
763+
mParams.ClearICDStayActiveDurationMsec();
764+
}
757765
}
758766
break;
759767
}
@@ -822,10 +830,8 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
822830
case CommissioningStage::kICDRegistration:
823831
// Noting to do. DevicePairingDelegate will handle this.
824832
break;
825-
case CommissioningStage::kICDSendStayActive:
826-
// Nothing to do.
827-
break;
828-
case CommissioningStage::kFindOperational:
833+
case CommissioningStage::kFindOperationalForStayActive:
834+
case CommissioningStage::kFindOperationalForCommissioningComplete:
829835
mOperationalDeviceProxy = report.Get<OperationalNodeFoundData>().operationalProxy;
830836
break;
831837
case CommissioningStage::kCleanup:
@@ -861,7 +867,7 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
861867

862868
DeviceProxy * AutoCommissioner::GetDeviceProxyForStep(CommissioningStage nextStage)
863869
{
864-
if (nextStage == CommissioningStage::kSendComplete ||
870+
if (nextStage == CommissioningStage::kSendComplete || nextStage == CommissioningStage::kICDSendStayActive ||
865871
(nextStage == CommissioningStage::kCleanup && mOperationalDeviceProxy.GetDeviceId() != kUndefinedNodeId))
866872
{
867873
return &mOperationalDeviceProxy;

src/controller/AutoCommissioner.h

-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ class AutoCommissioner : public CommissioningDelegate
123123
bool mNeedsDST = false;
124124

125125
bool mNeedIcdRegistration = false;
126-
127126
// TODO: Why were the nonces statically allocated, but the certs dynamically allocated?
128127
uint8_t * mDAC = nullptr;
129128
uint16_t mDACLen = 0;

src/controller/CHIPDeviceController.cpp

+63-21
Original file line numberDiff line numberDiff line change
@@ -1201,24 +1201,39 @@ void DeviceCommissioner::OnFailedToExtendedArmFailSafeDeviceAttestation(void * c
12011201
void DeviceCommissioner::OnICDManagementRegisterClientResponse(
12021202
void * context, const app::Clusters::IcdManagement::Commands::RegisterClientResponse::DecodableType & data)
12031203
{
1204+
CHIP_ERROR err = CHIP_NO_ERROR;
12041205
DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
1205-
VerifyOrReturn(commissioner != nullptr, ChipLogProgress(Controller, "Command response callback with null context. Ignoring"));
1206+
VerifyOrExit(commissioner != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
1207+
VerifyOrExit(commissioner->mCommissioningStage == CommissioningStage::kICDRegistration, err = CHIP_ERROR_INCORRECT_STATE);
1208+
VerifyOrExit(commissioner->mDeviceBeingCommissioned != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
12061209

1207-
if (commissioner->mCommissioningStage != CommissioningStage::kICDRegistration)
1210+
if (commissioner->mPairingDelegate != nullptr)
12081211
{
1209-
return;
1212+
commissioner->mPairingDelegate->OnICDRegistrationComplete(commissioner->mDeviceBeingCommissioned->GetDeviceId(),
1213+
data.ICDCounter);
12101214
}
12111215

1212-
if (commissioner->mDeviceBeingCommissioned == nullptr)
1213-
{
1214-
return;
1215-
}
1216+
exit:
1217+
CommissioningDelegate::CommissioningReport report;
1218+
commissioner->CommissioningStageComplete(err, report);
1219+
}
1220+
1221+
void DeviceCommissioner::OnICDManagementStayActiveResponse(
1222+
void * context, const app::Clusters::IcdManagement::Commands::StayActiveResponse::DecodableType & data)
1223+
{
1224+
CHIP_ERROR err = CHIP_NO_ERROR;
1225+
DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
1226+
VerifyOrExit(commissioner != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
1227+
VerifyOrExit(commissioner->mCommissioningStage == CommissioningStage::kICDSendStayActive, err = CHIP_ERROR_INCORRECT_STATE);
1228+
VerifyOrExit(commissioner->mDeviceBeingCommissioned != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
12161229

12171230
if (commissioner->mPairingDelegate != nullptr)
12181231
{
1219-
commissioner->mPairingDelegate->OnICDRegistrationComplete(commissioner->mDeviceBeingCommissioned->GetDeviceId(),
1220-
data.ICDCounter);
1232+
commissioner->mPairingDelegate->OnICDStayActiveComplete(commissioner->mDeviceBeingCommissioned->GetDeviceId(),
1233+
data.promisedActiveDuration);
12211234
}
1235+
1236+
exit:
12221237
CommissioningDelegate::CommissioningReport report;
12231238
commissioner->CommissioningStageComplete(CHIP_NO_ERROR, report);
12241239
}
@@ -1854,7 +1869,8 @@ void DeviceCommissioner::OnDeviceConnectedFn(void * context, Messaging::Exchange
18541869
{
18551870
// CASE session established.
18561871
DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
1857-
VerifyOrDie(commissioner->mCommissioningStage == CommissioningStage::kFindOperational);
1872+
VerifyOrDie(commissioner->mCommissioningStage == CommissioningStage::kFindOperationalForStayActive ||
1873+
commissioner->mCommissioningStage == CommissioningStage::kFindOperationalForCommissioningComplete);
18581874
VerifyOrDie(commissioner->mDeviceBeingCommissioned->GetDeviceId() == sessionHandle->GetPeer().GetNodeId());
18591875
commissioner->CancelCASECallbacks(); // ensure all CASE callbacks are unregistered
18601876

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

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

19101927
auto self = static_cast<DeviceCommissioner *>(context);
1911-
VerifyOrDie(self->mCommissioningStage == CommissioningStage::kFindOperational);
1928+
VerifyOrDie(self->GetCommissioningStage() == CommissioningStage::kFindOperationalForStayActive ||
1929+
self->GetCommissioningStage() == CommissioningStage::kFindOperationalForCommissioningComplete);
19121930
VerifyOrDie(self->mDeviceBeingCommissioned->GetDeviceId() == peerId.GetNodeId());
19131931

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

19381956
// A false return is fine; we don't want to make the fail-safe shorter here.
1939-
self->ExtendArmFailSafeInternal(commissioneeDevice, CommissioningStage::kFindOperational, failsafeTimeout,
1957+
self->ExtendArmFailSafeInternal(commissioneeDevice, self->GetCommissioningStage(), failsafeTimeout,
19401958
MakeOptional(kMinimumCommissioningStepTimeout), OnExtendFailsafeForCASERetrySuccess,
19411959
OnExtendFailsafeForCASERetryFailure, /* fireAndForget = */ true);
19421960
}
@@ -3180,13 +3198,7 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
31803198
}
31813199
}
31823200
break;
3183-
case CommissioningStage::kICDSendStayActive: {
3184-
// TODO(#24259): Send StayActiveRequest once server supports this.
3185-
CommissioningStageComplete(CHIP_NO_ERROR);
3186-
}
3187-
break;
3188-
case CommissioningStage::kFindOperational: {
3189-
// If there is an error, CommissioningStageComplete will be called from OnDeviceConnectionFailureFn.
3201+
case CommissioningStage::kEvictPreviousCaseSessions: {
31903202
auto scopedPeerId = GetPeerScopedId(proxy->GetDeviceId());
31913203

31923204
// If we ever had a commissioned device with this node ID before, we may
@@ -3196,7 +3208,13 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
31963208
// clearing the ones associated with our fabric index is good enough and
31973209
// we don't need to worry about ExpireAllSessionsOnLogicalFabric.
31983210
mSystemState->SessionMgr()->ExpireAllSessions(scopedPeerId);
3199-
3211+
CommissioningStageComplete(CHIP_NO_ERROR);
3212+
return;
3213+
}
3214+
case CommissioningStage::kFindOperationalForStayActive:
3215+
case CommissioningStage::kFindOperationalForCommissioningComplete: {
3216+
// If there is an error, CommissioningStageComplete will be called from OnDeviceConnectionFailureFn.
3217+
auto scopedPeerId = GetPeerScopedId(proxy->GetDeviceId());
32003218
mSystemState->CASESessionMgr()->FindOrEstablishSession(scopedPeerId, &mOnDeviceConnectedCallback,
32013219
&mOnDeviceConnectionFailureCallback
32023220
#if CHIP_DEVICE_CONFIG_ENABLE_AUTOMATIC_CASE_RETRIES
@@ -3206,7 +3224,31 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
32063224
);
32073225
}
32083226
break;
3227+
case CommissioningStage::kICDSendStayActive: {
3228+
if (!(params.GetICDStayActiveDurationMsec().HasValue()))
3229+
{
3230+
ChipLogProgress(Controller, "Skipping kICDSendStayActive");
3231+
CommissioningStageComplete(CHIP_NO_ERROR);
3232+
return;
3233+
}
3234+
3235+
// StayActive Command happens over CASE Connection
3236+
IcdManagement::Commands::StayActiveRequest::Type request;
3237+
request.stayActiveDuration = params.GetICDStayActiveDurationMsec().Value();
3238+
ChipLogError(Controller, "Send ICD StayActive with Duration %u", request.stayActiveDuration);
3239+
CHIP_ERROR err =
3240+
SendCommissioningCommand(proxy, request, OnICDManagementStayActiveResponse, OnBasicFailure, endpoint, timeout);
3241+
if (err != CHIP_NO_ERROR)
3242+
{
3243+
// We won't get any async callbacks here, so just complete our stage.
3244+
ChipLogError(Controller, "Failed to send IcdManagement.StayActive command: %" CHIP_ERROR_FORMAT, err.Format());
3245+
CommissioningStageComplete(err);
3246+
return;
3247+
}
3248+
}
3249+
break;
32093250
case CommissioningStage::kSendComplete: {
3251+
// CommissioningComplete command happens over the CASE connection.
32103252
GeneralCommissioning::Commands::CommissioningComplete::Type request;
32113253
CHIP_ERROR err =
32123254
SendCommissioningCommand(proxy, request, OnCommissioningCompleteResponse, OnBasicFailure, endpoint, timeout);

src/controller/CHIPDeviceController.h

+4
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,10 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
921921
static void OnICDManagementRegisterClientResponse(
922922
void * context, const app::Clusters::IcdManagement::Commands::RegisterClientResponse::DecodableType & data);
923923

924+
static void
925+
OnICDManagementStayActiveResponse(void * context,
926+
const app::Clusters::IcdManagement::Commands::StayActiveResponse::DecodableType & data);
927+
924928
/**
925929
* @brief
926930
* This function processes the CSR sent by the device.

0 commit comments

Comments
 (0)