-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[Fix] Possible fix for matter-test-scripts issue #228: Remove PICS from RVC tests #34181
base: master
Are you sure you want to change the base?
Changes from 4 commits
5048552
fd9f558
3881efe
f871894
a9b96f7
4f912dd
242a564
66f8e00
f55030c
2ed9e31
e6ce20e
d43154a
8ef7176
d46ab09
2f8c4bc
fd40db1
53e2575
26985ab
454ddd8
221bc8c
9c4e4e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,7 +47,7 @@ def __init__(self, *args): | |
self.supported_modes_dut = [] | ||
|
||
async def read_mod_attribute_expect_success(self, endpoint, attribute): | ||
cluster = Clusters.Objects.RvcCleanMode | ||
cluster = Clusters.RvcCleanMode | ||
return await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=attribute) | ||
|
||
def pics_TC_RVCCLEANM_1_2(self) -> list[str]: | ||
|
@@ -57,11 +57,16 @@ def pics_TC_RVCCLEANM_1_2(self) -> list[str]: | |
async def test_TC_RVCCLEANM_1_2(self): | ||
self.endpoint = self.matter_test_config.endpoint | ||
|
||
attributes = Clusters.RvcCleanMode.Attributes | ||
RVCClean_cluster = Clusters.RvcCleanMode | ||
attributes = RVCClean_cluster.Attributes | ||
RVCClean_attr_list = attributes.AttributeList | ||
attribute_list = await self.read_single_attribute_check_success(endpoint=self.endpoint, cluster=RVCClean_cluster, attribute=RVCClean_attr_list) | ||
supported_modes_attr_id = attributes.SupportedModes.attribute_id | ||
current_mode_attr_id = attributes.CurrentMode.attribute_id | ||
|
||
self.print_step(1, "Commissioning, already done") | ||
|
||
if self.check_pics("RVCCLEANM.S.A0000"): | ||
if supported_modes_attr_id in attribute_list: | ||
self.print_step(2, "Read SupportedModes attribute") | ||
supported_modes = await self.read_mod_attribute_expect_success(endpoint=self.endpoint, attribute=attributes.SupportedModes) | ||
|
||
|
@@ -124,7 +129,7 @@ async def test_TC_RVCCLEANM_1_2(self): | |
asserts.assert_true(has_vacuum_or_mop_mode_tag, | ||
"At least one ModeOptionsStruct entry must include either the Vacuum or Mop mode tag") | ||
|
||
if self.check_pics("RVCCLEANM.S.A0001"): | ||
if current_mode_attr_id in attribute_list: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto here |
||
self.print_step(3, "Read CurrentMode attribute") | ||
current_mode = await self.read_mod_attribute_expect_success(endpoint=self.endpoint, attribute=attributes.CurrentMode) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,19 +84,40 @@ async def test_TC_RVCCLEANM_2_1(self): | |
self.endpoint = self.matter_test_config.endpoint | ||
self.mode_ok = self.matter_test_config.global_test_params['PIXIT.RVCCLEANM.MODE_CHANGE_OK'] | ||
self.mode_fail = self.matter_test_config.global_test_params['PIXIT.RVCCLEANM.MODE_CHANGE_FAIL'] | ||
self.is_ci = self.check_pics("PICS_SDK_CI_ONLY") | ||
if self.is_ci: | ||
app_pid = self.matter_test_config.app_pid | ||
if app_pid == 0: | ||
asserts.fail("The --app-pid flag must be set when PICS_SDK_CI_ONLY is set") | ||
app_pid = self.matter_test_config.app_pid | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can just leave this CI check in for now. The question of how to determine whether an app is running in the CI is fairly open, but this PICS code is kind of meta. The goal for these issues is to remove the PICS codes for things that are represented on the device already (clusters, features, attributes, commands etc). This one is fine. |
||
if app_pid != 0: | ||
cecille marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.is_ci = True | ||
self.app_pipe = self.app_pipe + str(app_pid) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that this is needed as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you @hicklin, |
||
|
||
asserts.assert_true(self.check_pics("RVCCLEANM.S.A0000"), "RVCCLEANM.S.A0000 must be supported") | ||
asserts.assert_true(self.check_pics("RVCCLEANM.S.A0001"), "RVCCLEANM.S.A0001 must be supported") | ||
asserts.assert_true(self.check_pics("RVCCLEANM.S.C00.Rsp"), "RVCCLEANM.S.C00.Rsp must be supported") | ||
asserts.assert_true(self.check_pics("RVCCLEANM.S.C01.Tx"), "RVCCLEANM.S.C01.Tx must be supported") | ||
RVCClean_cluster = Clusters.RvcCleanMode | ||
|
||
# Gathering Available Attributes and associated ids | ||
attributes = RVCClean_cluster.Attributes | ||
RVCClean_attr_list = attributes.AttributeList | ||
attribute_list = await self.read_single_attribute_check_success(endpoint=self.endpoint, cluster=RVCClean_cluster, attribute=RVCClean_attr_list) | ||
supported_modes_attr_id = attributes.SupportedModes.attribute_id | ||
current_mode_attr_id = attributes.CurrentMode.attribute_id | ||
|
||
# Gathering Accepted and Generated Commands and associated ids | ||
commands = RVCClean_cluster.Commands | ||
RVCClean_accptcmd_list = attributes.AcceptedCommandList | ||
RVCClean_gencmd_list = attributes.GeneratedCommandList | ||
accepted_cmd_list = await self.read_single_attribute_check_success(endpoint=self.endpoint, cluster=RVCClean_cluster, attribute=RVCClean_accptcmd_list) | ||
generated_cmd_list = await self.read_single_attribute_check_success(endpoint=self.endpoint, cluster=RVCClean_cluster, attribute=RVCClean_gencmd_list) | ||
chg_mode_cmd_id = commands.ChangeToMode.command_id | ||
chg_rsp_cmd_id = commands.ChangeToModeResponse.command_id | ||
|
||
if supported_modes_attr_id not in attribute_list: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same q here - these are all mandatory, would it make sense to just remove these given that we have conformance checks now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there are other ways of checking this, yes. |
||
asserts.fail("supported modes needs to be supported attribute") | ||
|
||
if current_mode_attr_id not in attribute_list: | ||
asserts.fail("Current mode needs to be supported attribute") | ||
|
||
if chg_mode_cmd_id not in accepted_cmd_list: | ||
asserts.fail("Change To Mode receiving commands needs to be supported") | ||
|
||
attributes = Clusters.RvcCleanMode.Attributes | ||
if chg_rsp_cmd_id not in generated_cmd_list: | ||
asserts.fail("Change To Mode Response to transmit commands needs to be supported") | ||
|
||
self.print_step(1, "Commissioning, already done") | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,7 +75,7 @@ def print_instruction(self, step_number, instruction): | |
def pics_TC_RVCCLEANM_2_2(self) -> list[str]: | ||
return ["RVCCLEANM.S"] | ||
|
||
# Sends and out-of-band command to the rvc-app | ||
# Sends an out-of-band command to the rvc-app | ||
def write_to_app_pipe(self, command): | ||
with open(self.app_pipe, "w") as app_pipe: | ||
app_pipe.write(command + "\n") | ||
|
@@ -86,16 +86,24 @@ def write_to_app_pipe(self, command): | |
@async_test_body | ||
async def test_TC_RVCCLEANM_2_2(self): | ||
self.endpoint = self.matter_test_config.endpoint | ||
self.is_ci = self.check_pics("PICS_SDK_CI_ONLY") | ||
if self.is_ci: | ||
app_pid = self.matter_test_config.app_pid | ||
if app_pid == 0: | ||
asserts.fail("The --app-pid flag must be set when PICS_SDK_CI_ONLY is set.c") | ||
app_pid = self.matter_test_config.app_pid | ||
if app_pid != 0: | ||
self.is_ci = True | ||
self.app_pipe = self.app_pipe + str(app_pid) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that this is needed as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you, you are indeed correct that was definitely needed in there and not sure how I had deleted it. |
||
|
||
asserts.assert_true(self.check_pics("RVCCLEANM.S"), "RVCCLEANM.S must be supported") | ||
asserts.assert_true(self.check_pics("RVCRUNM.S.A0000"), "RVCRUNM.S.A0000 must be supported") | ||
asserts.assert_true(self.check_pics("RVCRUNM.S.A0001"), "RVCRUNM.S.A0001 must be supported") | ||
# replaces the RVCRUNM attributes from PICS file | ||
RVCRun_cluster = Clusters.RvcRunMode | ||
attributes = Clusters.RvcRunMode.Attributes | ||
RVCRun_attr_list = attributes.AttributeList | ||
attribute_list = await self.read_single_attribute_check_success(endpoint=self.endpoint, cluster=RVCRun_cluster, attribute=RVCRun_attr_list) | ||
supported_modes_attr_id = attributes.SupportedModes.attribute_id | ||
current_mode_attr_id = attributes.CurrentMode.attribute_id | ||
|
||
if supported_modes_attr_id not in attribute_list: | ||
asserts.fail("Supported modes needs to be supported attribute") | ||
|
||
if current_mode_attr_id not in attribute_list: | ||
asserts.fail("Current mode needs to be supported attribute") | ||
hicklin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
self.print_step(1, "Commissioning, already done") | ||
|
||
|
@@ -105,8 +113,10 @@ async def test_TC_RVCCLEANM_2_2(self): | |
|
||
self.print_step( | ||
2, "Manually put the device in a state in which the RVC Run Mode cluster’s CurrentMode attribute is set to a mode without the Idle mode tag.") | ||
|
||
if self.is_ci: | ||
await self.send_run_change_to_mode_cmd(1) | ||
|
||
else: | ||
self.wait_for_user_input( | ||
prompt_msg="Manually put the device in a state in which the RVC Run Mode cluster’s CurrentMode attribute is set to a mode without the Idle mode tag, and press Enter when done.") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -79,19 +79,36 @@ async def test_TC_RVCRUNM_2_1(self): | |
self.endpoint = self.matter_test_config.endpoint | ||
self.mode_ok = self.matter_test_config.global_test_params['PIXIT.RVCRUNM.MODE_CHANGE_OK'] | ||
self.mode_fail = self.matter_test_config.global_test_params['PIXIT.RVCRUNM.MODE_CHANGE_FAIL'] | ||
self.is_ci = self.check_pics("PICS_SDK_CI_ONLY") | ||
if self.is_ci: | ||
app_pid = self.matter_test_config.app_pid | ||
if app_pid == 0: | ||
asserts.fail("The --app-pid flag must be set when PICS_SDK_CI_ONLY is set.c") | ||
self.app_pipe = self.app_pipe + str(app_pid) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this may be needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you, you are correct that was definitely needed in there. I have returned it to its rightful place here now. |
||
|
||
asserts.assert_true(self.check_pics("RVCRUNM.S.A0000"), "RVCRUNM.S.A0000 must be supported") | ||
asserts.assert_true(self.check_pics("RVCRUNM.S.A0001"), "RVCRUNM.S.A0001 must be supported") | ||
asserts.assert_true(self.check_pics("RVCRUNM.S.C00.Rsp"), "RVCRUNM.S.C00.Rsp must be supported") | ||
asserts.assert_true(self.check_pics("RVCRUNM.S.C01.Tx"), "RVCRUNM.S.C01.Tx must be supported") | ||
RVCRun_cluster = Clusters.RvcRunMode | ||
|
||
# Gathering Available Attributes and associated ids | ||
attributes = RVCRun_cluster.Attributes | ||
RVCRun_attr_list = attributes.AttributeList | ||
attribute_list = await self.read_single_attribute_check_success(endpoint=self.endpoint, cluster=RVCRun_cluster, attribute=RVCRun_attr_list) | ||
supported_modes_attr_id = attributes.SupportedModes.attribute_id | ||
current_mode_attr_id = attributes.CurrentMode.attribute_id | ||
|
||
# Gathering Accepted and Generated Commands and associated ids | ||
commands = RVCRun_cluster.Commands | ||
RVCRun_accptcmd_list = attributes.AcceptedCommandList | ||
RVCRun_gencmd_list = attributes.GeneratedCommandList | ||
accepted_cmd_list = await self.read_single_attribute_check_success(endpoint=self.endpoint, cluster=RVCRun_cluster, attribute=RVCRun_accptcmd_list) | ||
generated_cmd_list = await self.read_single_attribute_check_success(endpoint=self.endpoint, cluster=RVCRun_cluster, attribute=RVCRun_gencmd_list) | ||
chg_mode_cmd_id = commands.ChangeToMode.command_id | ||
chg_rsp_cmd_id = commands.ChangeToModeResponse.command_id | ||
|
||
if supported_modes_attr_id not in attribute_list: | ||
asserts.fail("supported modes needs to be supported attribute") | ||
|
||
if current_mode_attr_id not in attribute_list: | ||
asserts.fail("Current mode needs to be supported attribute") | ||
|
||
if chg_mode_cmd_id not in accepted_cmd_list: | ||
asserts.fail("Change To Mode receiving commands needs to be supported") | ||
|
||
attributes = Clusters.RvcRunMode.Attributes | ||
if chg_rsp_cmd_id not in generated_cmd_list: | ||
asserts.fail("Change To Mode Response to send commands needs to be supported") | ||
|
||
self.print_step(1, "Commissioning, already done") | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,21 +118,42 @@ async def test_TC_RVCRUNM_2_2(self): | |
"PIXIT.RVCRUNM.MODE_B:<mode id>") | ||
|
||
self.endpoint = self.matter_test_config.endpoint | ||
self.is_ci = self.check_pics("PICS_SDK_CI_ONLY") | ||
self.mode_a = self.matter_test_config.global_test_params['PIXIT.RVCRUNM.MODE_A'] | ||
self.mode_b = self.matter_test_config.global_test_params['PIXIT.RVCRUNM.MODE_B'] | ||
if self.is_ci: | ||
app_pid = self.matter_test_config.app_pid | ||
if app_pid == 0: | ||
asserts.fail("The --app-pid flag must be set when PICS_SDK_CI_ONLY is set.c") | ||
app_pid = self.matter_test_config.app_pid | ||
if app_pid != 0: | ||
self.is_ci = True | ||
self.app_pipe = self.app_pipe + str(app_pid) | ||
|
||
app_pid = self.matter_test_config.app_pid | ||
if app_pid != 0: | ||
self.is_ci = True | ||
self.app_pipe = self.app_pipe + str(app_pid) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may be needed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you, you are correct that was definitely needed in there. I have returned it to its rightful place here now. |
||
|
||
asserts.assert_true(self.check_pics("RVCRUNM.S"), "RVCRUNM.S must be supported") | ||
# I think that the following PICS should be listed in the preconditions section in the test plan as if either | ||
# of these PICS is not supported, this test would not be useful. | ||
asserts.assert_true(self.check_pics("RVCRUNM.S.A0000"), "RVCRUNM.S.A0000 must be supported") | ||
asserts.assert_true(self.check_pics("RVCRUNM.S.A0001"), "RVCRUNM.S.A0001 must be supported") | ||
asserts.assert_true(self.check_pics("RVCRUNM.S.C00.Rsp"), "RVCRUNM.S.C00.Rsp must be supported") | ||
RVCRun_cluster = Clusters.RvcRunMode | ||
|
||
# Gathering Available Attributes and associated ids | ||
attributes = RVCRun_cluster.Attributes | ||
RVCRun_attr_list = attributes.AttributeList | ||
attribute_list = await self.read_single_attribute_check_success(endpoint=self.endpoint, cluster=RVCRun_cluster, attribute=RVCRun_attr_list) | ||
supported_modes_attr_id = attributes.SupportedModes.attribute_id | ||
current_mode_attr_id = attributes.CurrentMode.attribute_id | ||
|
||
# Gathering Accepted and Generated Commands and associated ids | ||
commands = RVCRun_cluster.Commands | ||
RVCRun_accptcmd_list = attributes.AcceptedCommandList | ||
accepted_cmd_list = await self.read_single_attribute_check_success(endpoint=self.endpoint, cluster=RVCRun_cluster, attribute=RVCRun_accptcmd_list) | ||
chg_mode_cmd_id = commands.ChangeToMode.command_id | ||
|
||
if supported_modes_attr_id not in attribute_list: | ||
asserts.fail("Supported modes needs to be supported attribute") | ||
|
||
if current_mode_attr_id not in attribute_list: | ||
asserts.fail("Current mode needs to be supported attribute") | ||
|
||
if chg_mode_cmd_id not in accepted_cmd_list: | ||
asserts.fail("Change To Mode receiving commands needs to be supported") | ||
|
||
asserts.assert_true(self.check_pics("RVCRUNM.S.M.CAN_MANUALLY_CONTROLLED"), | ||
"RVCRUNM.S.M.CAN_MANUALLY_CONTROLLED must be supported") | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q for the RVC folks - this is mandatory, right? Can we just remove this check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is. Same of
A0001
. Should this check be made somewhere else, or is it dealt by the data model?