From 775d86b3cf6fe6de0a420fd1953accba65bb3a1e Mon Sep 17 00:00:00 2001 From: James Swan <122404367+swan-amazon@users.noreply.github.com> Date: Tue, 14 Jan 2025 15:04:51 -0800 Subject: [PATCH] Remove [commissioner] redundant TC acknowledgement flag (#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. --- examples/chip-tool/commands/pairing/PairingCommand.cpp | 3 --- examples/chip-tool/commands/pairing/PairingCommand.h | 6 ------ src/controller/CHIPDeviceController.cpp | 9 +-------- src/controller/CommissioningDelegate.h | 9 --------- .../python/ChipDeviceController-ScriptBinding.cpp | 8 -------- src/controller/python/chip/ChipDeviceCtrl.py | 10 ---------- .../chip/testing/matter_testing.py | 3 --- 7 files changed, 1 insertion(+), 47 deletions(-) diff --git a/examples/chip-tool/commands/pairing/PairingCommand.cpp b/examples/chip-tool/commands/pairing/PairingCommand.cpp index 2862a3fa432425..96ccd6965d395c 100644 --- a/examples/chip-tool/commands/pairing/PairingCommand.cpp +++ b/examples/chip-tool/commands/pairing/PairingCommand.cpp @@ -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()) diff --git a/examples/chip-tool/commands/pairing/PairingCommand.h b/examples/chip-tool/commands/pairing/PairingCommand.h index b6903e34d53529..5572a724e918dd 100644 --- a/examples/chip-tool/commands/pairing/PairingCommand.h +++ b/examples/chip-tool/commands/pairing/PairingCommand.h @@ -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"); @@ -272,7 +267,6 @@ class PairingCommand : public CHIPCommand, chip::Optional mICDMonitoredSubject; chip::Optional mICDClientType; chip::Optional mICDStayActiveDurationMsec; - chip::Optional mRequireTCAcknowledgements; chip::Optional mTCAcknowledgements; chip::Optional mTCAcknowledgementVersion; chip::app::DataModel::List mTimeZoneList; diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index c7988103a902c6..76e82d7f21ac85 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -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; diff --git a/src/controller/CommissioningDelegate.h b/src/controller/CommissioningDelegate.h index dcbef908c3dd4f..660a3d11d128f9 100644 --- a/src/controller/CommissioningDelegate.h +++ b/src/controller/CommissioningDelegate.h @@ -174,8 +174,6 @@ class CommissioningParameters // The country code to be used for the node, if set. Optional GetCountryCode() const { return mCountryCode; } - bool GetRequireTermsAndConditionsAcknowledgement() const { return mRequireTermsAndConditionsAcknowledgement; } - Optional GetTermsAndConditionsAcknowledgement() const { return mTermsAndConditionsAcknowledgement; @@ -353,12 +351,6 @@ class CommissioningParameters return *this; } - CommissioningParameters & SetRequireTermsAndConditionsAcknowledgement(bool requireTermsAndConditionsAcknowledgement) - { - mRequireTermsAndConditionsAcknowledgement = requireTermsAndConditionsAcknowledgement; - return *this; - } - CommissioningParameters & SetTermsAndConditionsAcknowledgement(TermsAndConditionsAcknowledgement termsAndConditionsAcknowledgement) { @@ -670,7 +662,6 @@ class CommissioningParameters Optional mICDStayActiveDurationMsec; ICDRegistrationStrategy mICDRegistrationStrategy = ICDRegistrationStrategy::kIgnore; bool mCheckForMatchingFabric = false; - bool mRequireTermsAndConditionsAcknowledgement = false; }; struct RequestedCertificate diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index 536e55364674f7..b9ed46feeef15d 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -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); @@ -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( diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py index 3bc4414b54073d..250512f1c4ebe1 100644 --- a/src/controller/python/chip/ChipDeviceCtrl.py +++ b/src/controller/python/chip/ChipDeviceCtrl.py @@ -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] @@ -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() diff --git a/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py b/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py index 46c32a2e178b79..fb87d549a608a7 100644 --- a/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py +++ b/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py @@ -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: