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

Stop storing breadcrumb in persistent storage. #17769

Merged
merged 1 commit into from
Apr 27, 2022
Merged

Stop storing breadcrumb in persistent storage. #17769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion examples/chip-tool-darwin/templates/tests/tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,8 @@ function getTests()
'TestDescriptorCluster',
// TestBasicInformation needs Reboot
//'TestBasicInformation',
'TestGeneralCommissioning',
// TestGeneralCommissioning needs Reboot
//'TestGeneralCommissioning',
'TestGroupsCluster',
'TestGroupKeyManagementCluster',
'TestIdentifyCluster',
Expand Down
51 changes: 20 additions & 31 deletions src/app/clusters/general-commissioning-server/general-commissioning-server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ class GeneralCommissioningAttrAccess : public AttributeAccessInterface
GeneralCommissioningAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::Missing(), GeneralCommissioning::Id) {}

CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
CHIP_ERROR Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) override;

private:
CHIP_ERROR ReadIfSupported(CHIP_ERROR (ConfigurationManager::*getter)(uint8_t &), AttributeValueEncoder & aEncoder);
Expand Down Expand Up @@ -100,37 +99,13 @@ CHIP_ERROR GeneralCommissioningAttrAccess::Read(const ConcreteReadAttributePath
case SupportsConcurrentConnection::Id: {
return ReadSupportsConcurrentConnection(aEncoder);
}
case Breadcrumb::Id: {
return aEncoder.Encode(DeviceLayer::DeviceControlServer::DeviceControlSvr().GetBreadcrumb());
}
default: {
break;
}
}
return CHIP_NO_ERROR;
}

CHIP_ERROR GeneralCommissioningAttrAccess::Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder)
{
// TODO: There was discussion about moving the breadcrumb to the attribute store, which would make this function obsolete

if (aPath.mClusterId != GeneralCommissioning::Id)
{
// We shouldn't have been called at all.
return CHIP_ERROR_INVALID_ARGUMENT;
}

switch (aPath.mAttributeId)
{
case Attributes::Breadcrumb::Id:
Attributes::Breadcrumb::TypeInfo::DecodableType value;
ReturnErrorOnFailure(aDecoder.Decode(value));
return DeviceLayer::DeviceControlServer::DeviceControlSvr().SetBreadcrumb(value);
default:
return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
}
}

CHIP_ERROR GeneralCommissioningAttrAccess::ReadIfSupported(CHIP_ERROR (ConfigurationManager::*getter)(uint8_t &),
AttributeValueEncoder & aEncoder)
{
Expand Down Expand Up @@ -208,6 +183,8 @@ bool emberAfGeneralCommissioningClusterArmFailSafeCallback(app::CommandHandler *
{
// Force the timer to expire immediately.
failSafeContext.ForceFailSafeTimerExpiry();
// Don't set the breadcrumb, since expiring the failsafe should
// reset it anyway.
response.errorCode = CommissioningError::kOk;
commandObj->AddResponse(commandPath, response);
}
Expand All @@ -216,10 +193,10 @@ bool emberAfGeneralCommissioningClusterArmFailSafeCallback(app::CommandHandler *
CheckSuccess(
failSafeContext.ArmFailSafe(accessingFabricIndex, System::Clock::Seconds16(commandData.expiryLengthSeconds)),
Failure);
Breadcrumb::Set(commandPath.mEndpointId, commandData.breadcrumb);
response.errorCode = CommissioningError::kOk;
commandObj->AddResponse(commandPath, response);
}
DeviceLayer::DeviceControlServer::DeviceControlSvr().SetBreadcrumb(commandData.breadcrumb);
}
else
{
Expand Down Expand Up @@ -265,11 +242,11 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback(
CheckSuccess(server->CommissioningComplete(handle->AsSecureSession()->GetPeerNodeId(), handle->GetFabricIndex()),
Failure);

Breadcrumb::Set(commandPath.mEndpointId, 0);
response.errorCode = CommissioningError::kOk;
}
}

DeviceLayer::DeviceControlServer::DeviceControlSvr().SetBreadcrumb(0);
commandObj->AddResponse(commandPath, response);

return true;
Expand All @@ -282,9 +259,8 @@ bool emberAfGeneralCommissioningClusterSetRegulatoryConfigCallback(app::CommandH
MATTER_TRACE_EVENT_SCOPE("SetRegulatoryConfig", "GeneralCommissioning");
DeviceControlServer * server = &DeviceLayer::DeviceControlServer::DeviceControlSvr();

CheckSuccess(server->SetRegulatoryConfig(to_underlying(commandData.newRegulatoryConfig), commandData.countryCode,
commandData.breadcrumb),
Failure);
CheckSuccess(server->SetRegulatoryConfig(to_underlying(commandData.newRegulatoryConfig), commandData.countryCode), Failure);
Breadcrumb::Set(commandPath.mEndpointId, commandData.breadcrumb);

Commands::SetRegulatoryConfigResponse::Type response;
response.errorCode = CommissioningError::kOk;
Expand All @@ -293,8 +269,21 @@ bool emberAfGeneralCommissioningClusterSetRegulatoryConfigCallback(app::CommandH
return true;
}

namespace {
void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg)
{
if (event->Type == DeviceLayer::DeviceEventType::kFailSafeTimerExpired)
{
// Spec says to reset Breadcrumb attribute to 0.
Breadcrumb::Set(0, 0);
}
}

} // anonymous namespace

void MatterGeneralCommissioningPluginServerInitCallback()
{
DeviceLayer::DeviceControlServer::DeviceControlSvr().SetBreadcrumb(0);
Breadcrumb::Set(0, 0);
registerAttributeAccessOverride(&gAttrAccess);
DeviceLayer::PlatformMgrImpl().AddEventHandler(OnPlatformEventHandler);
}
214 changes: 214 additions & 0 deletions src/app/tests/suites/TestGeneralCommissioning.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ config:
nodeId: 0x12344321
cluster: "General Commissioning"
endpoint: 0
discriminator:
type: INT16U
defaultValue: 3840
payload:
type: CHAR_STRING
defaultValue: "MT:-24J0AFN00KA0648G00" # This value needs to be generated automatically

tests:
- label: "Wait for the commissioned device to be retrieved"
Expand Down Expand Up @@ -52,6 +58,214 @@ tests:
response:
value: 81

- label: "Reboot to reset Breadcrumb"
cluster: "SystemCommands"
command: "Reboot"

- label: "Connect to the device again"
cluster: "DelayCommands"
command: "WaitForCommissionee"
arguments:
values:
- name: "nodeId"
value: nodeId

- label: "Read back Breadcrumb after reboot and ensure it was not persisted"
command: "readAttribute"
attribute: "Breadcrumb"
response:
value: 0

- label: "Set Breadcrumb to nonzero value"
command: "writeAttribute"
attribute: "Breadcrumb"
arguments:
value: 1

- label: "Check Breadcrumb set worked"
command: "readAttribute"
attribute: "Breadcrumb"
response:
value: 1

- label: "Send CommissioningComplete without armed fail-safe"
command: "CommissioningComplete"
response:
values:
- name: errorCode
value: 3 # NoFailSafe

- label: "Check Breadcrumb was not touched by invalid CommissioningComplete"
command: "readAttribute"
attribute: "Breadcrumb"
response:
value: 1

- label: "Open Commissioning Window from alpha"
cluster: "AdministratorCommissioning"
command: "OpenBasicCommissioningWindow"
timedInteractionTimeoutMs: 10000
arguments:
values:
- name: "CommissioningTimeout"
value: 180

# Arming a fail-safe over CASE while a commissioning window is open should not work.
- label: "Try to arm fail-safe"
command: "ArmFailSafe"
arguments:
values:
- name: expiryLengthSeconds
value: 10
- name: breadcrumb
value: 5000
response:
values:
- name: errorCode
value: 4 # BusyWithOtherAdmin

- label:
"Check Breadcrumb was not touched by ArmFailSafe with commissioning
window open"
command: "readAttribute"
attribute: "Breadcrumb"
response:
value: 1

- label: "Reset Breadcrumb to 0 so we can commission"
command: "writeAttribute"
attribute: "Breadcrumb"
arguments:
value: 0

- label: "Commission from beta"
identity: "beta"
cluster: "CommissionerCommands"
command: "PairWithQRCode"
arguments:
values:
- name: "nodeId"
value: 0x12345
- name: "payload"
value: payload

- label: "Wait for the commissioned device to be retrieved for beta"
identity: "beta"
cluster: "DelayCommands"
command: "WaitForCommissionee"
arguments:
values:
- name: "nodeId"
value: 0x12345

- label: "Arm fail-safe"
command: "ArmFailSafe"
arguments:
values:
- name: expiryLengthSeconds
value: 500
- name: breadcrumb
value: 2
response:
values:
- name: errorCode
value: 0 # OK

- label: "Check Breadcrumb was properly set by ArmFailSafe"
command: "readAttribute"
attribute: "Breadcrumb"
response:
value: 2

- label: "Try to arm fail-safe from wrong fabric"
command: "ArmFailSafe"
identity: "beta"
arguments:
values:
- name: expiryLengthSeconds
value: 10
- name: breadcrumb
value: 5000
response:
values:
- name: errorCode
value: 4 # BusyWithOtherAdmin

- label:
"Check Breadcrumb was not touched by ArmFailSafe with existing
fail-safe armed"
command: "readAttribute"
attribute: "Breadcrumb"
response:
value: 2

- label: "Send CommissioningComplete from wrong fabric"
command: "CommissioningComplete"
identity: "beta"
response:
values:
- name: errorCode
value: 2 # InvalidAuthentication

- label:
"Check Breadcrumb was not touched by CommissioningComplete from wrong
fabric"
command: "readAttribute"
attribute: "Breadcrumb"
response:
value: 2

- label: "Close out the fail-safe gracefully"
command: "CommissioningComplete"
response:
values:
- name: errorCode
value: 0 # Ok

- label: "Check Breadcrumb was reset to 0 by CommissioningComplete"
command: "readAttribute"
attribute: "Breadcrumb"
response:
value: 0

- label: "Arm fail-safe again"
command: "ArmFailSafe"
arguments:
values:
- name: expiryLengthSeconds
value: 500
- name: breadcrumb
value: 3
response:
values:
- name: errorCode
value: 0 # OK

- label: "Check Breadcrumb was set by arming fail-safe again"
command: "readAttribute"
attribute: "Breadcrumb"
response:
value: 3

- label: "Force-expire the fail-safe"
command: "ArmFailSafe"
arguments:
values:
- name: expiryLengthSeconds
value: 0
- name: breadcrumb
value: 4
response:
values:
- name: errorCode
value: 0 # OK

- label: "Check Breadcrumb was reset by expiring the fail-safe"
command: "readAttribute"
attribute: "Breadcrumb"
response:
value: 0

- label: "Validate presence of SupportsConcurrentConnection"
command: "readAttribute"
attribute: "SupportsConcurrentConnection"
Expand Down
4 changes: 3 additions & 1 deletion src/darwin/Framework/CHIP/templates/tests/tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,9 @@ function getTests()
'TestDescriptorCluster',
// TestBasicInformation needs support for Reboot with no discriminator
//'TestBasicInformation',
'TestGeneralCommissioning',
// TestGeneralCommissioning does reboots that need to have side-effects, so
// can't be tested in this test framework.
//'TestGeneralCommissioning',
'TestGroupsCluster',
'TestGroupKeyManagementCluster',
'TestIdentifyCluster',
Expand Down
Loading