runners: jlink: Add support for thread info #34660
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MaureenHelm
requested changes
Apr 28, 2021
carlescufi
force-pushed
the
jlink-rtos
branch
3 times, most recently
from
f36bb7f
to
3120c1f
Compare
carlescufi
commented
Apr 28, 2021
carlescufi
force-pushed
the
jlink-rtos
branch
6 times, most recently
from
ddfe2e6
to
c3b2787
Compare
gmarull
reviewed
Apr 28, 2021
Copy a fix from test_nrfjprog.py to the other runner test suites. The current code will enter an infinite recursion if you hit the path where os.path.isfile is called, since it's been patched to os_path_isfile_patch in the calling context. The fix is to cache the 'real' version in the parent scope and call it directly as a fallback. Signed-off-by: Martí Bolívar <[email protected]>
Make it easy to get the full path to a required binary. Signed-off-by: Martí Bolívar <[email protected]>
This option has existed since the beginning of the runners package, which greatly predates the way DT is used in zephyr right now. It never really worked the way I wanted it to but it's too much work to fix it now. Try to improve the help a bit at least while I'm looking at it again. Signed-off-by: Martí Bolívar <[email protected]>
Separate the logic that gets the right address from .config into its own helper. Signed-off-by: Martí Bolívar <[email protected]>
Just to make these match check_output(). Signed-off-by: Martí Bolívar <[email protected]>
Make it easier to get a BuildConfiguration from runner code. Stash the result so it only has to be computed once. Signed-off-by: Martí Bolívar <[email protected]>
This will be used to deal with the Segger shared library in a portable way in the jlink runner. Signed-off-by: Martí Bolívar <[email protected]>
|
@carlescufi I've pushed a version here which should fix all the BuildConfiguration issues and includes this support as well: https:/mbolivar-nordic/zephyr/tree/jlink-rtos It also fixes J-Link version detection on Windows, which we will need for this PR. However, I can't get thread info printed reliably, even though I can see the commands I want are being run, like so: https://gist.github.com/mbolivar-nordic/0add8fa4e10deabbfc21ecca12c3ca5a It does work sometimes, though, and I get something like this at the end: https://gist.github.com/mbolivar-nordic/8e62ba863f918c2d2b89532143868843 Could this be a bug in the plugin? |
Thread detection seems to work much more reliably when I test on my Windows machine than on my Linux box. |
mbolivar-nordic
force-pushed
the
jlink-rtos
branch
from
1c91d25
to
40de463
Compare
mbolivar-nordic
force-pushed
the
jlink-rtos
branch
2 times, most recently
from
2341c07
to
2545e80
Compare
|
Thanks to @carlescufi for helping me figure out what was wrong on my Linux box. This is working for me now. |
mbolivar-nordic
force-pushed
the
jlink-rtos
branch
from
2545e80
to
77c5a6a
Compare
|
@MaureenHelm @gmarull please revisit your reviews; I've rewritten this since the original version. |
JLink versions like 'V7.0a' do not conform to PEP 440 version conventions; the 'a' part is used by PEP 440 compliant versions for alphas. It gets parsed to a legacy type by the packaging library, which always is treated as a lower value when compared with a conforming version string. To fix, get the version from the shared library distributed with the JLink tools. This has the side benefit of making the code work on Windows. That's merely a nice to have for -nogui 1 detection for now, but will be essential in the next commit. Reported-by: Jake Mercer <[email protected]> Signed-off-by: Martí Bolívar <[email protected]>
The default commander path is platform specific. Signed-off-by: Martí Bolívar <[email protected]>
Put it all in one log message rather than splitting it up. This makes it look cleaner now that each log message is prefixed with 'runners.link:'. Signed-off-by: Martí Bolívar <[email protected]>
Defer loading .config until we really need it, when we are flashing a binary. Pre-emptively loading it is wasted effort if we're flashing a .hex, which has been the default behavior when possible since dcaabb8 ("west: runners: jlink: prefer .hex over .bin"). Signed-off-by: Martí Bolívar <[email protected]>
This is really verbose, and I doubt anyone cares unless there is a problem. Keep it around when run as 'west -v flash' to allow for debugging, though. Signed-off-by: Martí Bolívar <[email protected]>
Instead of mocking out the BuildConfiguration class, just create its input file and let the real class do the work. This in turn exposes a bug in the way the board name is being pulled out of the BuildConfiguration, which we fix to keep the tests passing. Signed-off-by: Martí Bolívar <[email protected]>
Set options that are definitely true or false to True or False in the options dict. Add a 'getboolean' method that also allows a fallback to False in case the option is not mentioned in .config due to unmet dependencies. This allows calling code to just ask about the option they are interested in, even if the .config file doesn't mention the option at all. Propagate this to users within the runners package and 'west sign', taking advantage of the new build_conf property. Rename the 'bcfg' internal variable in sign.py to 'build_conf' to match other source files that use BuildConfiguration instances, to make it easier to grep for users. Signed-off-by: Martí Bolívar <[email protected]>
This makes it easier for runners to check if the binary has thread info support turned on, allowing automatic configuration of the underlying tool to support threads, if possible. Signed-off-by: Martí Bolívar <[email protected]>
Support this when debugging also. Tweak the style for brevity also while we're here by propertizing the supports_nogui method, etc. Signed-off-by: Martí Bolívar <[email protected]>
Automatically enable per-thread info in GDB if the binary and J-Link support it. Signed-off-by: Martí Bolívar <[email protected]>
mbolivar-nordic
force-pushed
the
jlink-rtos
branch
from
77c5a6a
to
0413848
Compare
gmarull
reviewed
Apr 30, 2021
MaureenHelm
approved these changes
Apr 30, 2021
|
Hello @mbolivar-nordic , |
gmarull
approved these changes
May 3, 2021
|
@mbolivar, @carlescufi: Thank you for providing this! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Starting with J-Link v7.11b, The J-Link Software and Documentation Pack
supports Zephyr RTOS awareness, aka thread info.
For this to work, the Zephyr application needs to be built with
CONFIG_DEBUG_THREAD_INFO=y.
This patch adds the corresponding JLinkGDBServer option when invoking
it, in order to activate the plugin.
Signed-off-by: Carles Cufi [email protected]