nrf: Enable source line on tracebacks. Improve run-tests. #6395
Conversation
790aa15 to
c272f43
Compare
c272f43 to
48ad869
Compare
|
Rebased on top of master as 2021-08-05 |
48ad869 to
83ac2e0
Compare
Codecov Report
@@ Coverage Diff @@
## master #6395 +/- ##
=======================================
Coverage 98.29% 98.29%
=======================================
Files 154 154
Lines 20015 20015
=======================================
Hits 19674 19674
Misses 341 341
Continue to review full report at Codecov.
|
ports/nrf/mpconfigdevice_nrf51822.h
Outdated
| @@ -59,3 +59,11 @@ | |||
| #define MICROPY_PY_UBINASCII (1) | |||
| #endif | |||
| #endif | |||
|
|
|||
| #ifndef MICROPY_ENABLE_SOURCE_LINE | |||
There was a problem hiding this comment.
It's good to keep these config options in the same order as what appears in py/mpconfig.h, in which case this one should go before MICROPY_PY_ARRAY_SLICE_ASSIGN (and after MICROPY_VFS). Same goes for all additions in this PR.
There was a problem hiding this comment.
Sure, good to know. Done
tests/feature_check/lineno.py
Outdated
| @@ -0,0 +1,6 @@ | |||
| import sys | |||
There was a problem hiding this comment.
I can't see this file being used by tests/run-tests.py, I don't think it does anything having it here.
There was a problem hiding this comment.
I think I wasnt sure about how do we want to add new user facing tests to the project. I included it for completeness
There was a problem hiding this comment.
There are 2 ways to skip tests:
- provide a feature check test (like here)
- explicitly check for the feature as part of the test and skip it if the feature is not supported
Usually (1) is used when the feature is complicated to test for, or if lots of tests rely on it. For one-off tests, option (2) is better.
There was a problem hiding this comment.
Appreciate your comments and took note. I'm taking the test out for now and the PR looks in good shape.
3e5127c to
ab4238e
Compare
ab4238e to
240c395
Compare
|
Thanks for updating. Rebased and merged in 8645b7c |
Improve traceback debugging on nrf targets with enough flash, adding missing source line info and improve run-tests to properly handle opt_level and opt_level_lineno tests.
It will be nice to add line number on tracebacks for nrf targets. This is an important REPL and debugging feature, specially if the board has enough room to acommodate it. This feature adds 204 extra bytes to the firmware. AFAICT, this is a standard feature for all targets with flash >= 1M throught the entire repository and for most targets with exception of minimal, bare-arm and pic16bit.
Currently, run-tests fails on opt_level_lineno for nrf52840 and other nrf targets. This PR adds a feature check to fix the issue.
@glenrub: Other features on the nrf family were also checked, UBINASCII (772 bytes), SYS_STDFILES (268 bytes) and ARRAY_SLICE_ASSIGN (176 bytes) before defining MICROPY_ENABLE_SOURCE_LINE on nrf52832 and conditionally defining it on nrf51822. We certainly can revert the changes on nrf52832 and nrf51822 if 204 extra bytes is too much for them.
Here are the tests performed