-
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
[TC-EPREF-2.1]Adding EPREF Automation script #37008
base: master
Are you sure you want to change the base?
Conversation
PR #37008: Size comparison from f25f635 to 97dae9d Full report (70 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
energy_balances_entries = len(energy_balances) | ||
logging.info(f"EnergyBalances: {energy_balances_entries} entries") | ||
for index, balance_struct in enumerate(energy_balances, start=1): | ||
logging.info(f"[{index}]: {{") |
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.
should there be an ending }
at the end of the loop? Or remove the {
to make this look like YAML?
Maygbe step and label should be indented
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.
@andy31415 As per your suggesstion, updated the code. Could you please review and close the conversation.
src/python_testing/TC_EPREF_2_1.py
Outdated
|
||
# If there are more than 2 BalanceStructs, verify the 'step' values are in ascending order | ||
if list_size > 2: | ||
for i in range(1, list_size): |
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.
you could do something like (unsure if clearer because of the enumerate...)
for i, (a,b) in enumerate(zip(energy_balances[:-1], energy_balances[1:])):
asserts.assert_true(a.step < b.step, f"The step at index {i+1} ({b.step}) should larger than the previous step ({a.step})")
This check seems to work for list size of 2 as well (even if retundant because we checked for 0 and 100 already) so maybe for code simplicity we can remove that condition.
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.
similar comments below for similar code
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.
@andy31415 As per your suggesstion, updated the code. Could you please review and close the conversation.
src/python_testing/TC_EPREF_2_1.py
Outdated
self.step("3a") | ||
if self.pics_guard(self.check_pics("EPREF.S.A0000") and self.check_pics("EPREF.S.A0001")): | ||
energy_balances = await self.read_energy_balances(endpoint=endpoint) | ||
if len(energy_balances) > 0: |
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.
if len(energy_balances) > 0: | |
if energy_balances: |
is more pythonic
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.
@andy31415 As per your suggesstion, updated the code. Could you please review and close the conversation.
src/python_testing/TC_EPREF_2_1.py
Outdated
|
||
# Logging the LowPowerModeSensitivities Attribute output responses from the DUT: | ||
num_of_entries = len(low_power_mode_sensitivities) | ||
logging.info(f"\nLowPowerModeSensitivities: {num_of_entries} entries") |
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.
why is there a newline in the log?
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.
@andy31415 As per your suggesstion, updated the code. Could you please review and close the conversation.
num_of_entries = len(low_power_mode_sensitivities) | ||
logging.info(f"\nLowPowerModeSensitivities: {num_of_entries} entries") | ||
for index, balance_struct in enumerate(low_power_mode_sensitivities, start=1): | ||
logging.info(f"[{index}]: {{") |
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.
closing {
? Indent seems ok here.
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.
@andy31415 As per your suggesstion, updated the code. Could you please review and close the conversation.
PR #37008: Size comparison from 292665e to f735fee Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
Currently the test case TC-EPREF-2.1 has Manual verification steps and no automation script available, The Manual Verfication steps will take a lot of time, hence adding TC_EPREF_2_1.py python script will reduce validation time.
Fix for project-chip/matter-test-scripts#324
Testing