From 2eb233c1c06dbe9655cd9d83586b1af2981994d0 Mon Sep 17 00:00:00 2001 From: James Swan <122404367+swan-amazon@users.noreply.github.com> Date: Mon, 6 Jan 2025 20:14:08 +0000 Subject: [PATCH 1/5] 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/CommissioningDelegate.h | 6 ------ .../python/ChipDeviceController-ScriptBinding.cpp | 8 -------- src/controller/python/chip/ChipDeviceCtrl.py | 10 ---------- 5 files changed, 33 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/CommissioningDelegate.h b/src/controller/CommissioningDelegate.h index dcbef908c3dd4f..49b5a6710c40b8 100644 --- a/src/controller/CommissioningDelegate.h +++ b/src/controller/CommissioningDelegate.h @@ -353,12 +353,6 @@ class CommissioningParameters return *this; } - CommissioningParameters & SetRequireTermsAndConditionsAcknowledgement(bool requireTermsAndConditionsAcknowledgement) - { - mRequireTermsAndConditionsAcknowledgement = requireTermsAndConditionsAcknowledgement; - return *this; - } - CommissioningParameters & SetTermsAndConditionsAcknowledgement(TermsAndConditionsAcknowledgement termsAndConditionsAcknowledgement) { 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() From d821752cb50be32b23222cf7e85040e5c81e5c7c Mon Sep 17 00:00:00 2001 From: James Swan <122404367+swan-amazon@users.noreply.github.com> Date: Mon, 6 Jan 2025 21:17:27 +0000 Subject: [PATCH 2/5] --- src/controller/CommissioningDelegate.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/controller/CommissioningDelegate.h b/src/controller/CommissioningDelegate.h index 49b5a6710c40b8..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; @@ -664,7 +662,6 @@ class CommissioningParameters Optional mICDStayActiveDurationMsec; ICDRegistrationStrategy mICDRegistrationStrategy = ICDRegistrationStrategy::kIgnore; bool mCheckForMatchingFabric = false; - bool mRequireTermsAndConditionsAcknowledgement = false; }; struct RequestedCertificate From 19d18065d68f4de1a1882df07ea4c877499e7d38 Mon Sep 17 00:00:00 2001 From: James Swan <122404367+swan-amazon@users.noreply.github.com> Date: Mon, 6 Jan 2025 21:22:33 +0000 Subject: [PATCH 3/5] --- src/controller/CHIPDeviceController.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) 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; From 6a1b606de4fd35747ad0180fdec086335f3fb003 Mon Sep 17 00:00:00 2001 From: James Swan Date: Thu, 9 Jan 2025 17:59:04 +0000 Subject: [PATCH 4/5] From 97a073f759ade48ca544c7822f687ef5ce3ee215 Mon Sep 17 00:00:00 2001 From: James Swan Date: Tue, 14 Jan 2025 20:53:05 +0000 Subject: [PATCH 5/5] --- .../chip/testing/matter_testing.py | 3 --- 1 file changed, 3 deletions(-) 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: