Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TC-TCTL-2.3 python test #36703

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ jobs:
src/app/zap-templates/zcl/data-model/chip/software-diagnostics-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/switch-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/target-navigator-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/temperature-control-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/temperature-measurement-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/test-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/thermostat-user-interface-configuration-cluster.xml \
Expand Down
97 changes: 97 additions & 0 deletions src/python_testing/TC_TCTL_2_3.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#
# Copyright (c) 2024 Project CHIP Authors
# All rights reserved.
#
# 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.
#
# === BEGIN CI TEST ARGUMENTS ===
# test-runner-runs:
# run1:
# app: ${ALL_CLUSTERS_APP}
# app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
# script-args: >
# --storage-path admin_storage.json
# --commissioning-method on-network
# --discriminator 1234
# --passcode 20202021
# --trace-to json:${TRACE_TEST_JSON}.json
# --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
# --PICS src/app/tests/suites/certification/ci-pics-values
# factory-reset: true
# quiet: true
# === END CI TEST ARGUMENTS ===

import chip.clusters as Clusters
from chip.testing.matter_testing import MatterBaseTest, TestStep, async_test_body, default_matter_test_main
from mobly import asserts


class TC_TCTL_2_3(MatterBaseTest):
def desc_TC_TCTL_2_3(self) -> str:
return "[TC-TCTL-2.3] Optional temperature level attributes with DUT as Server"

def pics_TC_TCTL_2_3(self):
"""Return the PICS definitions associated with this test."""
pics = [
"TCTL.S", # Temperature Control as a Server
"TCTL.S.F01", # Does a device support temperature level feature
]
return pics

def steps_TC_TCTL_2_3(self) -> list[TestStep]:
steps = [
TestStep(1, "Commissioning, already done", is_commissioning=True),
TestStep(2, "TH reads from the DUT the SelectedTemperatureLevel attribute"),
TestStep(3, "TH reads from the DUT the SupportedTemperatureLevels attribute and verifies string lengths"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind adding in the expected outcome strings here too? It's not a huge deal, but it makes it much easier to update the test plan.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, added

]
return steps

@async_test_body
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct as of today, but it's something we want to change. The ability to change this is going to be dependent on this PR landing #36727. So nothing to do right now, but either a follow up or something that can be addressed in this PR if the other PR lands before you can undraft.

Right now, the purpose of the pics_ function above is to allow the test harness to select the tests used for certification. This is not used at all in CI, and the CI works because we know the all-clusters-app happens to have this feature enabled.

The PICS come from an external file (actually set of files) that describe what's on the device at a certain endpoint. The PICS in this test say that there is a temperature control server on the endpoint (TCTL.S) and the temperature control server supports feature 1 (TCTL.S.F01). Both of these are things that we can read directly from the device - we don't need to be going out to an external file for this information, and it's preferable not to since that's more error prone (file might be incorrect, incorrect file might have been loaded, maybe no file was supplied etc.)

We recently started testing with self-selecting test. We have a set of decorators that will only run the test if certain conditions hold. However, as noted in this PR description (#36727), we still need to keep this top level pics_ method for the test harness because even just starting tests from the test harness is slow.

So, once that PR lands, I'm going to suggest changing @async_test_body to
@run_if_endpoint_matches(has_feature(Clusters.TemperatureControl, Clusters.TemperatureControl.Bitmaps.Feature.kTemperatureLevel))

That will allow the automation that GD is writing to automatically select this test based on the device composition.

TL;DR - nothing to do now, if that PR lands before this goes in, let's update

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependency is merged so I have decorated with @run_if_endpoint_matches

async def test_TC_TCTL_2_3(self):
self.step(1)

# Step 2: Read SelectedTemperatureLevel attribute
self.step(2)
if self.check_pics("TCTL.S.A0004"): # SelectedTemperatureLevel attribute
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above, this PICS just indicates whether a attribute 4 is supported on this cluster / endpoint and that information is available from the device. Getting this information directly from the device rather than from the PICS file is less error prone.

I've asked one of the CSA folks working on test automation to add in a guard function to just read this from the device. In other tests, we've done this by reading the attribute list directly in the test, but since we do this a lot it makes sense to centralize this. The PR to add that is here: #34290.

As above, if that PR happens to land before this gets undrafted and goes for review, it makes sense to switch over to use that, otherwise it can be done as a followup.

the change here would be something like
if attribute_guard(Clusters.TemperatureControl.Attributes.SelectedTemperatureLevel):

also below.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependency is merged so I'm using attribute_guard here

selected_temp = await self.default_controller.ReadAttribute(
asirko-soft marked this conversation as resolved.
Show resolved Hide resolved
nodeid=self.dut_node_id,
attributes=[(1, Clusters.TemperatureControl.Attributes.SelectedTemperatureLevel)]
)
temp_level = selected_temp[1][Clusters.TemperatureControl][Clusters.TemperatureControl.Attributes.SelectedTemperatureLevel]
asserts.assert_true(0 <= temp_level <= 31,
f"SelectedTemperatureLevel {temp_level} is out of range [0-31]")

# Step 3: Read SupportedTemperatureLevels attribute
self.step(3)
if self.check_pics("TCTL.S.A0005"): # SupportedTemperatureLevels attribute
supported_temps = await self.default_controller.ReadAttribute(
nodeid=self.dut_node_id,
attributes=[(1, Clusters.TemperatureControl.Attributes.SupportedTemperatureLevels)]
)

temp_levels = supported_temps[1][Clusters.TemperatureControl][Clusters.TemperatureControl.Attributes.SupportedTemperatureLevels]
asserts.assert_true(isinstance(temp_levels, list),
"SupportedTemperatureLevels should be a list")
asserts.assert_true(len(temp_levels) <= 32,
f"SupportedTemperatureLevels list length {len(temp_levels)} exceeds maximum of 32")

# Verify string lengths
for level in temp_levels:
asserts.assert_true(isinstance(level, str),
f"Temperature level {level} is not a string")
asserts.assert_true(len(level) <= 16,
f"Temperature level string '{level}' exceeds maximum length of 16")


if __name__ == "__main__":
default_matter_test_main()
Loading