From 8c79a18d93644e44d3094ecda321700d1a02faf4 Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Mon, 26 Aug 2024 07:04:33 -0700 Subject: [PATCH 01/28] [HVAC] Check if number of preset scenarios exceeds maximum number of scenarios --- .../include/thermostat-delegate-impl.h | 2 + .../src/thermostat-delegate-impl.cpp | 5 +- .../thermostat-server-presets.cpp | 92 +++++++++---------- 3 files changed, 49 insertions(+), 50 deletions(-) diff --git a/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h b/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h index b57ee2492122e7..fcc164d2b78031 100644 --- a/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h +++ b/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h @@ -39,6 +39,8 @@ static constexpr uint8_t kMaxNumberOfPresetTypes = 6; // We will support only one preset of each preset type. static constexpr uint8_t kMaxNumberOfPresetsOfEachType = 1; +static constexpr uint8_t kMaxNumberOfPresetsSupported = kMaxNumberOfPresetTypes * kMaxNumberOfPresetsOfEachType - 1; + class ThermostatDelegate : public Delegate { public: diff --git a/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp b/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp index 7991e48323a62f..da58c840049275 100644 --- a/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp +++ b/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp @@ -31,7 +31,7 @@ ThermostatDelegate ThermostatDelegate::sInstance; ThermostatDelegate::ThermostatDelegate() { - mNumberOfPresets = kMaxNumberOfPresetTypes * kMaxNumberOfPresetsOfEachType; + mNumberOfPresets = kMaxNumberOfPresetsSupported; mNextFreeIndexInPresetsList = 0; mNextFreeIndexInPendingPresetsList = 0; @@ -87,6 +87,9 @@ CHIP_ERROR ThermostatDelegate::GetPresetTypeAtIndex(size_t index, PresetTypeStru { .presetScenario = PresetScenarioEnum::kVacation, .numberOfPresets = kMaxNumberOfPresetsOfEachType, .presetTypeFeatures = to_underlying(PresetTypeFeaturesBitmap::kSupportsNames) }, + { .presetScenario = PresetScenarioEnum::kUserDefined, + .numberOfPresets = kMaxNumberOfPresetsOfEachType, + .presetTypeFeatures = to_underlying(PresetTypeFeaturesBitmap::kSupportsNames) }, }; if (index < ArraySize(presetTypes)) { diff --git a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp index 9413513fd0cedd..8596d767f3e1b0 100644 --- a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp +++ b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp @@ -152,51 +152,14 @@ bool GetMatchingPresetInPresets(Delegate * delegate, const DataModel::NullableGetPendingPresetAtIndex(i, pendingPreset); - - if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) - { - break; - } - if (err != CHIP_NO_ERROR) - { - ChipLogError(Zcl, "CountNumberOfPendingPresets: GetPendingPresetAtIndex failed with error %" CHIP_ERROR_FORMAT, - err.Format()); - return 0; - } - numberOfPendingPresets++; - } - - return numberOfPendingPresets; -} - -/** - * @brief Checks if the presetScenario is present in the PresetTypes attribute. + * @brief Gets the maximum number of presets allowed for a given preset scenario. * * @param[in] delegate The delegate to use. * @param[in] presetScenario The presetScenario to match with. * - * @return true if the presetScenario is found, false otherwise. + * @return The maximum number of presets allowed for the preset scenario */ -bool PresetScenarioExistsInPresetTypes(Delegate * delegate, PresetScenarioEnum presetScenario) +uint8_t MaximumPresetScenarioCount(Delegate * delegate, PresetScenarioEnum presetScenario) { VerifyOrReturnValue(delegate != nullptr, false); @@ -206,15 +169,17 @@ bool PresetScenarioExistsInPresetTypes(Delegate * delegate, PresetScenarioEnum p auto err = delegate->GetPresetTypeAtIndex(i, presetType); if (err != CHIP_NO_ERROR) { - return false; + // Either we failed to fetch the next preset type, in which case we should error higher, + // or we exhausted the list trying to find the preset type + return 0; } if (presetType.presetScenario == presetScenario) { - return true; + return presetType.numberOfPresets; } } - return false; + return 0; } /** @@ -359,6 +324,10 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele return CHIP_IM_GLOBAL_STATUS(ConstraintError); } + // We're going to append this preset, so let's assume a count as though it had already been inserted + size_t presetCount = 1; + size_t presetScenarioCount = 1; + if (preset.GetPresetHandle().IsNull()) { if (IsBuiltIn(preset)) @@ -410,8 +379,12 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele } } - if (!PresetScenarioExistsInPresetTypes(delegate, preset.GetPresetScenario())) + size_t maximumPresetCount = delegate->GetNumberOfPresets(); + size_t maximumPresetScenarioCount = MaximumPresetScenarioCount(delegate, preset.GetPresetScenario()); + + if (maximumPresetScenarioCount == 0) { + // This is not a supported preset scenario return CHIP_IM_GLOBAL_STATUS(ConstraintError); } @@ -423,16 +396,37 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele // Before adding this preset to the pending presets, if the expected length of the pending presets' list // exceeds the total number of presets supported, return RESOURCE_EXHAUSTED. Note that the preset has not been appended yet. - uint8_t numberOfPendingPresets = CountNumberOfPendingPresets(delegate); + for (uint8_t i = 0; true; i++) + { + PresetStructWithOwnedMembers otherPreset; + CHIP_ERROR err = delegate->GetPendingPresetAtIndex(i, otherPreset); - // We will be adding one more preset, so reject if the length is already at max. - if (numberOfPendingPresets >= delegate->GetNumberOfPresets()) + if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) + { + break; + } + if (err != CHIP_NO_ERROR) + { + return CHIP_IM_GLOBAL_STATUS(InvalidInState); + } + presetCount++; + if (preset.GetPresetScenario() == otherPreset.GetPresetScenario()) + { + presetScenarioCount++; + } + } + + if (presetCount > maximumPresetCount) { + ChipLogError(Zcl, "Preset count exceeded %zu: %zu ", maximumPresetCount, presetCount); return CHIP_IM_GLOBAL_STATUS(ResourceExhausted); } - // TODO #34556 : Check if the number of presets for each presetScenario exceeds the max number of presets supported for that - // scenario. We plan to support only one preset for each presetScenario for our use cases so defer this for re-evaluation. + if (presetScenarioCount > maximumPresetScenarioCount) + { + ChipLogError(Zcl, "Preset scenario count exceeded %zu: %zu ", maximumPresetScenarioCount, presetScenarioCount); + return CHIP_IM_GLOBAL_STATUS(ResourceExhausted); + } return delegate->AppendToPendingPresetList(preset); } From 4fae149d5a1b60bc0e4b87376da710127ceb1c66 Mon Sep 17 00:00:00 2001 From: Axel Le Bourhis <45206070+axelnxp@users.noreply.github.com> Date: Wed, 28 Aug 2024 20:09:13 +0200 Subject: [PATCH 02/28] [NXP][Zephyr] Provide AP band in connection request parameters (#35181) Signed-off-by: Axel Le Bourhis --- src/platform/Zephyr/wifi/WiFiManager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/platform/Zephyr/wifi/WiFiManager.cpp b/src/platform/Zephyr/wifi/WiFiManager.cpp index 11187a7cef1655..01d0d937b19b3a 100644 --- a/src/platform/Zephyr/wifi/WiFiManager.cpp +++ b/src/platform/Zephyr/wifi/WiFiManager.cpp @@ -347,7 +347,8 @@ void WiFiManager::ScanResultHandler(Platform::UniquePtr data) Instance().mWiFiParams.mParams.security = scanResult->security <= WIFI_SECURITY_TYPE_MAX ? scanResult->security : WIFI_SECURITY_TYPE_PSK; Instance().mWiFiParams.mParams.psk_length = static_cast(Instance().mWantedNetwork.passLen); - Instance().mWiFiParams.mParams.mfp = (scanResult->mfp == WIFI_MFP_REQUIRED) ? WIFI_MFP_REQUIRED : WIFI_MFP_OPTIONAL; + Instance().mWiFiParams.mParams.mfp = scanResult->mfp; + Instance().mWiFiParams.mParams.band = scanResult->band; // If the security is none, WiFi driver expects the psk to be nullptr if (Instance().mWiFiParams.mParams.security == WIFI_SECURITY_TYPE_NONE) From 69cd03608a78d979fcb494bd0965b6b3017f800e Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 28 Aug 2024 20:10:45 +0200 Subject: [PATCH 03/28] Plumbing for CADMIN attribute updates from fabric-admin to fabric-bridge (#35222) --- .../protos/fabric_bridge_service.proto | 8 ++++ .../pigweed/rpc_services/FabricBridge.h | 6 +++ examples/fabric-admin/rpc/RpcClient.cpp | 17 +++++++ examples/fabric-admin/rpc/RpcClient.h | 11 +++++ .../include/BridgedDevice.h | 3 +- .../src/BridgedDevice.cpp | 46 +++++++++++++++++++ .../fabric-bridge-app/linux/RpcServer.cpp | 40 ++++++++++++++++ 7 files changed, 130 insertions(+), 1 deletion(-) diff --git a/examples/common/pigweed/protos/fabric_bridge_service.proto b/examples/common/pigweed/protos/fabric_bridge_service.proto index 0de3b3fa243d77..1c699e6942b358 100644 --- a/examples/common/pigweed/protos/fabric_bridge_service.proto +++ b/examples/common/pigweed/protos/fabric_bridge_service.proto @@ -26,8 +26,16 @@ message KeepActiveChanged { uint32 promised_active_duration_ms = 2; } +message AdministratorCommissioningChanged { + uint64 node_id = 1; + uint32 window_status = 2; + optional uint32 opener_fabric_index = 3; + optional uint32 opener_vendor_id = 4; +} + service FabricBridge { rpc AddSynchronizedDevice(SynchronizedDevice) returns (pw.protobuf.Empty){} rpc RemoveSynchronizedDevice(SynchronizedDevice) returns (pw.protobuf.Empty){} rpc ActiveChanged(KeepActiveChanged) returns (pw.protobuf.Empty){} + rpc AdminCommissioningAttributeChanged(AdministratorCommissioningChanged) returns (pw.protobuf.Empty){} } diff --git a/examples/common/pigweed/rpc_services/FabricBridge.h b/examples/common/pigweed/rpc_services/FabricBridge.h index aab714223968df..9bd520278c13b4 100644 --- a/examples/common/pigweed/rpc_services/FabricBridge.h +++ b/examples/common/pigweed/rpc_services/FabricBridge.h @@ -48,6 +48,12 @@ class FabricBridge : public pw_rpc::nanopb::FabricBridge::Service { return pw::Status::Unimplemented(); } + + virtual pw::Status AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChanged & request, + pw_protobuf_Empty & response) + { + return pw::Status::Unimplemented(); + } }; } // namespace rpc diff --git a/examples/fabric-admin/rpc/RpcClient.cpp b/examples/fabric-admin/rpc/RpcClient.cpp index f5672fc0bd8f85..29fc2b1ea9d73c 100644 --- a/examples/fabric-admin/rpc/RpcClient.cpp +++ b/examples/fabric-admin/rpc/RpcClient.cpp @@ -180,3 +180,20 @@ CHIP_ERROR ActiveChanged(chip::NodeId nodeId, uint32_t promisedActiveDurationMs) return WaitForResponse(call); } + +CHIP_ERROR AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChanged & data) +{ + ChipLogProgress(NotSpecified, "AdminCommissioningAttributeChanged"); + + // The RPC call is kept alive until it completes. When a response is received, it will be logged by the handler + // function and the call will complete. + auto call = fabricBridgeClient.AdminCommissioningAttributeChanged(data, RpcCompletedWithEmptyResponse); + + if (!call.active()) + { + // The RPC call was not sent. This could occur due to, for example, an invalid channel ID. Handle if necessary. + return CHIP_ERROR_INTERNAL; + } + + return WaitForResponse(call); +} diff --git a/examples/fabric-admin/rpc/RpcClient.h b/examples/fabric-admin/rpc/RpcClient.h index 8edee9a3b256ed..6dd2b5b20bbd90 100644 --- a/examples/fabric-admin/rpc/RpcClient.h +++ b/examples/fabric-admin/rpc/RpcClient.h @@ -74,3 +74,14 @@ CHIP_ERROR RemoveSynchronizedDevice(chip::NodeId nodeId); * - CHIP_ERROR_INTERNAL: An internal error occurred while activating the RPC call. */ CHIP_ERROR ActiveChanged(chip::NodeId nodeId, uint32_t promisedActiveDurationMs); + +/** + * @brief CADMIN attribute has changed of one of the bridged devices that was previously added. + * + * @param data information regarding change in AdministratorCommissioning attributes + * @return CHIP_ERROR An error code indicating the success or failure of the operation. + * - CHIP_NO_ERROR: The RPC command was successfully processed. + * - CHIP_ERROR_BUSY: Another operation is currently in progress. + * - CHIP_ERROR_INTERNAL: An internal error occurred while activating the RPC call. + */ +CHIP_ERROR AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChanged & data); diff --git a/examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDevice.h b/examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDevice.h index 7081278f4dc0f3..0de35ecbfa32e8 100644 --- a/examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDevice.h +++ b/examples/fabric-bridge-app/fabric-bridge-common/include/BridgedDevice.h @@ -68,7 +68,8 @@ class BridgedDevice [[nodiscard]] const BridgedAttributes & GetBridgedAttributes() const { return mAttributes; } void SetBridgedAttributes(const BridgedAttributes & value) { mAttributes = value; } - // TODO(#35077): Need to allow mAdminCommissioningAttributes to be set from fabric-admin. + + void SetAdminCommissioningAttributes(const AdminCommissioningAttributes & aAdminCommissioningAttributes); const AdminCommissioningAttributes & GetAdminCommissioningAttributes() const { return mAdminCommissioningAttributes; } /// Convenience method to set just the unique id of a bridged device as it diff --git a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp index 21f19189afefe6..27364976d121f5 100644 --- a/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp +++ b/examples/fabric-bridge-app/fabric-bridge-common/src/BridgedDevice.cpp @@ -21,6 +21,7 @@ #include #include +#include #include namespace { @@ -31,6 +32,14 @@ struct ActiveChangeEventWorkData uint32_t mPromisedActiveDuration; }; +struct ReportAttributeChangedWorkData +{ + chip::EndpointId mEndpointId; + bool mWindowChanged = false; + bool mFabricIndexChanged = false; + bool mVendorChanged = false; +}; + void ActiveChangeEventWork(intptr_t arg) { ActiveChangeEventWorkData * data = reinterpret_cast(arg); @@ -47,6 +56,28 @@ void ActiveChangeEventWork(intptr_t arg) chip::Platform::Delete(data); } +void ReportAttributeChangedWork(intptr_t arg) +{ + ReportAttributeChangedWorkData * data = reinterpret_cast(arg); + + if (data->mWindowChanged) + { + MatterReportingAttributeChangeCallback(data->mEndpointId, chip::app::Clusters::AdministratorCommissioning::Id, + chip::app::Clusters::AdministratorCommissioning::Attributes::WindowStatus::Id); + } + if (data->mFabricIndexChanged) + { + MatterReportingAttributeChangeCallback(data->mEndpointId, chip::app::Clusters::AdministratorCommissioning::Id, + chip::app::Clusters::AdministratorCommissioning::Attributes::AdminFabricIndex::Id); + } + if (data->mVendorChanged) + { + MatterReportingAttributeChangeCallback(data->mEndpointId, chip::app::Clusters::AdministratorCommissioning::Id, + chip::app::Clusters::AdministratorCommissioning::Attributes::AdminVendorId::Id); + } + chip::Platform::Delete(data); +} + } // namespace using namespace chip::app::Clusters::Actions; @@ -80,3 +111,18 @@ void BridgedDevice::SetReachable(bool reachable) ChipLogProgress(NotSpecified, "BridgedDevice[%s]: OFFLINE", mAttributes.uniqueId.c_str()); } } + +void BridgedDevice::SetAdminCommissioningAttributes(const AdminCommissioningAttributes & aAdminCommissioningAttributes) +{ + ReportAttributeChangedWorkData * workdata = chip::Platform::New(); + + workdata->mEndpointId = mEndpointId; + workdata->mWindowChanged = + (aAdminCommissioningAttributes.commissioningWindowStatus != mAdminCommissioningAttributes.commissioningWindowStatus); + workdata->mFabricIndexChanged = + (aAdminCommissioningAttributes.openerFabricIndex != mAdminCommissioningAttributes.openerFabricIndex); + workdata->mVendorChanged = (aAdminCommissioningAttributes.openerVendorId != mAdminCommissioningAttributes.openerVendorId); + + mAdminCommissioningAttributes = aAdminCommissioningAttributes; + chip::DeviceLayer::PlatformMgr().ScheduleWork(ReportAttributeChangedWork, reinterpret_cast(workdata)); +} diff --git a/examples/fabric-bridge-app/linux/RpcServer.cpp b/examples/fabric-bridge-app/linux/RpcServer.cpp index bae007ed484935..bc8eed5c3dd9f0 100644 --- a/examples/fabric-bridge-app/linux/RpcServer.cpp +++ b/examples/fabric-bridge-app/linux/RpcServer.cpp @@ -46,6 +46,8 @@ class FabricBridge final : public chip::rpc::FabricBridge pw::Status AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) override; pw::Status RemoveSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) override; pw::Status ActiveChanged(const chip_rpc_KeepActiveChanged & request, pw_protobuf_Empty & response) override; + pw::Status AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChanged & request, + pw_protobuf_Empty & response) override; }; pw::Status FabricBridge::AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) @@ -160,6 +162,44 @@ pw::Status FabricBridge::ActiveChanged(const chip_rpc_KeepActiveChanged & reques return pw::OkStatus(); } +pw::Status FabricBridge::AdminCommissioningAttributeChanged(const chip_rpc_AdministratorCommissioningChanged & request, + pw_protobuf_Empty & response) +{ + NodeId nodeId = request.node_id; + ChipLogProgress(NotSpecified, "Received CADMIN attribut change: " ChipLogFormatX64, ChipLogValueX64(nodeId)); + + auto * device = BridgeDeviceMgr().GetDeviceByNodeId(nodeId); + if (device == nullptr) + { + ChipLogError(NotSpecified, "Could not find bridged device associated with nodeId=0x" ChipLogFormatX64, + ChipLogValueX64(nodeId)); + return pw::Status::NotFound(); + } + + BridgedDevice::AdminCommissioningAttributes adminCommissioningAttributes; + + uint32_t max_window_status_value = + static_cast(chip::app::Clusters::AdministratorCommissioning::CommissioningWindowStatusEnum::kUnknownEnumValue); + VerifyOrReturnValue(request.window_status < max_window_status_value, pw::Status::InvalidArgument()); + adminCommissioningAttributes.commissioningWindowStatus = + static_cast(request.window_status); + if (request.has_opener_fabric_index) + { + VerifyOrReturnValue(request.opener_fabric_index >= chip::kMinValidFabricIndex, pw::Status::InvalidArgument()); + VerifyOrReturnValue(request.opener_fabric_index <= chip::kMaxValidFabricIndex, pw::Status::InvalidArgument()); + adminCommissioningAttributes.openerFabricIndex = static_cast(request.opener_fabric_index); + } + + if (request.has_opener_vendor_id) + { + VerifyOrReturnValue(request.opener_vendor_id != chip::VendorId::NotSpecified, pw::Status::InvalidArgument()); + adminCommissioningAttributes.openerVendorId = static_cast(request.opener_vendor_id); + } + + device->SetAdminCommissioningAttributes(adminCommissioningAttributes); + return pw::OkStatus(); +} + FabricBridge fabric_bridge_service; #endif // defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE From e5f99fb55613729bd7860f8634571d55708b7b43 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 28 Aug 2024 20:32:41 +0200 Subject: [PATCH 04/28] Fix TC_BRBINFO_4_1 for execution on TH (#35257) --- src/python_testing/TC_BRBINFO_4_1.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/python_testing/TC_BRBINFO_4_1.py b/src/python_testing/TC_BRBINFO_4_1.py index 5e1474adc35038..595d87912ad012 100644 --- a/src/python_testing/TC_BRBINFO_4_1.py +++ b/src/python_testing/TC_BRBINFO_4_1.py @@ -129,14 +129,16 @@ async def setup_class(self): asserts.fail('This test requires a TH_ICD_SERVER app. Specify app path with --string-arg th_icd_server_app_path:') self.kvs = f'kvs_{str(uuid.uuid4())}' - self.port = 5543 discriminator = 3850 passcode = 20202021 - app_args = f'--secured-device-port {self.port} --discriminator {discriminator} --passcode {passcode} --KVS {self.kvs} ' - cmd = f'{app} {app_args}' + cmd = [app] + cmd.extend(['--secured-device-port', str(5543)]) + cmd.extend(['--discriminator', str(discriminator)]) + cmd.extend(['--passcode', str(passcode)]) + cmd.extend(['--KVS', self.kvs]) logging.info("Starting ICD Server App") - self.app_process = subprocess.Popen(cmd, bufsize=0, shell=True) + self.app_process = subprocess.Popen(cmd) logging.info("ICD started") time.sleep(3) From 560350bb522c581d1afc16c64750999bb2e1ef8b Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 28 Aug 2024 12:03:19 -0700 Subject: [PATCH 05/28] [Fabric-Admin] Move DeviceSynchronization from pairing command to device_manager (#35260) * Move DeviceSynchronization from pairing command to device_manager * Restyled by gn --------- Co-authored-by: Restyled.io --- examples/fabric-admin/BUILD.gn | 4 ++-- examples/fabric-admin/commands/pairing/PairingCommand.cpp | 2 +- .../pairing => device_manager}/DeviceSynchronization.cpp | 0 .../pairing => device_manager}/DeviceSynchronization.h | 0 4 files changed, 3 insertions(+), 3 deletions(-) rename examples/fabric-admin/{commands/pairing => device_manager}/DeviceSynchronization.cpp (100%) rename examples/fabric-admin/{commands/pairing => device_manager}/DeviceSynchronization.h (100%) diff --git a/examples/fabric-admin/BUILD.gn b/examples/fabric-admin/BUILD.gn index 77e851a53c36d2..51e6bc60daa306 100644 --- a/examples/fabric-admin/BUILD.gn +++ b/examples/fabric-admin/BUILD.gn @@ -76,14 +76,14 @@ static_library("fabric-admin-utils") { "commands/common/StayActiveSender.cpp", "commands/common/StayActiveSender.h", "commands/fabric-sync/FabricSyncCommand.cpp", - "commands/pairing/DeviceSynchronization.cpp", - "commands/pairing/DeviceSynchronization.h", "commands/pairing/OpenCommissioningWindowCommand.cpp", "commands/pairing/OpenCommissioningWindowCommand.h", "commands/pairing/PairingCommand.cpp", "commands/pairing/ToTLVCert.cpp", "device_manager/DeviceManager.cpp", "device_manager/DeviceManager.h", + "device_manager/DeviceSynchronization.cpp", + "device_manager/DeviceSynchronization.h", ] deps = [ "${chip_root}/src/app:events" ] diff --git a/examples/fabric-admin/commands/pairing/PairingCommand.cpp b/examples/fabric-admin/commands/pairing/PairingCommand.cpp index aecd259d373d3f..9a7adfa3f910e5 100644 --- a/examples/fabric-admin/commands/pairing/PairingCommand.cpp +++ b/examples/fabric-admin/commands/pairing/PairingCommand.cpp @@ -21,9 +21,9 @@ #include #include #include -#include #include #include +#include #include #include #include diff --git a/examples/fabric-admin/commands/pairing/DeviceSynchronization.cpp b/examples/fabric-admin/device_manager/DeviceSynchronization.cpp similarity index 100% rename from examples/fabric-admin/commands/pairing/DeviceSynchronization.cpp rename to examples/fabric-admin/device_manager/DeviceSynchronization.cpp diff --git a/examples/fabric-admin/commands/pairing/DeviceSynchronization.h b/examples/fabric-admin/device_manager/DeviceSynchronization.h similarity index 100% rename from examples/fabric-admin/commands/pairing/DeviceSynchronization.h rename to examples/fabric-admin/device_manager/DeviceSynchronization.h From d174d62e2a0d36371d49aed05387efa6b063053e Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 28 Aug 2024 21:51:01 +0200 Subject: [PATCH 06/28] Add command-line argument to allow userprompt at start of ECOINFO_2_1 (#35234) --------- Co-authored-by: Restyled.io Co-authored-by: saurabhst --- src/python_testing/TC_ECOINFO_2_1.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/python_testing/TC_ECOINFO_2_1.py b/src/python_testing/TC_ECOINFO_2_1.py index 8397402983d163..5b952647ab4113 100644 --- a/src/python_testing/TC_ECOINFO_2_1.py +++ b/src/python_testing/TC_ECOINFO_2_1.py @@ -115,6 +115,11 @@ async def test_TC_ECOINFO_2_1(self): self.print_step(0, "Commissioning, already done") + pause_for_pre_condition = self.user_params.get("pause_for_pre_condition", False) + if pause_for_pre_condition: + self.wait_for_user_input( + "Paused test to allow for manufacturer to satisfy precondition where one or more bridged devices of a supported type is connected to DUT") + self.step(1) endpoint_wild_card_read = await dev_ctrl.ReadAttribute(dut_node_id, [(Clusters.EcosystemInformation.Attributes.ClusterRevision)]) list_of_endpoints = list(endpoint_wild_card_read.keys()) From 3f02786efa2a8b5d963d8caa6a04da72d82d4332 Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Wed, 28 Aug 2024 17:27:48 -0400 Subject: [PATCH 07/28] Testing fixes for TC_SWTCH from TE2 (#34984) * Testing fixes for TC_SWTCH from TE2 - all-clusters-app was not generating button position changes in some cases. This was not detected in some situations since the test cases don't always test for this. - Prompts are missing endpoint ID which makes it hard when running per-endpoint tests to know where it applies. - Some partials could fail on decode errors, causing test errors instead of fails. This PR: - Adds correct generation of positions on press/release. - Adds a way to claim endpoint tested in user prompts - Fixes failing on decode errors in partials Testing done: - TC_SWTCH still passes - Manually validated button position in multi-press test/simulation (update to TC_SWTCH needs test plan changes). Issue is in all-clusters-app for CI only. See https://github.com/CHIP-Specifications/chip-test-plans/issues/4493 * Restyled by autopep8 * Update prompt support --------- Co-authored-by: Restyled.io --- .../linux/ButtonEventsSimulator.cpp | 2 ++ .../matter_yamltests/hooks.py | 4 ++- src/python_testing/TC_SWTCH.py | 12 ++++++--- src/python_testing/matter_testing_support.py | 25 ++++++++++++++++--- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/examples/all-clusters-app/linux/ButtonEventsSimulator.cpp b/examples/all-clusters-app/linux/ButtonEventsSimulator.cpp index 53a08672fdbc07..0122eba4336340 100644 --- a/examples/all-clusters-app/linux/ButtonEventsSimulator.cpp +++ b/examples/all-clusters-app/linux/ButtonEventsSimulator.cpp @@ -235,6 +235,7 @@ void ButtonEventsSimulator::Next() break; } case ButtonEventsSimulator::State::kEmitStartOfMultiPress: { + SetButtonPosition(mEndpointId, mPressedButtonId); EmitInitialPress(mEndpointId, mPressedButtonId); if (mFeatureMap & static_cast(Clusters::Switch::Feature::kActionSwitch)) { @@ -268,6 +269,7 @@ void ButtonEventsSimulator::Next() { EmitShortRelease(mEndpointId, mPressedButtonId); } + SetButtonPosition(mEndpointId, mIdleButtonId); StartTimer(mMultiPressReleasedTimeMillis); break; } diff --git a/scripts/py_matter_yamltests/matter_yamltests/hooks.py b/scripts/py_matter_yamltests/matter_yamltests/hooks.py index 78905826f55757..ca739b8ea27633 100644 --- a/scripts/py_matter_yamltests/matter_yamltests/hooks.py +++ b/scripts/py_matter_yamltests/matter_yamltests/hooks.py @@ -221,7 +221,9 @@ async def step_manual(self): def show_prompt(self, msg: str, placeholder: Optional[str] = None, - default_value: Optional[str] = None) -> None: + default_value: Optional[str] = None, + endpoint_id: Optional[int] = None, + ) -> None: """ This method is called when the step needs to ask the user to perform some action or provide some value. """ diff --git a/src/python_testing/TC_SWTCH.py b/src/python_testing/TC_SWTCH.py index d6c4694560b16d..8f9a694db7c496 100644 --- a/src/python_testing/TC_SWTCH.py +++ b/src/python_testing/TC_SWTCH.py @@ -44,6 +44,8 @@ logger = logging.getLogger(__name__) +SIMULATED_LONG_PRESS_LENGTH_SECONDS = 2.0 + def bump_substep(step: str) -> str: """Given a string like "5a", bump it to "5b", or "6c" to "6d" """ @@ -94,7 +96,8 @@ def _send_multi_press_named_pipe_command(self, endpoint_id: int, number_of_press def _send_long_press_named_pipe_command(self, endpoint_id: int, pressed_position: int, feature_map: int): command_dict = {"Name": "SimulateLongPress", "EndpointId": endpoint_id, - "ButtonId": pressed_position, "LongPressDelayMillis": 5000, "LongPressDurationMillis": 5500, "FeatureMap": feature_map} + "ButtonId": pressed_position, "LongPressDelayMillis": int(1000 * (SIMULATED_LONG_PRESS_LENGTH_SECONDS - 0.5)), + "LongPressDurationMillis": int(1000 * SIMULATED_LONG_PRESS_LENGTH_SECONDS), "FeatureMap": feature_map} self._send_named_pipe_command(command_dict) def _send_latching_switch_named_pipe_command(self, endpoint_id: int, new_position: int): @@ -168,7 +171,9 @@ def _ask_for_release(self): prompt_msg="Release the button." ) else: - time.sleep(self.keep_pressed_delay/1000) + # This will await for the events to be generated properly. Note that there is a bit of a + # race here for the button simulator, but this race is extremely unlikely to be lost. + time.sleep(SIMULATED_LONG_PRESS_LENGTH_SECONDS + 0.5) def _await_sequence_of_events(self, event_queue: queue.Queue, endpoint_id: int, sequence: list[ClusterObjects.ClusterEvent], timeout_sec: float): start_time = time.time() @@ -373,7 +378,6 @@ async def test_TC_SWTCH_2_3(self): self.step(5) # We're using a long press here with a very long duration (in computer-land). This will let us check the intermediate values. # This is 1s larger than the subscription ceiling - self.keep_pressed_delay = 6000 self.pressed_position = 1 self._ask_for_keep_pressed(endpoint_id, self.pressed_position, feature_map) event_listener.wait_for_event_report(cluster.Events.InitialPress) @@ -595,7 +599,7 @@ def steps_TC_SWTCH_2_5(self): @staticmethod def should_run_SWTCH_2_5(wildcard, endpoint): msm = has_feature(Clusters.Switch, Clusters.Switch.Bitmaps.Feature.kMomentarySwitchMultiPress) - asf = has_feature(Clusters.Switch, 0x20) + asf = has_feature(Clusters.Switch, Clusters.Switch.Bitmaps.Feature.kActionSwitch) return msm(wildcard, endpoint) and not asf(wildcard, endpoint) @per_endpoint_test(should_run_SWTCH_2_5) diff --git a/src/python_testing/matter_testing_support.py b/src/python_testing/matter_testing_support.py index 4127ead64c3083..20c33a0d36b785 100644 --- a/src/python_testing/matter_testing_support.py +++ b/src/python_testing/matter_testing_support.py @@ -1380,11 +1380,21 @@ def wait_for_user_input(self, Returns: str: User input or none if input is closed. """ + + # TODO(#31928): Remove any assumptions of test params for endpoint ID. + + # Get the endpoint user param instead of `--endpoint-id` result, if available, temporarily. + endpoint_id = self.user_params.get("endpoint", None) + if endpoint_id is None or not isinstance(endpoint_id, int): + endpoint_id = self.matter_test_config.endpoint + if self.runner_hook: + # TODO(#31928): Add endpoint support to hooks. self.runner_hook.show_prompt(msg=prompt_msg, placeholder=prompt_msg_placeholder, default_value=default_value) - logging.info("========= USER PROMPT =========") + + logging.info(f"========= USER PROMPT for Endpoint {endpoint_id} =========") logging.info(f">>> {prompt_msg.rstrip()} (press enter to confirm)") try: return input() @@ -1881,6 +1891,9 @@ def _has_attribute(wildcard, endpoint, attribute: ClusterObjects.ClusterAttribut cluster = get_cluster_from_attribute(attribute) try: attr_list = wildcard.attributes[endpoint][cluster][cluster.Attributes.AttributeList] + if not isinstance(attr_list, list): + asserts.fail( + f"Failed to read mandatory AttributeList attribute value for cluster {cluster} on endpoint {endpoint}: {attr_list}.") return attribute.attribute_id in attr_list except KeyError: return False @@ -1910,9 +1923,13 @@ def has_attribute(attribute: ClusterObjects.ClusterAttributeDescriptor) -> Endpo return partial(_has_attribute, attribute=attribute) -def _has_feature(wildcard, endpoint, cluster: ClusterObjects.ClusterObjectDescriptor, feature: IntFlag) -> bool: +def _has_feature(wildcard, endpoint: int, cluster: ClusterObjects.ClusterObjectDescriptor, feature: IntFlag) -> bool: try: feature_map = wildcard.attributes[endpoint][cluster][cluster.Attributes.FeatureMap] + if not isinstance(feature_map, int): + asserts.fail( + f"Failed to read mandatory FeatureMap attribute value for cluster {cluster} on endpoint {endpoint}: {feature_map}.") + return (feature & feature_map) != 0 except KeyError: return False @@ -1926,8 +1943,8 @@ def has_feature(cluster: ClusterObjects.ClusterObjectDescriptor, feature: IntFla EP0: cluster A, B, C EP1: cluster D with feature F0 - EP2, cluster D with feature F0 - EP3, cluster D without feature F0 + EP2: cluster D with feature F0 + EP3: cluster D without feature F0 And the following test specification: @per_endpoint_test(has_feature(Clusters.D.Bitmaps.Feature.F0)) From 69cfdf5927cf0816e163320d6d148275dfd21183 Mon Sep 17 00:00:00 2001 From: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com> Date: Wed, 28 Aug 2024 14:38:05 -0700 Subject: [PATCH 08/28] =?UTF-8?q?Add=20test=20cases=20for=20testing=20addi?= =?UTF-8?q?tional=20Presets=20write=20and=20commit=20constr=E2=80=A6=20(#3?= =?UTF-8?q?5141)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add test cases for testing additional Presets write and commit constraints - Add a test for adding a preset with a preset scenario not present in PresetTypes - Add a test for testing addition of presets such that the total number of presets added is greater than the total number of presets supported * Add rollback after test step 18 * Modify the number of presets supported test case to read the number of presets supported and build a preset list whose size exceeds that to test * Modify the number of presets supported test case to read the number of presets supported and build a preset list whose size exceeds that to test * Update thermostat-delegate-impl.h * Address review comments * Add support to check for numberOfPresets supported for each preset type and build the presets list with multiple presets of each type * Restyled by autopep8 * Fix log line formatting * Update src/python_testing/TC_TSTAT_4_2.py Co-authored-by: Boris Zbarsky * Fix test step 17 to find a preset scenario in PresetScenarioEnum that is not present in PresetTypes to run the test - Fix test step 18 to build a presets list that exceeds the number of presets supported correctly * Restyled by autopep8 * Fix lint errors * Add a while loop to add more presets until max is reached --------- Co-authored-by: Restyled.io Co-authored-by: Boris Zbarsky --- .../include/thermostat-delegate-impl.h | 2 + src/python_testing/TC_TSTAT_4_2.py | 85 ++++++++++++++++++- 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h b/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h index fcc164d2b78031..7726fc337866bb 100644 --- a/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h +++ b/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h @@ -39,6 +39,8 @@ static constexpr uint8_t kMaxNumberOfPresetTypes = 6; // We will support only one preset of each preset type. static constexpr uint8_t kMaxNumberOfPresetsOfEachType = 1; +// For testing the use case where number of presets added exceeds the number of presets supported, we will have the value of +// kMaxNumberOfPresetsSupported < kMaxNumberOfPresetTypes * kMaxNumberOfPresetsOfEachType static constexpr uint8_t kMaxNumberOfPresetsSupported = kMaxNumberOfPresetTypes * kMaxNumberOfPresetsOfEachType - 1; class ThermostatDelegate : public Delegate diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index b133b1be2c8f22..641c827b388882 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -210,6 +210,10 @@ def steps_TC_TSTAT_4_2(self) -> list[TestStep]: "Verify that the write request is rejected"), TestStep("16", "TH starts an atomic write, and before it's complete, TH2 removes TH's fabric; TH2 then opens an atomic write", "Verify that the atomic request is successful"), + TestStep("17", "TH writes to the Presets attribute with a preset that has a presetScenario not present in PresetTypes attribute", + "Verify that the write request returned CONSTRAINT_ERROR (0x87)."), + TestStep("18", "TH writes to the Presets attribute such that the total number of presets is greater than the number of presets supported", + "Verify that the write request returned RESOURCE_EXHAUSTED (0x89)."), ] return steps @@ -469,7 +473,86 @@ async def test_TC_TSTAT_4_2(self): # Roll back await self.send_atomic_request_rollback_command() - # TODO: Add tests for the total number of Presets exceeds the NumberOfPresets supported. Also Add tests for adding presets with preset scenario not present in PresetTypes. + self.step("17") + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): + + # Read the PresetTypes to get the preset scenarios supported by the Thermostat. + presetTypes = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.PresetTypes) + + scenarioNotPresent = None + + # Find a preset scenario not present in PresetTypes to run this test. + for scenario in cluster.Enums.PresetScenarioEnum: + foundMatchingScenario = False + for presetType in presetTypes: + if presetType.presetScenario == scenario: + foundMatchingScenario = True + break + if not foundMatchingScenario: + scenarioNotPresent = scenario + break + + if scenarioNotPresent is None: + logger.info( + "Couldn't run test step 17 since all preset types in PresetScenarioEnum are supported by this Thermostat") + else: + test_presets = new_presets_with_handle.copy() + test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=scenarioNotPresent, + name="Preset", coolingSetpoint=2500, heatingSetpoint=1700, builtIn=False)) + + # Send the AtomicRequest begin command + await self.send_atomic_request_begin_command() + + await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) + + # Clear state for next test. + await self.send_atomic_request_rollback_command() + + self.step("18") + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): + + # Read the numberOfPresets supported. + numberOfPresetsSupported = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.NumberOfPresets) + + # Read the PresetTypes to get the preset scenarios to build the Presets list. + presetTypes = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.PresetTypes) + + # Read the Presets to copy the existing presets into our testPresets list below. + presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) + + # Calculate the length of the Presets list that could be created using the preset scenarios in PresetTypes and numberOfPresets supported for each scenario. + totalExpectedPresetsLength = 0 + for presetType in presetTypes: + totalExpectedPresetsLength += presetType.numberOfPresets + + if totalExpectedPresetsLength > numberOfPresetsSupported: + testPresets = [] + for presetType in presetTypes: + scenario = presetType.presetScenario + + # For each supported scenario, copy all the existing presets that match it, then add more presets + # until we hit the cap on the number of presets for that scenario. + presetsAddedForScenario = 0 + for preset in presets: + if scenario == preset.presetScenario: + testPresets.append(preset) + presetsAddedForScenario = presetsAddedForScenario + 1 + + while presetsAddedForScenario < presetType.numberOfPresets: + testPresets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=scenario, + name="Preset", coolingSetpoint=2500, heatingSetpoint=1700, builtIn=False)) + presetsAddedForScenario = presetsAddedForScenario + 1 + + # Send the AtomicRequest begin command + await self.send_atomic_request_begin_command() + + await self.write_presets(endpoint=endpoint, presets=testPresets, expected_status=Status.ResourceExhausted) + + # Clear state for next test. + await self.send_atomic_request_rollback_command() + else: + logger.info( + "Couldn't run test step 18 since there are not enough preset types to build a Presets list that exceeds the number of presets supported") if __name__ == "__main__": From 97ad41432fcb7def1012d307cda46612e3c44387 Mon Sep 17 00:00:00 2001 From: Thomas Lea <35579828+tleacmcsa@users.noreply.github.com> Date: Wed, 28 Aug 2024 16:51:47 -0500 Subject: [PATCH 09/28] Allow TestAccessControl to run with ARL (#35262) * Allow TestAccessControl to run with ARL Since RequestType is required, set each test data entry to some value that will pass AccessRestrictionProvider checks (since the focus is on AccessControl for these tests). * Copy the test data's request path and optionally add RequestType --- src/access/tests/TestAccessControl.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/access/tests/TestAccessControl.cpp b/src/access/tests/TestAccessControl.cpp index 400ea04b76e535..00b95baf39164b 100644 --- a/src/access/tests/TestAccessControl.cpp +++ b/src/access/tests/TestAccessControl.cpp @@ -1752,7 +1752,11 @@ TEST_F(TestAccessControl, TestCheck) for (const auto & checkData : checkData1) { CHIP_ERROR expectedResult = checkData.allow ? CHIP_NO_ERROR : CHIP_ERROR_ACCESS_DENIED; - EXPECT_EQ(accessControl.Check(checkData.subjectDescriptor, checkData.requestPath, checkData.privilege), expectedResult); + auto requestPath = checkData.requestPath; +#if CHIP_CONFIG_USE_ACCESS_RESTRICTIONS + requestPath.requestType = Access::RequestType::kAttributeReadRequest; +#endif + EXPECT_EQ(accessControl.Check(checkData.subjectDescriptor, requestPath, checkData.privilege), expectedResult); } } From 1631e3b73d849796b35f60c93ac146aa9881d791 Mon Sep 17 00:00:00 2001 From: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com> Date: Thu, 29 Aug 2024 10:24:02 +1200 Subject: [PATCH 10/28] Make zap_downloadl.py create a usable zap.app on Mac (#35242) Use the unzip utility on Mac for unzipping instead of zipfile. In addition to not supporting file modes (which the script already works around) the zipfile module also doesn't support symlinks. The embedded frameworks inside zap.app rely on symlinks for the application to work. --- scripts/tools/zap/zap_download.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/scripts/tools/zap/zap_download.py b/scripts/tools/zap/zap_download.py index 72dea27048a0c6..9d1a553feac0e8 100755 --- a/scripts/tools/zap/zap_download.py +++ b/scripts/tools/zap/zap_download.py @@ -23,6 +23,7 @@ import shutil import subprocess import sys +import tempfile import zipfile from typing import Optional @@ -123,13 +124,22 @@ def _SetupReleaseZap(install_directory: str, zap_version: str): logging.info("Fetching: %s", url) r = requests.get(url, stream=True) - z = zipfile.ZipFile(io.BytesIO(r.content)) - - logging.info("Data downloaded, extracting ...") - # extractall() does not preserve permissions (https://github.com/python/cpython/issues/59999) - for entry in z.filelist: - path = z.extract(entry, install_directory) - os.chmod(path, (entry.external_attr >> 16) & 0o777) + if zap_platform == 'mac': + # zipfile does not support symlinks (https://github.com/python/cpython/issues/82102), + # making a zap.app extracted with it unusable due to embedded frameworks. + with tempfile.NamedTemporaryFile(suffix='.zip') as tf: + for chunk in r.iter_content(chunk_size=4096): + tf.write(chunk) + tf.flush() + os.makedirs(install_directory, exist_ok=True) + _ExecuteProcess(['/usr/bin/unzip', '-oq', tf.name], install_directory) + else: + z = zipfile.ZipFile(io.BytesIO(r.content)) + logging.info("Data downloaded, extracting ...") + # extractall() does not preserve permissions (https://github.com/python/cpython/issues/59999) + for entry in z.filelist: + path = z.extract(entry, install_directory) + os.chmod(path, (entry.external_attr >> 16) & 0o777) logging.info("Done extracting.") From 7d02d7cd416cb683ca46b328943157b87e0197cd Mon Sep 17 00:00:00 2001 From: marchemi <56955785+marchemi@users.noreply.github.com> Date: Thu, 29 Aug 2024 01:04:02 +0200 Subject: [PATCH 11/28] TBRM Tests scripts consistency with te2 fixes (#35153) * Add files via upload Add yaml test script for TBRM * Update TEST_TC_TBRM_2.2.yaml * Update TEST_TC_TBRM_2.3.yaml * Update TEST_TC_TBRM_2.4.yaml * Test script consitancy wit test plan after TE2 * Test script consitancy wit test plan after TE2 * Update src/app/tests/suites/certification/Test_TC_TBRM_2_3.yaml Co-authored-by: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com> * Update src/app/tests/suites/certification/Test_TC_TBRM_2_2.yaml Co-authored-by: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com> * Restyled by whitespace * Restyled by prettier-yaml * Test_TC_TBRM_2_4. synchronisation with TC-THNETDIR-2.3 according test Plan * Restyled by whitespace * Test tweaks to get CI to pass - Use pairing payload matching the other parameters - Check response of ArmFailSafe commands - Fix bad merge in commissioner_commands.py * Restyled by prettier-yaml --------- Co-authored-by: StephaneGUELEC <94045077+StephaneGUELEC@users.noreply.github.com> Co-authored-by: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com> Co-authored-by: Restyled.io Co-authored-by: Karsten Sperling --- .../certification/Test_TC_TBRM_2_2.yaml | 22 ++++- .../certification/Test_TC_TBRM_2_3.yaml | 29 +++++-- .../certification/Test_TC_TBRM_2_4.yaml | 86 +++++++++++++++++++ .../Test_TC_TBRM_3_1_Simulated.yaml | 44 ++++++++++ 4 files changed, 170 insertions(+), 11 deletions(-) create mode 100644 src/app/tests/suites/certification/Test_TC_TBRM_2_4.yaml create mode 100644 src/app/tests/suites/certification/Test_TC_TBRM_3_1_Simulated.yaml diff --git a/src/app/tests/suites/certification/Test_TC_TBRM_2_2.yaml b/src/app/tests/suites/certification/Test_TC_TBRM_2_2.yaml index 852e18a73adc8a..a6b54026729c8f 100644 --- a/src/app/tests/suites/certification/Test_TC_TBRM_2_2.yaml +++ b/src/app/tests/suites/certification/Test_TC_TBRM_2_2.yaml @@ -41,19 +41,21 @@ tests: values: - name: nodeId value: nodeId - + # Step 1 - label: "TH reads the ActiveDatasetTimestamp attribute from the DUT" command: readAttribute attribute: ActiveDatasetTimestamp response: value: null + # Step 2 - label: "TH reads the PendingDatasetTimestamp attribute from the DUT" command: readAttribute attribute: PendingDatasetTimestamp response: value: null + # Step 3 - label: "TH sends a valid ActiveDatasetRequest command to the DUT without having armed the fail-safe" @@ -65,6 +67,7 @@ tests: response: error: FAILSAFE_REQUIRED + # Step 4 - label: "TH sends ArmFailSafe command to the DUT" cluster: General Commissioning command: ArmFailSafe @@ -75,7 +78,12 @@ tests: value: 60 - name: Breadcrumb value: 1 + response: + values: + - name: ErrorCode + value: CommissioningErrorEnum.OK + # Step 5 - label: "TH sends an invalid ActiveDatasetRequest command to the DUT" command: SetActiveDatasetRequest arguments: @@ -85,6 +93,7 @@ tests: response: error: INVALID_COMMAND + # Step 6 - label: "TH sends a valid ActiveDatasetRequest command to the DUT" command: SetActiveDatasetRequest arguments: @@ -92,12 +101,20 @@ tests: - name: ActiveDataset value: PIXIT.TBRM.THREAD_ACTIVE_DATASET + # Step 7 + - label: "TH sends CommissioningComplete command to the DUT" + cluster: General Commissioning + command: CommissioningComplete + endpoint: 0 + + # Step 8 - label: "TH reads the InterfaceEnabled attribute from the DUT" command: readAttribute attribute: InterfaceEnabled response: value: true + # Step 9 - label: "TH reads the ActiveDatasetTimestamp attribute from the DUT" command: readAttribute attribute: ActiveDatasetTimestamp @@ -106,7 +123,8 @@ tests: constraints: type: int64u - - label: "TH sends a valid GetActiveDatasetRequest command to the DUT" + # Step 10 + - label: "TH send a GetActiveDatasetRequest command to the DUT" command: GetActiveDatasetRequest response: values: diff --git a/src/app/tests/suites/certification/Test_TC_TBRM_2_3.yaml b/src/app/tests/suites/certification/Test_TC_TBRM_2_3.yaml index 7a55191f471f0b..a0ecd11693ef8d 100644 --- a/src/app/tests/suites/certification/Test_TC_TBRM_2_3.yaml +++ b/src/app/tests/suites/certification/Test_TC_TBRM_2_3.yaml @@ -49,7 +49,7 @@ tests: constraints: type: int64u - - label: "If the ActiveDatasetTimestamp attribute not null, go to step 4" + - label: "If the ActiveDatasetTimestamp attribute not null, go to step 5" cluster: EqualityCommands command: UnsignedNumberEquals arguments: @@ -75,6 +75,10 @@ tests: value: 60 - name: Breadcrumb value: 1 + response: + values: + - name: ErrorCode + value: CommissioningErrorEnum.OK # Step 3 - label: "TH sends a valid ActiveDatasetRequest command to the DUT" @@ -86,6 +90,13 @@ tests: value: PIXIT.TBRM.THREAD_ACTIVE_DATASET # Step 4 + - label: "TH sends CommissioningComplete command to the DUT" + runIf: noActiveDataset + cluster: General Commissioning + command: CommissioningComplete + endpoint: 0 + + # Step 5 - label: "TH reads the PendingDatasetTimestamp attribute from the DUT" command: readAttribute attribute: PendingDatasetTimestamp @@ -94,7 +105,7 @@ tests: constraints: type: int64u - # Step 5 + # Step 6 - label: "TH sends a SetPendingDatasetRequest command to the DUT" command: SetPendingDatasetRequest arguments: @@ -102,7 +113,7 @@ tests: - name: PendingDataset value: PIXIT.TBRM.THREAD_PENDING_DATASET - # Step 6 + # Step 7 - label: "TH sends a GetPendingDatasetRequest command to the DUT" command: GetPendingDatasetRequest response: @@ -112,7 +123,7 @@ tests: type: octet_string # TODO: This should be PIXIT.TBRM.THREAD_PENDING_DATASET but ignoring the Delay Timer element if present - # Step 7 + # Step 8 - label: "TH reads the PendingDatasetTimestamp attribute from the DUT" command: readAttribute attribute: PendingDatasetTimestamp @@ -121,7 +132,7 @@ tests: type: int64u notValue: initialPendingTimestamp - # Step 8 + # Step 9 - label: "TH subscribes to the ActiveDatasetTimestamp attribute from the DUT" command: subscribeAttribute @@ -141,15 +152,15 @@ tests: constraints: hasValue: true - # Step 9 + # Step 10 - label: "TH reads the PendingDatasetTimestamp attribute from the DUT" command: readAttribute attribute: PendingDatasetTimestamp response: value: null - # Step 10 - - label: "TH sends a valid GetActiveDatasetRequest command to the DUT" + # Step 11 + - label: "TH sends a GetActiveDatasetRequest command to the DUT" command: GetActiveDatasetRequest response: values: @@ -158,7 +169,7 @@ tests: constraints: type: octet_string - # Step 11 + # Step 12 - label: "TH reads the InterfaceEnabled attribute from the DUT" command: readAttribute attribute: InterfaceEnabled diff --git a/src/app/tests/suites/certification/Test_TC_TBRM_2_4.yaml b/src/app/tests/suites/certification/Test_TC_TBRM_2_4.yaml new file mode 100644 index 00000000000000..d1483c1c5a930a --- /dev/null +++ b/src/app/tests/suites/certification/Test_TC_TBRM_2_4.yaml @@ -0,0 +1,86 @@ +# Copyright (c) 2024 Project CHIP Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: + "[TC-TBRM-2.4] Verify that getting Active or Pending Dataset in the PASE + session results in unsupported access" + +PICS: + - TBRM.S + +config: + nodeId: 0x12344321 + + payload: "MT:-24J0AFN00KA0648G00" + discriminator: 3840 + PakeVerifier: + type: octet_string + defaultValue: "hex:b96170aae803346884724fe9a3b287c30330c2a660375d17bb205a8cf1aecb350457f8ab79ee253ab6a8e46bb09e543ae422736de501e3db37d441fe344920d09548e4c18240630c4ff4913c53513839b7c07fcc0627a1b8573a149fcd1fa466cf" + +tests: + - label: "Wait for the commissioned device to be retrieved" + cluster: DelayCommands + command: WaitForCommissionee + arguments: + values: + - name: nodeId + value: nodeId + + - label: "Open Commissioning Window" + endpoint: 0 + cluster: Administrator Commissioning + command: OpenCommissioningWindow + timedInteractionTimeoutMs: 2000 + arguments: + values: + - name: CommissioningTimeout + value: 180 + - name: PAKEPasscodeVerifier + value: PakeVerifier + - name: Discriminator + value: discriminator + - name: Iterations + value: 1000 + - name: Salt + value: "SPAKE2P Key Salt" + + - label: "TH2 establishes a PASE session with the DUT" + identity: beta + cluster: CommissionerCommands + endpoint: 0 + command: EstablishPASESession + arguments: + values: + - name: nodeId + value: nodeId + - name: payload + value: payload + + - label: + "TH2 send GetActiveDatasetRequest command to the DUT in PASE session" + identity: beta + cluster: Thread Border Router Management + endpoint: 1 + command: GetActiveDatasetRequest + response: + error: UNSUPPORTED_ACCESS + + - label: + "TH2 send GetPendingDatasetRequest command to the DUT in PASE session" + identity: beta + cluster: Thread Border Router Management + endpoint: 1 + command: GetPendingDatasetRequest + response: + error: UNSUPPORTED_ACCESS diff --git a/src/app/tests/suites/certification/Test_TC_TBRM_3_1_Simulated.yaml b/src/app/tests/suites/certification/Test_TC_TBRM_3_1_Simulated.yaml new file mode 100644 index 00000000000000..095711a0fcee95 --- /dev/null +++ b/src/app/tests/suites/certification/Test_TC_TBRM_3_1_Simulated.yaml @@ -0,0 +1,44 @@ +# Copyright (c) 2024 Project CHIP Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: "[TC-TBRM-3.1] Functionality with client as DUT" + +PICS: + - TBRM.C + +config: + nodeId: 0x12344321 + cluster: Thread Border Router Management + endpoint: 1 + +tests: + # Step 1 + - label: "DUT send SetActiveDatasetRequest to TH" + PICS: TBRM.C.C03.Tx + wait: "SetActiveDatasetRequest" + + # Step 2 + - label: "DUT send SetPendingDatasetRequest to TH" + PICS: TBRM.C.C04.Tx + wait: "SetPendingDatasetRequest" + + # Step 3 + - label: "DUT send GetActiveDatasetRequest to TH" + PICS: TBRM.C.C00.Tx + wait: "GetActiveDatasetRequest" + + # Step 4 + - label: "DUT send GetPendingDatasetRequest to TH" + PICS: TBRM.C.C01.Tx + wait: "GetPendingDatasetRequest" From 9565641bcd1720f81f31deb92d54abff23e0112b Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Thu, 29 Aug 2024 08:50:32 -0700 Subject: [PATCH 12/28] [HVAC] Alter Thermostat Preset tests to not rely on knowledge of the server's initial state --- src/python_testing/TC_TSTAT_4_2.py | 422 ++++++++++++++++++----------- 1 file changed, 265 insertions(+), 157 deletions(-) diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index 641c827b388882..e4217a27e33601 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -28,6 +28,7 @@ import copy import logging +import random import chip.clusters as Clusters from chip import ChipDeviceCtrl # Needed before chip.FabricAdmin @@ -42,21 +43,6 @@ cluster = Clusters.Thermostat -initial_presets = [] -initial_presets.append(cluster.Structs.PresetStruct(presetHandle=b'\x01', presetScenario=cluster.Enums.PresetScenarioEnum.kOccupied, - name=None, coolingSetpoint=2500, heatingSetpoint=2100, builtIn=True)) -initial_presets.append(cluster.Structs.PresetStruct(presetHandle=b'\x02', presetScenario=cluster.Enums.PresetScenarioEnum.kUnoccupied, - name=None, coolingSetpoint=2600, heatingSetpoint=2000, builtIn=True)) - -new_presets = initial_presets.copy() -new_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=cluster.Enums.PresetScenarioEnum.kSleep, - name="Sleep", coolingSetpoint=2700, heatingSetpoint=1900, builtIn=False)) - -new_presets_with_handle = initial_presets.copy() -new_presets_with_handle.append(cluster.Structs.PresetStruct( - presetHandle=b'\x03', presetScenario=cluster.Enums.PresetScenarioEnum.kSleep, name="Sleep", coolingSetpoint=2700, heatingSetpoint=1900, builtIn=False)) - - class TC_TSTAT_4_2(MatterBaseTest): def check_atomic_response(self, response: object, expected_status: Status = Status.Success, @@ -87,6 +73,34 @@ def check_atomic_response(self, response: object, expected_status: Status = Stat "Schedules attribute should have the right status") asserts.assert_true(found_preset_status, "Preset attribute should have a status") + def check_saved_presets(self, sent_presets: list, returned_presets: list): + asserts.assert_true(len(sent_presets) == len(returned_presets), "Returned presets are a different length than sent presets") + for i, sent_preset in enumerate(sent_presets): + returned_preset = returned_presets[i] + if sent_preset.presetHandle is NullValue: + sent_preset = copy.copy(sent_preset) + sent_preset.presetHandle = returned_preset.presetHandle + if sent_preset.builtIn is NullValue: + sent_preset.builtIn = returned_preset.builtIn + asserts.assert_equal(sent_preset, returned_preset, + "Returned preset is not the same as sent preset") + + def count_preset_scenarios(self, presets: list): + presetScenarios = {} + for preset in presets: + if not preset.presetScenario in presetScenarios: + presetScenarios[preset.presetScenario] = 1 + else: + presetScenarios[preset.presetScenario] += 1 + return presetScenarios + + def get_available_scenario(self, presetTypes: list, presetScenarioCounts: map): + availableScenarios = list(presetType.presetScenario for presetType in presetTypes if presetScenarioCounts.get( + presetType.presetScenario, 0) < presetType.numberOfPresets) + if len(availableScenarios) > 0: + return availableScenarios[0] + return None + async def write_presets(self, endpoint, presets, @@ -237,126 +251,203 @@ async def test_TC_TSTAT_4_2(self): nodeId=self.dut_node_id, setupPinCode=params.setupPinCode, filterType=ChipDeviceCtrl.DiscoveryFilterType.LONG_DISCRIMINATOR, filter=1234) + current_presets = [] + presetTypes = [] + presetScenarioCounts = {} + numberOfPresetsSupported = 0 + minHeatSetpointLimit = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.MinHeatSetpointLimit) + maxHeatSetpointLimit = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.MaxHeatSetpointLimit) + minCoolSetpointLimit = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.MinCoolSetpointLimit) + maxCoolSetpointLimit = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.MaxCoolSetpointLimit) + self.step("2") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050")): - presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) - logger.info(f"Rx'd Presets: {presets}") - asserts.assert_equal(presets, initial_presets, "Presets do not match initial value") + + # Read the numberOfPresets supported. + numberOfPresetsSupported = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.NumberOfPresets) + + # Read the PresetTypes to get the preset scenarios supported by the Thermostat. + presetTypes = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.PresetTypes) + logger.info(f"Rx'd Preset Types: {presetTypes}") + + current_presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) + logger.info(f"Rx'd Presets: {current_presets}") + + presetScenarioCounts = self.count_preset_scenarios(current_presets) # Write to the presets attribute without calling AtomicRequest command - await self.write_presets(endpoint=endpoint, presets=new_presets, expected_status=Status.InvalidInState) + await self.write_presets(endpoint=endpoint, presets=current_presets, expected_status=Status.InvalidInState) self.step("3") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - await self.send_atomic_request_begin_command() - # Set the new preset to a null built-in value; will be replaced with false on reading - test_presets = copy.deepcopy(new_presets) - test_presets[2].builtIn = NullValue + availableScenario = self.get_available_scenario(presetTypes=presetTypes, presetScenarioCounts=presetScenarioCounts) - # Write to the presets attribute after calling AtomicRequest command - status = await self.write_presets(endpoint=endpoint, presets=test_presets) - status_ok = (status == Status.Success) - asserts.assert_true(status_ok, "Presets write did not return Success as expected") + if availableScenario is not None and len(current_presets) < numberOfPresetsSupported: - # Read the presets attribute and verify it was updated by the write - presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) - logger.info(f"Rx'd Presets: {presets}") - asserts.assert_equal(presets, new_presets_with_handle, "Presets were updated, as expected") + # Set the preset builtIn fields to a null built-in value + test_presets = copy.deepcopy(current_presets) + for preset in test_presets: + preset.builtIn = NullValue - await self.send_atomic_request_rollback_command() + test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=availableScenario, + coolingSetpoint=2700, heatingSetpoint=1900, builtIn=False)) + + await self.send_atomic_request_begin_command() + + # Write to the presets attribute after calling AtomicRequest command + status = await self.write_presets(endpoint=endpoint, presets=test_presets) + status_ok = (status == Status.Success) + asserts.assert_true(status_ok, "Presets write did not return Success as expected") + + # Read the presets attribute and verify it was updated by the write + saved_presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) + logger.info(f"Rx'd Presets: {saved_presets}") + self.check_saved_presets(test_presets, saved_presets) + + await self.send_atomic_request_rollback_command() - # Read the presets attribute and verify it has been properly rolled back - presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) - asserts.assert_equal(presets, initial_presets, "Presets were updated which is not expected") + # Read the presets attribute and verify it has been properly rolled back + presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) + asserts.assert_equal(presets, current_presets, "Presets were updated which is not expected") + else: + logger.info( + "Couldn't run test step 3 since there was no available preset scenario to append") self.step("4") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - # Send the AtomicRequest begin command - await self.send_atomic_request_begin_command() + availableScenario = self.get_available_scenario(presetTypes=presetTypes, presetScenarioCounts=presetScenarioCounts) # Set the existing preset to a null built-in value; will be replaced with true on reading - test_presets = copy.deepcopy(new_presets) - test_presets[0].builtIn = NullValue + test_presets = copy.deepcopy(current_presets) - # Write to the presets attribute after calling AtomicRequest command - await self.write_presets(endpoint=endpoint, presets=test_presets) + builtInPresets = list(preset for preset in test_presets if preset.builtIn) - # Send the AtomicRequest commit command - await self.send_atomic_request_commit_command() + if availableScenario is not None and len(builtInPresets) > 0: + builtInPresets[0].builtIn = NullValue - # Read the presets attribute and verify it was updated since AtomicRequest commit was called after writing presets - presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) - logger.info(f"Rx'd Presets: {presets}") - asserts.assert_equal(presets, new_presets_with_handle, "Presets were not updated which is not expected") + test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=availableScenario, + coolingSetpoint=2700, heatingSetpoint=1900, builtIn=False)) + + # Send the AtomicRequest begin command + await self.send_atomic_request_begin_command() + + # Write to the presets attribute after calling AtomicRequest command + await self.write_presets(endpoint=endpoint, presets=test_presets) + + # Send the AtomicRequest commit command + await self.send_atomic_request_commit_command() + + # Read the presets attribute and verify it was updated since AtomicRequest commit was called after writing presets + current_presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) + logger.info(f"Rx'd Presets: {current_presets}") + self.check_saved_presets(test_presets, current_presets) + + presetScenarioCounts = self.count_preset_scenarios(current_presets) + else: + logger.info( + "Couldn't run test step 4 since there were no built-in presets") self.step("5") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - # Send the AtomicRequest begin command - await self.send_atomic_request_begin_command(timeout=5000, expected_timeout=3000) - # Write to the presets attribute after removing a built in preset from the list. Remove the first entry. - test_presets = new_presets_with_handle.copy() - test_presets.pop(0) - await self.write_presets(endpoint=endpoint, presets=test_presets) + test_presets = current_presets.copy() + + builtInPresets = list(preset for preset in test_presets if preset.builtIn) + if len(builtInPresets) > 0: + builtInPreset = builtInPresets[0] + test_presets.remove(builtInPreset) + + # Send the AtomicRequest begin command + await self.send_atomic_request_begin_command(timeout=5000, expected_timeout=3000) + + # Write to the presets attribute after calling AtomicRequest command + await self.write_presets(endpoint=endpoint, presets=test_presets) - # Send the AtomicRequest commit command and expect ConstraintError for presets. - await self.send_atomic_request_commit_command(expected_overall_status=Status.Failure, expected_preset_status=Status.ConstraintError) + # Send the AtomicRequest commit command and expect ConstraintError for presets. + await self.send_atomic_request_commit_command(expected_overall_status=Status.Failure, expected_preset_status=Status.ConstraintError) + else: + logger.info( + "Couldn't run test step 5 since there were no built-in presets") self.step("6") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.C06.Rsp") and self.check_pics("TSTAT.S.Cfe.Rsp")): - # Send the SetActivePresetRequest command - await self.send_set_active_preset_handle_request_command(value=b'\x03') + notBuiltInPresets = list(preset for preset in current_presets if preset.builtIn is False) + if len(notBuiltInPresets) > 0: + activePreset = notBuiltInPresets[0] - # Read the active preset handle attribute and verify it was updated to preset handle - activePresetHandle = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.ActivePresetHandle) - logger.info(f"Rx'd ActivePresetHandle: {activePresetHandle}") - asserts.assert_equal(activePresetHandle, b'\x03', "Active preset handle was not updated as expected") + # Send the SetActivePresetRequest command + await self.send_set_active_preset_handle_request_command(value=activePreset.presetHandle) - # Send the AtomicRequest begin command - await self.send_atomic_request_begin_command() + # Read the active preset handle attribute and verify it was updated to preset handle + activePresetHandle = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.ActivePresetHandle) + logger.info(f"Rx'd ActivePresetHandle: {activePresetHandle}") + asserts.assert_equal(activePresetHandle, activePreset.presetHandle, + "Active preset handle was not updated as expected") - # Write to the presets attribute after removing the preset that was set as the active preset handle. Remove the last entry with preset handle (b'\x03') - test_presets = new_presets_with_handle.copy() - del test_presets[-1] - await self.write_presets(endpoint=endpoint, presets=test_presets) + # Send the AtomicRequest begin command + await self.send_atomic_request_begin_command() - # Send the AtomicRequest commit command and expect InvalidInState for presets. - await self.send_atomic_request_commit_command(expected_overall_status=Status.Failure, expected_preset_status=Status.InvalidInState) + # Write to the presets attribute after removing the preset that was set as the active preset handle. Remove the last entry with preset handle (b'\x03') + test_presets = current_presets.copy() + test_presets = list(preset for preset in test_presets if preset.presetHandle is not activePresetHandle) + logger.info(f"Sending Presets: {test_presets}") + await self.write_presets(endpoint=endpoint, presets=test_presets) + + # Send the AtomicRequest commit command and expect InvalidInState for presets. + await self.send_atomic_request_commit_command(expected_overall_status=Status.Failure, expected_preset_status=Status.InvalidInState) + else: + logger.info( + "Couldn't run test step 6 since there were no non-built-in presets to activate and delete") self.step("7") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - # Send the AtomicRequest begin command - await self.send_atomic_request_begin_command() - # Write to the presets attribute after setting the builtIn flag to False for preset with handle (b'\x01') - test_presets = copy.deepcopy(new_presets_with_handle) - test_presets[0].builtIn = False + test_presets = copy.deepcopy(current_presets) - await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) + builtInPresets = list(preset for preset in test_presets if preset.builtIn is True) + if len(builtInPresets) > 0: - # Clear state for next test. - await self.send_atomic_request_rollback_command() + # Send the AtomicRequest begin command + await self.send_atomic_request_begin_command() + + builtInPresets[0].builtIn = False + + await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) + + # Clear state for next test. + await self.send_atomic_request_rollback_command() + else: + logger.info( + "Couldn't run test step 7 since there was no built-in presets") self.step("8") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - # Send the AtomicRequest begin command - await self.send_atomic_request_begin_command() + availableScenario = self.get_available_scenario(presetTypes=presetTypes, presetScenarioCounts=presetScenarioCounts) - # Write to the presets attribute after adding a preset with builtIn set to True - test_presets = copy.deepcopy(new_presets_with_handle) - test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=cluster.Enums.PresetScenarioEnum.kWake, - name="Wake", coolingSetpoint=2800, heatingSetpoint=1800, builtIn=True)) + if len(current_presets) > 0 and len(current_presets) < numberOfPresetsSupported and availableScenario is not None: - status = await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) + test_presets = copy.deepcopy(current_presets) - # Clear state for next test. - await self.send_atomic_request_rollback_command() + # Send the AtomicRequest begin command + await self.send_atomic_request_begin_command() + + # Write to the presets attribute after adding a preset with builtIn set to True + test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=availableScenario, + coolingSetpoint=2800, heatingSetpoint=1800, builtIn=True)) + + status = await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) + + # Clear state for next test. + await self.send_atomic_request_rollback_command() + else: + logger.info( + "Couldn't run test step 8 since there was no available preset scenario to append") self.step("9") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): @@ -365,8 +456,8 @@ async def test_TC_TSTAT_4_2(self): await self.send_atomic_request_begin_command() # Write to the presets attribute after adding a preset with a preset handle that doesn't exist in Presets attribute - test_presets = copy.deepcopy(new_presets_with_handle) - test_presets.append(cluster.Structs.PresetStruct(presetHandle=b'\x08', presetScenario=cluster.Enums.PresetScenarioEnum.kWake, + test_presets = copy.deepcopy(current_presets) + test_presets.append(cluster.Structs.PresetStruct(presetHandle=random.randbytes(16), presetScenario=cluster.Enums.PresetScenarioEnum.kWake, name="Wake", coolingSetpoint=2800, heatingSetpoint=1800, builtIn=True)) status = await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.NotFound) @@ -377,67 +468,106 @@ async def test_TC_TSTAT_4_2(self): self.step("10") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - # Send the AtomicRequest begin command - await self.send_atomic_request_begin_command() + availableScenario = self.get_available_scenario(presetTypes=presetTypes, presetScenarioCounts=presetScenarioCounts) - # Write to the presets attribute after adding a duplicate preset with handle (b'\x03') - test_presets = copy.deepcopy(new_presets_with_handle) - test_presets.append(cluster.Structs.PresetStruct( - presetHandle=b'\x03', presetScenario=cluster.Enums.PresetScenarioEnum.kSleep, name="Sleep", coolingSetpoint=2700, heatingSetpoint=1900, builtIn=False)) + if len(current_presets) > 0 and len(current_presets) < numberOfPresetsSupported and availableScenario is not None: - await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) + test_presets = copy.deepcopy(current_presets) + duplicatePreset = test_presets[0] - # Clear state for next test. - await self.send_atomic_request_rollback_command() + # Send the AtomicRequest begin command + await self.send_atomic_request_begin_command() + + # Write to the presets attribute after adding a duplicate preset with handle (b'\x03') + test_presets.append(cluster.Structs.PresetStruct( + presetHandle=duplicatePreset.presetHandle, presetScenario=availableScenario, coolingSetpoint=2700, heatingSetpoint=1900, builtIn=False)) + + await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) + + # Clear state for next test. + await self.send_atomic_request_rollback_command() + else: + logger.info( + "Couldn't run test step 10 since there was no available preset scenario to duplicate") self.step("11") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - # Send the AtomicRequest begin command - await self.send_atomic_request_begin_command() + test_presets = copy.deepcopy(current_presets) - # Write to the presets attribute after setting the builtIn flag to True for preset with handle (b'\x03') - test_presets = copy.deepcopy(new_presets_with_handle) - test_presets[2].builtIn = True + notBuiltInPresets = list(preset for preset in test_presets if preset.builtIn is False) - await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) + if len(notBuiltInPresets) > 0: + # Write to the presets attribute after setting the builtIn flag to True for preset with handle (b'\x03') + notBuiltInPresets[0].builtIn = True - # Clear state for next test. - await self.send_atomic_request_rollback_command() + # Send the AtomicRequest begin command + await self.send_atomic_request_begin_command() + + await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) + + # Clear state for next test. + await self.send_atomic_request_rollback_command() + else: + logger.info( + "Couldn't run test step 11 since there were no presets that were not built-in") self.step("12") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - # Send the AtomicRequest begin command - await self.send_atomic_request_begin_command() + availableScenarios = list(presetType.presetScenario for presetType in presetTypes if (presetType.presetTypeFeatures & cluster.Bitmaps.PresetTypeFeaturesBitmap.kSupportsNames) != 0 and presetScenarioCounts.get( + presetType.presetScenario, 0) < presetType.numberOfPresets) - # Write to the presets attribute after setting a name for preset with handle (b'\x01') that doesn't support names - test_presets = copy.deepcopy(new_presets_with_handle) - test_presets[0].name = "Occupied" + test_presets = copy.deepcopy(current_presets) + presets_without_name_support = list(preset for preset in test_presets if preset.presetScenario in availableScenarios) - await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) + if len(presets_without_name_support) is 0 and len(availableScenarios) > 0: + new_preset = cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=availableScenarios[0], + coolingSetpoint=2800, heatingSetpoint=1800, builtIn=True) + test_presets.append(new_preset) + presets_without_name_support = [new_preset] - # Clear state for next test. - await self.send_atomic_request_rollback_command() + if len(availableScenarios) > 0: + + # Write to the presets attribute after setting a name for preset with handle (b'\x01') that doesn't support names + presets_without_name_support[0].name = "Name" + + # Send the AtomicRequest begin command + await self.send_atomic_request_begin_command() + + await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) + + # Clear state for next test. + await self.send_atomic_request_rollback_command() + else: + logger.info( + "Couldn't run test step 12 since there was no available preset scenario without name support") self.step("13") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - # Send the AtomicRequest begin command - await self.send_atomic_request_begin_command() + availableScenario = self.get_available_scenario(presetTypes=presetTypes, presetScenarioCounts=presetScenarioCounts) - # Write to the presets attribute with a new valid preset added - test_presets = copy.deepcopy(new_presets_with_handle) - test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=cluster.Enums.PresetScenarioEnum.kWake, - name="Wake", coolingSetpoint=2800, heatingSetpoint=1800, builtIn=False)) + if len(current_presets) > 0 and len(current_presets) < numberOfPresetsSupported and availableScenario is not None: - await self.write_presets(endpoint=endpoint, presets=test_presets) + # Write to the presets attribute with a new valid preset added + test_presets = copy.deepcopy(current_presets) + test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=availableScenario, + coolingSetpoint=2800, heatingSetpoint=1800, builtIn=False)) - # Roll back - await self.send_atomic_request_rollback_command() + # Send the AtomicRequest begin command + await self.send_atomic_request_begin_command() + + await self.write_presets(endpoint=endpoint, presets=test_presets) - # Send the AtomicRequest commit command and expect InvalidInState as the previous edit request was cancelled - await self.send_atomic_request_commit_command(expected_status=Status.InvalidInState) + # Roll back + await self.send_atomic_request_rollback_command() + + # Send the AtomicRequest commit command and expect InvalidInState as the previous edit request was cancelled + await self.send_atomic_request_commit_command(expected_status=Status.InvalidInState) + else: + logger.info( + "Couldn't run test step 13 since there was no available preset scenario to add") self.step("14") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): @@ -453,7 +583,7 @@ async def test_TC_TSTAT_4_2(self): # Send the AtomicRequest begin command from the secondary controller await self.send_atomic_request_begin_command() - await self.write_presets(endpoint=endpoint, presets=test_presets, dev_ctrl=secondary_controller, expected_status=Status.Busy) + await self.write_presets(endpoint=endpoint, presets=current_presets, dev_ctrl=secondary_controller, expected_status=Status.Busy) # Roll back await self.send_atomic_request_rollback_command() @@ -476,27 +606,13 @@ async def test_TC_TSTAT_4_2(self): self.step("17") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - # Read the PresetTypes to get the preset scenarios supported by the Thermostat. - presetTypes = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.PresetTypes) - scenarioNotPresent = None # Find a preset scenario not present in PresetTypes to run this test. - for scenario in cluster.Enums.PresetScenarioEnum: - foundMatchingScenario = False - for presetType in presetTypes: - if presetType.presetScenario == scenario: - foundMatchingScenario = True - break - if not foundMatchingScenario: - scenarioNotPresent = scenario - break - - if scenarioNotPresent is None: - logger.info( - "Couldn't run test step 17 since all preset types in PresetScenarioEnum are supported by this Thermostat") - else: - test_presets = new_presets_with_handle.copy() + scenarioMap = dict(map(lambda presetType: (presetType.presetScenario, presetType), presetTypes)) + unavailableScenarios = list(preset for preset in test_presets if preset.presetScenario not in scenarioMap) + if len(unavailableScenarios) > 0: + test_presets = current_presets.copy() test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=scenarioNotPresent, name="Preset", coolingSetpoint=2500, heatingSetpoint=1700, builtIn=False)) @@ -507,23 +623,15 @@ async def test_TC_TSTAT_4_2(self): # Clear state for next test. await self.send_atomic_request_rollback_command() + else: + logger.info( + "Couldn't run test step 17 since all preset types in PresetScenarioEnum are supported by this Thermostat") self.step("18") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - # Read the numberOfPresets supported. - numberOfPresetsSupported = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.NumberOfPresets) - - # Read the PresetTypes to get the preset scenarios to build the Presets list. - presetTypes = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.PresetTypes) - - # Read the Presets to copy the existing presets into our testPresets list below. - presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) - # Calculate the length of the Presets list that could be created using the preset scenarios in PresetTypes and numberOfPresets supported for each scenario. - totalExpectedPresetsLength = 0 - for presetType in presetTypes: - totalExpectedPresetsLength += presetType.numberOfPresets + totalExpectedPresetsLength = sum(presetType.numberOfPresets for presetType in presetTypes) if totalExpectedPresetsLength > numberOfPresetsSupported: testPresets = [] @@ -533,14 +641,14 @@ async def test_TC_TSTAT_4_2(self): # For each supported scenario, copy all the existing presets that match it, then add more presets # until we hit the cap on the number of presets for that scenario. presetsAddedForScenario = 0 - for preset in presets: + for preset in current_presets: if scenario == preset.presetScenario: testPresets.append(preset) presetsAddedForScenario = presetsAddedForScenario + 1 while presetsAddedForScenario < presetType.numberOfPresets: testPresets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=scenario, - name="Preset", coolingSetpoint=2500, heatingSetpoint=1700, builtIn=False)) + coolingSetpoint=2500, heatingSetpoint=1700, builtIn=False)) presetsAddedForScenario = presetsAddedForScenario + 1 # Send the AtomicRequest begin command From d2ef1baba8f66ae6f63f0b66ff3d6b1527b165c2 Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Thu, 29 Aug 2024 08:56:27 -0700 Subject: [PATCH 13/28] Pick midpoint setpoints for new presets --- src/python_testing/TC_TSTAT_4_2.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index e4217a27e33601..b89349262bd5ac 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -260,6 +260,12 @@ async def test_TC_TSTAT_4_2(self): minCoolSetpointLimit = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.MinCoolSetpointLimit) maxCoolSetpointLimit = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.MaxCoolSetpointLimit) + asserts.assert_true(minHeatSetpointLimit < maxHeatSetpointLimit, "Heat setpoint range invalid") + asserts.assert_true(minCoolSetpointLimit < maxCoolSetpointLimit, "Cool setpoint range invalid") + + heatSetpoint = minHeatSetpointLimit + ((maxHeatSetpointLimit - minHeatSetpointLimit) / 2) + coolSetpoint = minCoolSetpointLimit + ((maxCoolSetpointLimit - minCoolSetpointLimit) / 2) + self.step("2") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050")): @@ -439,7 +445,7 @@ async def test_TC_TSTAT_4_2(self): # Write to the presets attribute after adding a preset with builtIn set to True test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=availableScenario, - coolingSetpoint=2800, heatingSetpoint=1800, builtIn=True)) + coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=True)) status = await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) @@ -458,7 +464,7 @@ async def test_TC_TSTAT_4_2(self): # Write to the presets attribute after adding a preset with a preset handle that doesn't exist in Presets attribute test_presets = copy.deepcopy(current_presets) test_presets.append(cluster.Structs.PresetStruct(presetHandle=random.randbytes(16), presetScenario=cluster.Enums.PresetScenarioEnum.kWake, - name="Wake", coolingSetpoint=2800, heatingSetpoint=1800, builtIn=True)) + name="Wake", coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=True)) status = await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.NotFound) @@ -523,7 +529,7 @@ async def test_TC_TSTAT_4_2(self): if len(presets_without_name_support) is 0 and len(availableScenarios) > 0: new_preset = cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=availableScenarios[0], - coolingSetpoint=2800, heatingSetpoint=1800, builtIn=True) + coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=True) test_presets.append(new_preset) presets_without_name_support = [new_preset] @@ -553,7 +559,7 @@ async def test_TC_TSTAT_4_2(self): # Write to the presets attribute with a new valid preset added test_presets = copy.deepcopy(current_presets) test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=availableScenario, - coolingSetpoint=2800, heatingSetpoint=1800, builtIn=False)) + coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)) # Send the AtomicRequest begin command await self.send_atomic_request_begin_command() From ad2a2e63032c1dec88659fd41ac3511597d0b847 Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Thu, 29 Aug 2024 09:13:29 -0700 Subject: [PATCH 14/28] Lint fixes --- src/python_testing/TC_TSTAT_4_2.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index b89349262bd5ac..8939ba20631624 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -88,7 +88,7 @@ def check_saved_presets(self, sent_presets: list, returned_presets: list): def count_preset_scenarios(self, presets: list): presetScenarios = {} for preset in presets: - if not preset.presetScenario in presetScenarios: + if preset.presetScenario not in presetScenarios: presetScenarios[preset.presetScenario] = 1 else: presetScenarios[preset.presetScenario] += 1 @@ -527,7 +527,7 @@ async def test_TC_TSTAT_4_2(self): test_presets = copy.deepcopy(current_presets) presets_without_name_support = list(preset for preset in test_presets if preset.presetScenario in availableScenarios) - if len(presets_without_name_support) is 0 and len(availableScenarios) > 0: + if len(presets_without_name_support) == 0 and len(availableScenarios) > 0: new_preset = cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=availableScenarios[0], coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=True) test_presets.append(new_preset) From 8e7c198317b4059e664996b2b018ab1486f046db Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Thu, 29 Aug 2024 10:39:21 -0700 Subject: [PATCH 15/28] Apply suggestions from code review Co-authored-by: Boris Zbarsky --- src/python_testing/TC_TSTAT_4_2.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index 8939ba20631624..b010af4752241f 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -397,9 +397,8 @@ async def test_TC_TSTAT_4_2(self): # Send the AtomicRequest begin command await self.send_atomic_request_begin_command() - # Write to the presets attribute after removing the preset that was set as the active preset handle. Remove the last entry with preset handle (b'\x03') - test_presets = current_presets.copy() - test_presets = list(preset for preset in test_presets if preset.presetHandle is not activePresetHandle) + # Write to the presets attribute after removing the preset that was set as the active preset handle. + test_presets = list(preset for preset in current_presets if preset.presetHandle is not activePresetHandle) logger.info(f"Sending Presets: {test_presets}") await self.write_presets(endpoint=endpoint, presets=test_presets) @@ -412,7 +411,7 @@ async def test_TC_TSTAT_4_2(self): self.step("7") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - # Write to the presets attribute after setting the builtIn flag to False for preset with handle (b'\x01') + # Write to the presets attribute after setting the builtIn flag to False for a built-in preset. test_presets = copy.deepcopy(current_presets) builtInPresets = list(preset for preset in test_presets if preset.builtIn is True) @@ -484,7 +483,7 @@ async def test_TC_TSTAT_4_2(self): # Send the AtomicRequest begin command await self.send_atomic_request_begin_command() - # Write to the presets attribute after adding a duplicate preset with handle (b'\x03') + # Write to the presets attribute after adding a duplicate preset test_presets.append(cluster.Structs.PresetStruct( presetHandle=duplicatePreset.presetHandle, presetScenario=availableScenario, coolingSetpoint=2700, heatingSetpoint=1900, builtIn=False)) @@ -504,7 +503,7 @@ async def test_TC_TSTAT_4_2(self): notBuiltInPresets = list(preset for preset in test_presets if preset.builtIn is False) if len(notBuiltInPresets) > 0: - # Write to the presets attribute after setting the builtIn flag to True for preset with handle (b'\x03') + # Write to the presets attribute after setting the builtIn flag to True for a non-built-in preset. notBuiltInPresets[0].builtIn = True # Send the AtomicRequest begin command From 231e17f236856d634683ea76753f89731ced3339 Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Thu, 29 Aug 2024 10:39:41 -0700 Subject: [PATCH 16/28] Fixes from code review --- .../include/thermostat-delegate-impl.h | 2 +- .../src/thermostat-delegate-impl.cpp | 3 --- src/python_testing/TC_TSTAT_4_2.py | 27 +++++++++---------- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h b/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h index 7726fc337866bb..6e6cd7c8d11696 100644 --- a/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h +++ b/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h @@ -33,7 +33,7 @@ namespace Thermostat { * */ -static constexpr uint8_t kMaxNumberOfPresetTypes = 6; +static constexpr uint8_t kMaxNumberOfPresetTypes = 5; // TODO: #34556 Support multiple presets of each type. // We will support only one preset of each preset type. diff --git a/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp b/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp index da58c840049275..a6995698fffa66 100644 --- a/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp +++ b/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp @@ -87,9 +87,6 @@ CHIP_ERROR ThermostatDelegate::GetPresetTypeAtIndex(size_t index, PresetTypeStru { .presetScenario = PresetScenarioEnum::kVacation, .numberOfPresets = kMaxNumberOfPresetsOfEachType, .presetTypeFeatures = to_underlying(PresetTypeFeaturesBitmap::kSupportsNames) }, - { .presetScenario = PresetScenarioEnum::kUserDefined, - .numberOfPresets = kMaxNumberOfPresetsOfEachType, - .presetTypeFeatures = to_underlying(PresetTypeFeaturesBitmap::kSupportsNames) }, }; if (index < ArraySize(presetTypes)) { diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index b010af4752241f..36a2ef4264ebf8 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -73,7 +73,7 @@ def check_atomic_response(self, response: object, expected_status: Status = Stat "Schedules attribute should have the right status") asserts.assert_true(found_preset_status, "Preset attribute should have a status") - def check_saved_presets(self, sent_presets: list, returned_presets: list): + def check_returned_presets(self, sent_presets: list, returned_presets: list): asserts.assert_true(len(sent_presets) == len(returned_presets), "Returned presets are a different length than sent presets") for i, sent_preset in enumerate(sent_presets): returned_preset = returned_presets[i] @@ -309,7 +309,7 @@ async def test_TC_TSTAT_4_2(self): # Read the presets attribute and verify it was updated by the write saved_presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) logger.info(f"Rx'd Presets: {saved_presets}") - self.check_saved_presets(test_presets, saved_presets) + self.check_returned_presets(test_presets, saved_presets) await self.send_atomic_request_rollback_command() @@ -348,7 +348,7 @@ async def test_TC_TSTAT_4_2(self): # Read the presets attribute and verify it was updated since AtomicRequest commit was called after writing presets current_presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) logger.info(f"Rx'd Presets: {current_presets}") - self.check_saved_presets(test_presets, current_presets) + self.check_returned_presets(test_presets, current_presets) presetScenarioCounts = self.count_preset_scenarios(current_presets) else: @@ -475,7 +475,7 @@ async def test_TC_TSTAT_4_2(self): availableScenario = self.get_available_scenario(presetTypes=presetTypes, presetScenarioCounts=presetScenarioCounts) - if len(current_presets) > 0 and len(current_presets) < numberOfPresetsSupported and availableScenario is not None: + if len(current_presets) < numberOfPresetsSupported and availableScenario is not None: test_presets = copy.deepcopy(current_presets) duplicatePreset = test_presets[0] @@ -485,7 +485,7 @@ async def test_TC_TSTAT_4_2(self): # Write to the presets attribute after adding a duplicate preset test_presets.append(cluster.Structs.PresetStruct( - presetHandle=duplicatePreset.presetHandle, presetScenario=availableScenario, coolingSetpoint=2700, heatingSetpoint=1900, builtIn=False)) + presetHandle=duplicatePreset.presetHandle, presetScenario=availableScenario, coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)) await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) @@ -520,8 +520,8 @@ async def test_TC_TSTAT_4_2(self): self.step("12") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - availableScenarios = list(presetType.presetScenario for presetType in presetTypes if (presetType.presetTypeFeatures & cluster.Bitmaps.PresetTypeFeaturesBitmap.kSupportsNames) != 0 and presetScenarioCounts.get( - presetType.presetScenario, 0) < presetType.numberOfPresets) + availableScenarios = list(presetType.presetScenario for presetType in presetTypes if (presetType.presetTypeFeatures & cluster.Bitmaps.PresetTypeFeaturesBitmap.kSupportsNames) == 0 and presetScenarioCounts.get( + presetType.presetScenario, 0) <= presetType.numberOfPresets) test_presets = copy.deepcopy(current_presets) presets_without_name_support = list(preset for preset in test_presets if preset.presetScenario in availableScenarios) @@ -553,7 +553,7 @@ async def test_TC_TSTAT_4_2(self): availableScenario = self.get_available_scenario(presetTypes=presetTypes, presetScenarioCounts=presetScenarioCounts) - if len(current_presets) > 0 and len(current_presets) < numberOfPresetsSupported and availableScenario is not None: + if len(current_presets) < numberOfPresetsSupported and availableScenario is not None: # Write to the presets attribute with a new valid preset added test_presets = copy.deepcopy(current_presets) @@ -611,15 +611,14 @@ async def test_TC_TSTAT_4_2(self): self.step("17") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): - scenarioNotPresent = None - # Find a preset scenario not present in PresetTypes to run this test. scenarioMap = dict(map(lambda presetType: (presetType.presetScenario, presetType), presetTypes)) - unavailableScenarios = list(preset for preset in test_presets if preset.presetScenario not in scenarioMap) + unavailableScenarios = list( + presetScenario for presetScenario in cluster.Enums.PresetScenarioEnum if presetScenario not in scenarioMap) if len(unavailableScenarios) > 0: test_presets = current_presets.copy() - test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=scenarioNotPresent, - name="Preset", coolingSetpoint=2500, heatingSetpoint=1700, builtIn=False)) + test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=unavailableScenarios[0], + name="Preset", coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)) # Send the AtomicRequest begin command await self.send_atomic_request_begin_command() @@ -653,7 +652,7 @@ async def test_TC_TSTAT_4_2(self): while presetsAddedForScenario < presetType.numberOfPresets: testPresets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=scenario, - coolingSetpoint=2500, heatingSetpoint=1700, builtIn=False)) + coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)) presetsAddedForScenario = presetsAddedForScenario + 1 # Send the AtomicRequest begin command From f50dc83c59c49cab850197a9ee417bd20e75286d Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Thu, 29 Aug 2024 11:50:57 -0700 Subject: [PATCH 17/28] Apply suggestions from code review Co-authored-by: Boris Zbarsky --- src/python_testing/TC_TSTAT_4_2.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index 36a2ef4264ebf8..22b9dd09eebc3e 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -612,9 +612,9 @@ async def test_TC_TSTAT_4_2(self): if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): # Find a preset scenario not present in PresetTypes to run this test. - scenarioMap = dict(map(lambda presetType: (presetType.presetScenario, presetType), presetTypes)) + supportedScenarios = set(presetType.presetScenario for presetType in presetTypes) unavailableScenarios = list( - presetScenario for presetScenario in cluster.Enums.PresetScenarioEnum if presetScenario not in scenarioMap) + presetScenario for presetScenario in cluster.Enums.PresetScenarioEnum if presetScenario not in supportedScenarios) if len(unavailableScenarios) > 0: test_presets = current_presets.copy() test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=unavailableScenarios[0], @@ -629,7 +629,7 @@ async def test_TC_TSTAT_4_2(self): await self.send_atomic_request_rollback_command() else: logger.info( - "Couldn't run test step 17 since all preset types in PresetScenarioEnum are supported by this Thermostat") + "Couldn't run test step 17 since all preset scenarios in PresetScenarioEnum are supported by this Thermostat") self.step("18") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): From 70daba425fdc31559d010f18f47ac4f20ae02586 Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Thu, 29 Aug 2024 11:57:48 -0700 Subject: [PATCH 18/28] Fix remaining places with hard-coded setpoints --- src/python_testing/TC_TSTAT_4_2.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index 22b9dd09eebc3e..24f07d1d601b27 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -297,7 +297,7 @@ async def test_TC_TSTAT_4_2(self): preset.builtIn = NullValue test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=availableScenario, - coolingSetpoint=2700, heatingSetpoint=1900, builtIn=False)) + coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)) await self.send_atomic_request_begin_command() @@ -334,7 +334,7 @@ async def test_TC_TSTAT_4_2(self): builtInPresets[0].builtIn = NullValue test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=availableScenario, - coolingSetpoint=2700, heatingSetpoint=1900, builtIn=False)) + coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)) # Send the AtomicRequest begin command await self.send_atomic_request_begin_command() From 0e34bfa1e3c291a3c157b886766f8f879dd2f5d6 Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Thu, 29 Aug 2024 11:58:19 -0700 Subject: [PATCH 19/28] Don't abort test if there are no built-in presets --- .../thermostat-common/src/thermostat-delegate-impl.cpp | 3 +++ src/python_testing/TC_TSTAT_4_2.py | 7 ++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp b/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp index a6995698fffa66..da58c840049275 100644 --- a/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp +++ b/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp @@ -87,6 +87,9 @@ CHIP_ERROR ThermostatDelegate::GetPresetTypeAtIndex(size_t index, PresetTypeStru { .presetScenario = PresetScenarioEnum::kVacation, .numberOfPresets = kMaxNumberOfPresetsOfEachType, .presetTypeFeatures = to_underlying(PresetTypeFeaturesBitmap::kSupportsNames) }, + { .presetScenario = PresetScenarioEnum::kUserDefined, + .numberOfPresets = kMaxNumberOfPresetsOfEachType, + .presetTypeFeatures = to_underlying(PresetTypeFeaturesBitmap::kSupportsNames) }, }; if (index < ArraySize(presetTypes)) { diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index 24f07d1d601b27..f3bef3b846bfe1 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -328,10 +328,11 @@ async def test_TC_TSTAT_4_2(self): # Set the existing preset to a null built-in value; will be replaced with true on reading test_presets = copy.deepcopy(current_presets) - builtInPresets = list(preset for preset in test_presets if preset.builtIn) + if availableScenario is not None: + builtInPresets = list(preset for preset in test_presets if preset.builtIn) - if availableScenario is not None and len(builtInPresets) > 0: - builtInPresets[0].builtIn = NullValue + if len(builtInPresets) > 0: + builtInPresets[0].builtIn = NullValue test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=availableScenario, coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)) From a9a0bd43bd4242e38d927248310d87d9655ff54c Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Thu, 29 Aug 2024 12:01:36 -0700 Subject: [PATCH 20/28] Remove unneeded length check --- src/python_testing/TC_TSTAT_4_2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index f3bef3b846bfe1..c868f2996096d1 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -436,7 +436,7 @@ async def test_TC_TSTAT_4_2(self): availableScenario = self.get_available_scenario(presetTypes=presetTypes, presetScenarioCounts=presetScenarioCounts) - if len(current_presets) > 0 and len(current_presets) < numberOfPresetsSupported and availableScenario is not None: + if len(current_presets) < numberOfPresetsSupported and availableScenario is not None: test_presets = copy.deepcopy(current_presets) From 25376bf4e44bdc81fa2e0319244c58d910eccb70 Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Thu, 29 Aug 2024 12:03:01 -0700 Subject: [PATCH 21/28] Fix max number of preset types --- .../thermostat-common/include/thermostat-delegate-impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h b/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h index 6e6cd7c8d11696..7726fc337866bb 100644 --- a/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h +++ b/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h @@ -33,7 +33,7 @@ namespace Thermostat { * */ -static constexpr uint8_t kMaxNumberOfPresetTypes = 5; +static constexpr uint8_t kMaxNumberOfPresetTypes = 6; // TODO: #34556 Support multiple presets of each type. // We will support only one preset of each preset type. From 5fd85ada80cff4459f56c7df50c5b4d725fb439e Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Thu, 29 Aug 2024 23:56:53 -0700 Subject: [PATCH 22/28] Add test for individual preset scenario limits --- src/python_testing/TC_TSTAT_4_2.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index c868f2996096d1..0982fec35199e0 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -619,7 +619,7 @@ async def test_TC_TSTAT_4_2(self): if len(unavailableScenarios) > 0: test_presets = current_presets.copy() test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=unavailableScenarios[0], - name="Preset", coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)) + coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)) # Send the AtomicRequest begin command await self.send_atomic_request_begin_command() @@ -635,6 +635,29 @@ async def test_TC_TSTAT_4_2(self): self.step("18") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): + # Generate list of tuples of scenarios and number of remaining presets per scenario allowed + presetScenarioHeadroom = list([presetType.presetScenario, + presetType.numberOfPresets - presetScenarioCounts.get(presetType.presetScenario, 0)] for presetType in presetTypes) + + if len(presetScenarioHeadroom) > 0: + # Find the preset scenario with the fewest number of remaining allowed presets + presetScenarioHeadroom = sorted(presetScenarioHeadroom, key=lambda diff: diff[1]) + presetScenario = presetScenarioHeadroom[0][0] + headroom = presetScenarioHeadroom[0][1] + + # Add one more preset than is allowed by the preset type + test_presets = copy.deepcopy(current_presets) + for _ in range(0, headroom + 1): + test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=presetScenario, + coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)) + + await self.send_atomic_request_begin_command() + + await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ResourceExhausted) + + # Clear state for next test. + await self.send_atomic_request_rollback_command() + # Calculate the length of the Presets list that could be created using the preset scenarios in PresetTypes and numberOfPresets supported for each scenario. totalExpectedPresetsLength = sum(presetType.numberOfPresets for presetType in presetTypes) From a01650b4b10c1fe8202f580ce8270c2dc0c8542e Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Fri, 30 Aug 2024 00:29:18 -0700 Subject: [PATCH 23/28] Fix lint issue --- .../clusters/thermostat-server/thermostat-server-presets.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp index 8596d767f3e1b0..a880ba236ac224 100644 --- a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp +++ b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp @@ -418,13 +418,14 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele if (presetCount > maximumPresetCount) { - ChipLogError(Zcl, "Preset count exceeded %zu: %zu ", maximumPresetCount, presetCount); + ChipLogError(Zcl, "Preset count exceeded %u: %u ", (unsigned) maximumPresetCount, (unsigned) presetCount); return CHIP_IM_GLOBAL_STATUS(ResourceExhausted); } if (presetScenarioCount > maximumPresetScenarioCount) { - ChipLogError(Zcl, "Preset scenario count exceeded %zu: %zu ", maximumPresetScenarioCount, presetScenarioCount); + ChipLogError(Zcl, "Preset scenario count exceeded %u: %u ", (unsigned) maximumPresetScenarioCount, + (unsigned) presetScenarioCount); return CHIP_IM_GLOBAL_STATUS(ResourceExhausted); } From d0a4777cbe585db524bc1f44d3f61ac42dbe861f Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Fri, 30 Aug 2024 11:17:27 -0700 Subject: [PATCH 24/28] Return invalid in state if we're unable to iterate over the preset types for some reason --- .../thermostat-server-presets.cpp | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp index a880ba236ac224..3de3fcd486a8fe 100644 --- a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp +++ b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp @@ -154,32 +154,34 @@ bool GetMatchingPresetInPresets(Delegate * delegate, const DataModel::NullableGetPresetTypeAtIndex(i, presetType); - if (err != CHIP_NO_ERROR) + if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) { - // Either we failed to fetch the next preset type, in which case we should error higher, - // or we exhausted the list trying to find the preset type - return 0; + // We exhausted the list trying to find the preset scenario + return CHIP_NO_ERROR; + } + else if (err != CHIP_NO_ERROR) + { + return err; } - if (presetType.presetScenario == presetScenario) { - return presetType.numberOfPresets; + count = presetType.numberOfPresets; + return CHIP_NO_ERROR; } } - return 0; + return CHIP_NO_ERROR; } /** @@ -379,8 +381,13 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele } } - size_t maximumPresetCount = delegate->GetNumberOfPresets(); - size_t maximumPresetScenarioCount = MaximumPresetScenarioCount(delegate, preset.GetPresetScenario()); + size_t maximumPresetCount = delegate->GetNumberOfPresets(); + + size_t maximumPresetScenarioCount = 0; + if (MaximumPresetScenarioCount(delegate, preset.GetPresetScenario(), maximumPresetScenarioCount) != CHIP_NO_ERROR) + { + return CHIP_IM_GLOBAL_STATUS(InvalidInState); + } if (maximumPresetScenarioCount == 0) { From adade9380c728824897bcd80257ab9c5d4101714 Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Fri, 30 Aug 2024 13:00:01 -0700 Subject: [PATCH 25/28] Apply suggestions from code review Co-authored-by: Boris Zbarsky --- .../thermostat-server/thermostat-server-presets.cpp | 6 +++--- src/python_testing/TC_TSTAT_4_2.py | 9 ++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp index 3de3fcd486a8fe..15c654a4ea4300 100644 --- a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp +++ b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp @@ -425,14 +425,14 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele if (presetCount > maximumPresetCount) { - ChipLogError(Zcl, "Preset count exceeded %u: %u ", (unsigned) maximumPresetCount, (unsigned) presetCount); + ChipLogError(Zcl, "Preset count exceeded %u: %u ", static_cast(maximumPresetCount), static_cast(presetCount)); return CHIP_IM_GLOBAL_STATUS(ResourceExhausted); } if (presetScenarioCount > maximumPresetScenarioCount) { - ChipLogError(Zcl, "Preset scenario count exceeded %u: %u ", (unsigned) maximumPresetScenarioCount, - (unsigned) presetScenarioCount); + ChipLogError(Zcl, "Preset scenario count exceeded %u: %u ", static_cast(maximumPresetScenarioCount), + static_cast(presetScenarioCount)); return CHIP_IM_GLOBAL_STATUS(ResourceExhausted); } diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index 1ec793487bd191..2eeb51cfbc79e1 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -649,14 +649,13 @@ async def test_TC_TSTAT_4_2(self): if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): # Generate list of tuples of scenarios and number of remaining presets per scenario allowed - presetScenarioHeadroom = list([presetType.presetScenario, - presetType.numberOfPresets - presetScenarioCounts.get(presetType.presetScenario, 0)] for presetType in presetTypes) + presetScenarioHeadroom = list((presetType.presetScenario, + presetType.numberOfPresets - presetScenarioCounts.get(presetType.presetScenario, 0)) for presetType in presetTypes) if len(presetScenarioHeadroom) > 0: - # Find the preset scenario with the fewest number of remaining allowed presets + # Find the preset scenario with the smallest number of remaining allowed presets presetScenarioHeadroom = sorted(presetScenarioHeadroom, key=lambda diff: diff[1]) - presetScenario = presetScenarioHeadroom[0][0] - headroom = presetScenarioHeadroom[0][1] + (presetScenario, headroom) = presetScenarioHeadroom[0] # Add one more preset than is allowed by the preset type test_presets = copy.deepcopy(current_presets) From 93206a3e7882c0ec2b5f8cafad38f59ad5e14274 Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Fri, 30 Aug 2024 13:04:25 -0700 Subject: [PATCH 26/28] Remove unneeded active preset setting --- src/python_testing/TC_TSTAT_4_2.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index 2eeb51cfbc79e1..4bdb8c39b17de4 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -356,12 +356,6 @@ async def test_TC_TSTAT_4_2(self): logger.info( "Couldn't run test step 4 since there were no built-in presets") - notBuiltInPresets = list(preset for preset in current_presets if preset.builtIn is False) - if len(notBuiltInPresets) > 0: - activePreset = notBuiltInPresets[0] - - activePresetHandle = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.ActivePresetHandle) - self.step("5") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): From 82a86a538f7878c5c3b0104036528f054e448469 Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Fri, 30 Aug 2024 14:11:49 -0700 Subject: [PATCH 27/28] Restyled patch --- .../clusters/thermostat-server/thermostat-server-presets.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp index 15c654a4ea4300..3c4ef01da632ad 100644 --- a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp +++ b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp @@ -425,7 +425,8 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele if (presetCount > maximumPresetCount) { - ChipLogError(Zcl, "Preset count exceeded %u: %u ", static_cast(maximumPresetCount), static_cast(presetCount)); + ChipLogError(Zcl, "Preset count exceeded %u: %u ", static_cast(maximumPresetCount), + static_cast(presetCount)); return CHIP_IM_GLOBAL_STATUS(ResourceExhausted); } From d1fe36dce500fb8eb94e1f4d3d134df419673664 Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Sat, 31 Aug 2024 17:39:09 -0700 Subject: [PATCH 28/28] Suggestions from code review --- .../thermostat-server-presets.cpp | 12 +++++------- src/python_testing/TC_TSTAT_4_2.py | 15 ++++++++------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp index 3c4ef01da632ad..1dede071619962 100644 --- a/src/app/clusters/thermostat-server/thermostat-server-presets.cpp +++ b/src/app/clusters/thermostat-server/thermostat-server-presets.cpp @@ -171,7 +171,7 @@ CHIP_ERROR MaximumPresetScenarioCount(Delegate * delegate, PresetScenarioEnum pr // We exhausted the list trying to find the preset scenario return CHIP_NO_ERROR; } - else if (err != CHIP_NO_ERROR) + if (err != CHIP_NO_ERROR) { return err; } @@ -326,10 +326,6 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele return CHIP_IM_GLOBAL_STATUS(ConstraintError); } - // We're going to append this preset, so let's assume a count as though it had already been inserted - size_t presetCount = 1; - size_t presetScenarioCount = 1; - if (preset.GetPresetHandle().IsNull()) { if (IsBuiltIn(preset)) @@ -381,8 +377,7 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele } } - size_t maximumPresetCount = delegate->GetNumberOfPresets(); - + size_t maximumPresetCount = delegate->GetNumberOfPresets(); size_t maximumPresetScenarioCount = 0; if (MaximumPresetScenarioCount(delegate, preset.GetPresetScenario(), maximumPresetScenarioCount) != CHIP_NO_ERROR) { @@ -403,6 +398,9 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * dele // Before adding this preset to the pending presets, if the expected length of the pending presets' list // exceeds the total number of presets supported, return RESOURCE_EXHAUSTED. Note that the preset has not been appended yet. + // We're going to append this preset, so let's assume a count as though it had already been inserted + size_t presetCount = 1; + size_t presetScenarioCount = 1; for (uint8_t i = 0; true; i++) { PresetStructWithOwnedMembers otherPreset; diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index 4bdb8c39b17de4..56f2afbf80a00d 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -29,6 +29,7 @@ import copy import logging import random +from collections import namedtuple import chip.clusters as Clusters from chip import ChipDeviceCtrl # Needed before chip.FabricAdmin @@ -642,20 +643,20 @@ async def test_TC_TSTAT_4_2(self): self.step("18") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): + ScenarioHeadroom = namedtuple("ScenarioHeadroom", "presetScenario remaining") # Generate list of tuples of scenarios and number of remaining presets per scenario allowed - presetScenarioHeadroom = list((presetType.presetScenario, + presetScenarioHeadrooms = list(ScenarioHeadroom(presetType.presetScenario, presetType.numberOfPresets - presetScenarioCounts.get(presetType.presetScenario, 0)) for presetType in presetTypes) - if len(presetScenarioHeadroom) > 0: + if presetScenarioHeadrooms: # Find the preset scenario with the smallest number of remaining allowed presets - presetScenarioHeadroom = sorted(presetScenarioHeadroom, key=lambda diff: diff[1]) - (presetScenario, headroom) = presetScenarioHeadroom[0] + presetScenarioHeadrooms = sorted(presetScenarioHeadrooms, key=lambda psh: psh.remaining) + presetScenarioHeadroom = presetScenarioHeadrooms[0] # Add one more preset than is allowed by the preset type test_presets = copy.deepcopy(current_presets) - for _ in range(0, headroom + 1): - test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=presetScenario, - coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)) + test_presets.extend([cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=presetScenarioHeadroom.presetScenario, + coolingSetpoint=coolSetpoint, heatingSetpoint=heatSetpoint, builtIn=False)] * (presetScenarioHeadroom.remaining + 1)) await self.send_atomic_request_begin_command()