-
Notifications
You must be signed in to change notification settings - Fork 10
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
tests: on_target: add ppk test #474
Conversation
fb57529
to
9b03968
Compare
9b03968
to
d12fd0f
Compare
see the badge here: https://github.com/hello-nrfcloud/firmware/tree/ppk_test |
.github/workflows/on_target.yml
Outdated
fi | ||
|
||
# Configure Git to trust the current working directory | ||
git config --global --add safe.directory /__w/firmware/firmware |
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.
Suggest: git config --global --add safe.directory `pwd`
.github/workflows/on_target.yml
Outdated
CSV_FILE=tests/on_target/power_measurements.csv | ||
|
||
# Check if the badge file exists at the specified location | ||
if [ -f $BADGE_FILE ]; then |
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 can use if [ ! -f $BADGE_FILE ];
, we don't need to log every time an expected file was found :)
tests/on_target/tests/conftest.py
Outdated
@@ -102,7 +102,7 @@ def t91x_traces(t91x_board): | |||
def hex_file(): | |||
# Search for the firmware hex file in the artifacts folder | |||
artifacts_dir = "artifacts" | |||
hex_pattern = r"hello\.nrfcloud\.com-[a-f0-9]+-thingy91x-nrf91\.hex" | |||
hex_pattern = r"hello\.nrfcloud\.com-[a-z.0-9]+-thingy91x-nrf91\.hex" |
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.
Not sure if the dot should be there? r"hello\.nrfcloud\.com-[a-z0-9]+-thingy91x-nrf91\.hex"
Perhaps just change to .*
Or maybe you want [0-9a-z\.]
for literal dot. (Needs escape)
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.
added the dot to match release files hello.nrfcloud.com-v2.0.1-thingy91x-nrf91.hex
I'll change to [0-9a-z\.]
tests/on_target/tests/test_power.py
Outdated
|
||
|
||
@pytest.fixture(scope="module") | ||
def ppk2(): |
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.
I think this should be named thingy91x_ppk2
ppk2_dev.use_ampere_meter() # set ampere meter mode | ||
ppk2_dev.toggle_DUT_power("ON") # enable DUT power | ||
|
||
time.sleep(10) |
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.
What are we waiting for here? DUT uart should be available all the time? PPK only cuts power to the nrf91, not the nrf53?
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.
I dont get uart if dut is not powered ON. That is also why I used a single fixture. Otherwise I could have also split ppk2 and uart into independent fixtures.
And I added the sleep to give uart time to show up, maybe can be removed
tests/on_target/tests/test_power.py
Outdated
SEGGER = os.getenv('SEGGER') | ||
UART_ID = os.getenv('UART_ID', SEGGER) | ||
|
||
def get_uarts(): |
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.
I think we can import this one from conftest.py instead of redefining?
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.
actually I am also both redefining get_uarts(), will import only
.github/workflows/on_target.yml
Outdated
working-directory: thingy91x-oob | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
run: | |
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.
Consider moving this to a seperate bash script since it's quite much commands.
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.
will do
tests/on_target/tests/test_power.py
Outdated
CSV_FILE = "power_measurements.csv" | ||
HMTL_PLOT_FILE = "power_measurements_plot.html" | ||
|
||
SEGGER = os.getenv('SEGGER') |
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.
These also are obtained in conftest.py
tests/on_target/tests/test_power.py
Outdated
logger.info(f"Minimum average current measured: {average}uA") | ||
if average < 0: | ||
pytest.fail(f"current cant be negative, current average: {average}") | ||
if average <= 10: |
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.
for consistency either make this to elif or the following to if. Or just a blank line in between between the previous if statement and this one.
ab1171e
to
2a0c613
Compare
16f5a62
to
c5f23d3
Compare
bc337b8
to
52495f8
Compare
52495f8
to
bbf692e
Compare
bbf692e
to
e56a0e5
Compare
e56a0e5
to
ef09879
Compare
ef09879
to
b9d7233
Compare
Add ppk test and update nightly badge accordingly. Also, add html interactive power plot, hosted on github pages. Signed-off-by: Giacomo Dematteis <[email protected]>
b9d7233
to
3fcf785
Compare
Add ppk test and update nightly badge accordingly. Also, add html interactive power plot, hosted on github pages.