Skip to content

Commit

Permalink
Commissioning: Implement time sync requirements (#27812)
Browse files Browse the repository at this point in the history
* Commissioning: Implement time sync requirements

Tests added in TestCommissioningTimeSync.py

Test: Ran TestCommissioningTimeSync.py against all-clusters
      (has a time cluster) and lock (no time cluster)
      commissioned all-clusters and lock with chip-tool

* Add missing include

* Note to self: commit ALL changes before uploading

* Restyled by isort

* Fix CGEN test for commissioning.

* Read time sync in initial read, fabrics later

Also fix tests

* Remove extra log

* Add new pairing delegate call, fix tv-app

---------

Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
cecille and restyled-commits authored Jul 18, 2023
1 parent 6844dd4 commit ac3535b
Show file tree
Hide file tree
Showing 14 changed files with 876 additions and 126 deletions.
1 change: 1 addition & 0 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,7 @@ jobs:
scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace_decode 1" --script "src/python_testing/TC_DA_1_2.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values"'
scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace_decode 1" --script "src/python_testing/TC_TIMESYNC_3_1.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021"'
scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace_decode 1" --script "src/python_testing/TC_DA_1_5.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values"'
scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace_decode 1" --script "src/python_testing/TestCommissioningTimeSync.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021"'
scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --app out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app --factoryreset --app-args "--discriminator 1234 --KVS kvs1 --trace_decode 1" --script "src/python_testing/TC_IDM_1_2.py" --script-args "--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021"'
scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --script "src/python_testing/TestMatterTestingSupport.py"'
- name: Uploading core files
Expand Down
4 changes: 4 additions & 0 deletions examples/platform/linux/CommissionerMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ class PairingCommand : public Controller::DevicePairingDelegate
void OnCommissioningStatusUpdate(PeerId peerId, CommissioningStage stageCompleted, CHIP_ERROR error) override;

void OnReadCommissioningInfo(const ReadCommissioningInfo & info) override;
void OnFabricCheck(const MatchingFabricInfo & info) override;

private:
#if CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED
Expand Down Expand Up @@ -344,7 +345,10 @@ void PairingCommand::OnReadCommissioningInfo(const ReadCommissioningInfo & info)
{
ChipLogProgress(AppServer, "OnReadCommissioningInfo - vendorId=0x%04X productId=0x%04X", info.basic.vendorId,
info.basic.productId);
}

void PairingCommand::OnFabricCheck(const MatchingFabricInfo & info)
{
if (info.nodeId != kUndefinedNodeId)
{
ChipLogProgress(AppServer, "ALREADY ON FABRIC WITH nodeId=0x" ChipLogFormatX64, ChipLogValueX64(info.nodeId));
Expand Down
86 changes: 84 additions & 2 deletions src/controller/AutoCommissioner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,53 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
// Per the spec, we restart from after adding the NOC.
return GetNextCommissioningStage(CommissioningStage::kSendNOC, lastErr);
}
if (mParams.GetCheckForMatchingFabric())
{
return CommissioningStage::kCheckForMatchingFabric;
}
return CommissioningStage::kArmFailsafe;
case CommissioningStage::kCheckForMatchingFabric:
return CommissioningStage::kArmFailsafe;
case CommissioningStage::kArmFailsafe:
return CommissioningStage::kConfigRegulatory;
case CommissioningStage::kConfigRegulatory:
if (mDeviceCommissioningInfo.requiresUTC)
{
return CommissioningStage::kConfigureUTCTime;
}
else
{
// Time cluster is not supported, move right to DA
return CommissioningStage::kSendPAICertificateRequest;
}
case CommissioningStage::kConfigureUTCTime:
if (mDeviceCommissioningInfo.requiresTimeZone && mParams.GetTimeZone().HasValue())
{
return kConfigureTimeZone;
}
else
{
return GetNextCommissioningStageInternal(CommissioningStage::kConfigureTimeZone, lastErr);
}
case CommissioningStage::kConfigureTimeZone:
if (mNeedsDST && mParams.GetDSTOffsets().HasValue())
{
return CommissioningStage::kConfigureDSTOffset;
}
else
{
return GetNextCommissioningStageInternal(CommissioningStage::kConfigureDSTOffset, lastErr);
}
case CommissioningStage::kConfigureDSTOffset:
if (mDeviceCommissioningInfo.requiresDefaultNTP && mParams.GetDefaultNTP().HasValue())
{
return CommissioningStage::kConfigureDefaultNTP;
}
else
{
return GetNextCommissioningStageInternal(CommissioningStage::kConfigureDefaultNTP, lastErr);
}
case CommissioningStage::kConfigureDefaultNTP:
return CommissioningStage::kSendPAICertificateRequest;
case CommissioningStage::kSendPAICertificateRequest:
return CommissioningStage::kSendDACCertificateRequest;
Expand All @@ -264,6 +307,15 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
case CommissioningStage::kSendTrustedRootCert:
return CommissioningStage::kSendNOC;
case CommissioningStage::kSendNOC:
if (mDeviceCommissioningInfo.requiresTrustedTimeSource && mParams.GetTrustedTimeSource().HasValue())
{
return CommissioningStage::kConfigureTrustedTimeSource;
}
else
{
return GetNextCommissioningStageInternal(CommissioningStage::kConfigureTrustedTimeSource, lastErr);
}
case CommissioningStage::kConfigureTrustedTimeSource:
// 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 Down Expand Up @@ -580,11 +632,20 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
.SetRemoteProductId(mDeviceCommissioningInfo.basic.productId)
.SetDefaultRegulatoryLocation(mDeviceCommissioningInfo.general.currentRegulatoryLocation)
.SetLocationCapability(mDeviceCommissioningInfo.general.locationCapability);
if (mDeviceCommissioningInfo.nodeId != kUndefinedNodeId)
// Don't send DST unless the device says it needs it
mNeedsDST = false;
break;
case CommissioningStage::kCheckForMatchingFabric: {
chip::NodeId nodeId = report.Get<MatchingFabricInfo>().nodeId;
if (nodeId != kUndefinedNodeId)
{
mParams.SetRemoteNodeId(mDeviceCommissioningInfo.nodeId);
mParams.SetRemoteNodeId(nodeId);
}
break;
}
case CommissioningStage::kConfigureTimeZone:
mNeedsDST = report.Get<TimeZoneResponseInfo>().requiresDSTOffsets;
break;
case CommissioningStage::kSendPAICertificateRequest:
SetPAI(report.Get<RequestedCertificate>().certificate);
break;
Expand Down Expand Up @@ -650,6 +711,7 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
mCommissioneeDeviceProxy = nullptr;
mOperationalDeviceProxy = OperationalDeviceProxy();
mDeviceCommissioningInfo = ReadCommissioningInfo();
mNeedsDST = false;
return CHIP_NO_ERROR;
default:
break;
Expand Down Expand Up @@ -692,6 +754,26 @@ CHIP_ERROR AutoCommissioner::PerformStep(CommissioningStage nextStage)
ChipLogError(Controller, "Invalid device for commissioning");
return CHIP_ERROR_INCORRECT_STATE;
}
// Perform any last minute parameter adjustments before calling the commissioner object
switch (nextStage)
{
case CommissioningStage::kConfigureTimeZone:
if (mParams.GetTimeZone().Value().size() > mDeviceCommissioningInfo.maxTimeZoneSize)
{
mParams.SetTimeZone(app::DataModel::List<app::Clusters::TimeSynchronization::Structs::TimeZoneStruct::Type>(
mParams.GetTimeZone().Value().SubSpan(0, mDeviceCommissioningInfo.maxTimeZoneSize)));
}
break;
case CommissioningStage::kConfigureDSTOffset:
if (mParams.GetDSTOffsets().Value().size() > mDeviceCommissioningInfo.maxDSTSize)
{
mParams.SetDSTOffsets(app::DataModel::List<app::Clusters::TimeSynchronization::Structs::DSTOffsetStruct::Type>(
mParams.GetDSTOffsets().Value().SubSpan(0, mDeviceCommissioningInfo.maxDSTSize)));
}
break;
default:
break;
}

mCommissioner->PerformCommissioningStep(proxy, nextStage, mParams, this, GetEndpoint(nextStage),
GetCommandTimeout(proxy, nextStage));
Expand Down
3 changes: 2 additions & 1 deletion src/controller/AutoCommissioner.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class AutoCommissioner : public CommissioningDelegate
* be used for sending the relevant command.
*/
Optional<System::Clock::Timeout> GetCommandTimeout(DeviceProxy * device, CommissioningStage stage) const;
CommissioningParameters mParams = CommissioningParameters();

private:
DeviceProxy * GetDeviceProxyForStep(CommissioningStage nextStage);
Expand Down Expand Up @@ -94,7 +95,6 @@ class AutoCommissioner : public CommissioningDelegate
DeviceCommissioner * mCommissioner = nullptr;
CommissioneeDeviceProxy * mCommissioneeDeviceProxy = nullptr;
OperationalCredentialsDelegate * mOperationalCredentialsDelegate = nullptr;
CommissioningParameters mParams = CommissioningParameters();
OperationalDeviceProxy mOperationalDeviceProxy;
// Memory space for the commisisoning parameters that come in as ByteSpans - the caller is not guaranteed to retain this memory
uint8_t mSsid[CommissioningParameters::kMaxSsidLen];
Expand All @@ -104,6 +104,7 @@ class AutoCommissioner : public CommissioningDelegate

bool mNeedsNetworkSetup = false;
ReadCommissioningInfo mDeviceCommissioningInfo;
bool mNeedsDST = false;

// TODO: Why were the nonces statically allocated, but the certs dynamically allocated?
uint8_t * mDAC = nullptr;
Expand Down
Loading

0 comments on commit ac3535b

Please sign in to comment.