Skip to content

Commit

Permalink
Remove [commissioner] redundant TC acknowledgement flag (project-chip…
Browse files Browse the repository at this point in the history
…#36966)

* Remove [commissioner] redundant TC acknowledgement flag

The flag controlling whether to require TC acknowledgement is no longer
needed since TC acceptance is now a mandatory pre-condition for
commissioning. This flag was originally added to support delayed TC
acceptance during the commissioning process, but since that flow has
been removed, the flag serves no purpose.

The TC acknowledgement state itself is still required and maintained,
but the additional boolean flag controlling the requirement is redundant
and can be safely removed.
  • Loading branch information
swan-amazon authored Jan 14, 2025
1 parent a4f6ca8 commit 775d86b
Show file tree
Hide file tree
Showing 7 changed files with 1 addition and 47 deletions.
3 changes: 0 additions & 3 deletions examples/chip-tool/commands/pairing/PairingCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,6 @@ CommissioningParameters PairingCommand::GetCommissioningParameters()
params.SetCountryCode(CharSpan::fromCharString(mCountryCode.Value()));
}

// Default requiring TCs to false, to preserve release 1.3 chip-tool behavior
params.SetRequireTermsAndConditionsAcknowledgement(mRequireTCAcknowledgements.ValueOr(false));

// mTCAcknowledgements and mTCAcknowledgementVersion are optional, but related. When one is missing, default the value to 0, to
// increase the test tools ability to test the applications.
if (mTCAcknowledgements.HasValue() || mTCAcknowledgementVersion.HasValue())
Expand Down
6 changes: 0 additions & 6 deletions examples/chip-tool/commands/pairing/PairingCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,6 @@ class PairingCommand : public CHIPCommand,
"DSTOffset list to use when setting Time Synchronization cluster's DSTOffset attribute",
Argument::kOptional);

AddArgument("require-tc-acknowledgements", 0, 1, &mRequireTCAcknowledgements,
"Indicates whether Terms and Conditions acknowledgements are required during commissioning. If set to "
"true, the tc-acknowledgements and tc-acknowledgements-version arguments must be provided for the "
"commissioning to succeed. If false, the T&C acknowledgement step will be skipped.");

AddArgument("tc-acknowledgements", 0, UINT16_MAX, &mTCAcknowledgements,
"Bit-field value indicating which Terms and Conditions have been accepted by the user. This value is sent "
"to the device during commissioning via the General Commissioning cluster");
Expand Down Expand Up @@ -272,7 +267,6 @@ class PairingCommand : public CHIPCommand,
chip::Optional<uint64_t> mICDMonitoredSubject;
chip::Optional<chip::app::Clusters::IcdManagement::ClientTypeEnum> mICDClientType;
chip::Optional<uint32_t> mICDStayActiveDurationMsec;
chip::Optional<bool> mRequireTCAcknowledgements;
chip::Optional<uint16_t> mTCAcknowledgements;
chip::Optional<uint16_t> mTCAcknowledgementVersion;
chip::app::DataModel::List<chip::app::Clusters::TimeSynchronization::Structs::TimeZoneStruct::Type> mTimeZoneList;
Expand Down
9 changes: 1 addition & 8 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3226,20 +3226,13 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
case CommissioningStage::kConfigureTCAcknowledgments: {
ChipLogProgress(Controller, "Setting Terms and Conditions");

if (!params.GetRequireTermsAndConditionsAcknowledgement())
if (!params.GetTermsAndConditionsAcknowledgement().HasValue())
{
ChipLogProgress(Controller, "Setting Terms and Conditions: Skipped");
CommissioningStageComplete(CHIP_NO_ERROR);
return;
}

if (!params.GetTermsAndConditionsAcknowledgement().HasValue())
{
ChipLogError(Controller, "No acknowledgements provided");
CommissioningStageComplete(CHIP_ERROR_INCORRECT_STATE);
return;
}

GeneralCommissioning::Commands::SetTCAcknowledgements::Type request;
TermsAndConditionsAcknowledgement termsAndConditionsAcknowledgement = params.GetTermsAndConditionsAcknowledgement().Value();
request.TCUserResponse = termsAndConditionsAcknowledgement.acceptedTermsAndConditions;
Expand Down
9 changes: 0 additions & 9 deletions src/controller/CommissioningDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,6 @@ class CommissioningParameters
// The country code to be used for the node, if set.
Optional<CharSpan> GetCountryCode() const { return mCountryCode; }

bool GetRequireTermsAndConditionsAcknowledgement() const { return mRequireTermsAndConditionsAcknowledgement; }

Optional<TermsAndConditionsAcknowledgement> GetTermsAndConditionsAcknowledgement() const
{
return mTermsAndConditionsAcknowledgement;
Expand Down Expand Up @@ -353,12 +351,6 @@ class CommissioningParameters
return *this;
}

CommissioningParameters & SetRequireTermsAndConditionsAcknowledgement(bool requireTermsAndConditionsAcknowledgement)
{
mRequireTermsAndConditionsAcknowledgement = requireTermsAndConditionsAcknowledgement;
return *this;
}

CommissioningParameters &
SetTermsAndConditionsAcknowledgement(TermsAndConditionsAcknowledgement termsAndConditionsAcknowledgement)
{
Expand Down Expand Up @@ -670,7 +662,6 @@ class CommissioningParameters
Optional<uint32_t> mICDStayActiveDurationMsec;
ICDRegistrationStrategy mICDRegistrationStrategy = ICDRegistrationStrategy::kIgnore;
bool mCheckForMatchingFabric = false;
bool mRequireTermsAndConditionsAcknowledgement = false;
};

struct RequestedCertificate
Expand Down
8 changes: 0 additions & 8 deletions src/controller/python/ChipDeviceController-ScriptBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,6 @@ PyChipError pychip_DeviceController_OpenCommissioningWindow(chip::Controller::De
bool pychip_DeviceController_GetIPForDiscoveredDevice(chip::Controller::DeviceCommissioner * devCtrl, int idx, char * addrStr,
uint32_t len);

PyChipError pychip_DeviceController_SetRequireTermsAndConditionsAcknowledgement(bool tcRequired);

PyChipError pychip_DeviceController_SetTermsAcknowledgements(uint16_t tcVersion, uint16_t tcUserResponse);

PyChipError pychip_DeviceController_SetSkipCommissioningComplete(bool skipCommissioningComplete);
Expand Down Expand Up @@ -578,12 +576,6 @@ PyChipError pychip_DeviceController_SetDefaultNtp(const char * defaultNTP)
return ToPyChipError(CHIP_NO_ERROR);
}

PyChipError pychip_DeviceController_SetRequireTermsAndConditionsAcknowledgement(bool tcRequired)
{
sCommissioningParameters.SetRequireTermsAndConditionsAcknowledgement(tcRequired);
return ToPyChipError(CHIP_NO_ERROR);
}

PyChipError pychip_DeviceController_SetTermsAcknowledgements(uint16_t tcVersion, uint16_t tcUserResponse)
{
sCommissioningParameters.SetTermsAndConditionsAcknowledgement(
Expand Down
10 changes: 0 additions & 10 deletions src/controller/python/chip/ChipDeviceCtrl.py
Original file line number Diff line number Diff line change
Expand Up @@ -2008,9 +2008,6 @@ def _InitLib(self):
self._dmLib.pychip_DeviceController_SetSkipCommissioningComplete.restype = PyChipError
self._dmLib.pychip_DeviceController_SetSkipCommissioningComplete.argtypes = [c_bool]

self._dmLib.pychip_DeviceController_SetRequireTermsAndConditionsAcknowledgement.restype = PyChipError
self._dmLib.pychip_DeviceController_SetRequireTermsAndConditionsAcknowledgement.argtypes = [c_bool]

self._dmLib.pychip_DeviceController_SetTermsAcknowledgements.restype = PyChipError
self._dmLib.pychip_DeviceController_SetTermsAcknowledgements.argtypes = [c_uint16, c_uint16]

Expand Down Expand Up @@ -2142,13 +2139,6 @@ def SetDSTOffset(self, offset: int, validStarting: int, validUntil: int):
lambda: self._dmLib.pychip_DeviceController_SetDSTOffset(offset, validStarting, validUntil)
).raise_on_error()

def SetTCRequired(self, tcRequired: bool):
''' Set whether TC Acknowledgements should be set during commissioning'''
self.CheckIsActive()
self._ChipStack.Call(
lambda: self._dmLib.pychip_DeviceController_SetRequireTermsAndConditionsAcknowledgement(tcRequired)
).raise_on_error()

def SetTCAcknowledgements(self, tcAcceptedVersion: int, tcUserResponse: int):
''' Set the TC acknowledgements to set during commissioning'''
self.CheckIsActive()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2378,9 +2378,6 @@ async def commission_device(instance: MatterBaseTest, i) -> bool:
logging.debug(
f"Setting TC Acknowledgements to version {conf.tc_version_to_simulate} with user response {conf.tc_user_response_to_simulate}.")
dev_ctrl.SetTCAcknowledgements(conf.tc_version_to_simulate, conf.tc_user_response_to_simulate)
dev_ctrl.SetTCRequired(True)
else:
dev_ctrl.SetTCRequired(False)

if conf.commissioning_method == "on-network":
try:
Expand Down

0 comments on commit 775d86b

Please sign in to comment.