-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
west: runners: jlink: add support for -nogui 1 command line parameter #27617
west: runners: jlink: add support for -nogui 1 command line parameter #27617
Conversation
@mbolivar-nordic Thanks for the inspiration on how to discover the version of J-Link Commander. It is a bit of a hack, but I do think we should consider it for inclusion. I have only tested this under GNU/Linux. |
6fc4d34
to
675c4e1
Compare
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.
It is a bit of a hack, but I do think we should consider it for inclusion.
I agree. I consider the fact that JLinkExe doesn't seem to have a way to report its own version a bug in the tool, which we are working around in the best way we know how. If we think of something better we can always add it.
My only requested changes would be:
- Please add a source code comment explaining why this odd code exists
- Can you do this by adding
**kwargs
support toZephyrBinaryRunner.check_output
and passing the timeout parameter that way instead of callingsubprocess
manually? It's important thatwest -v debug
prints all the subprocesses executed by the tool being wrapped, which we accomplish via theself._log_cmd
calls in theZephyrBinaryRunner
call
,check_call
, andcheck_output
methods.
675c4e1
to
c58b6d7
Compare
@mbolivar-nordic Good point.
Done.
I deliberately left it out of the log in order not to confuse anybody, but I can see that argument supports including it as well. I have added support for |
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.
Thanks for the updates. Seems sane to me.
This doesn't seem to work on Windows:
|
@MaureenHelm Could you please try with an increased timeout value? E.g. 0.2 seconds? |
No luck. I tried 0.2 and 1.0, but neither worked. |
Oh. The regex doesn't match on the windows version. I'll provide an update shortly. |
c58b6d7
to
24a93aa
Compare
@MaureenHelm Please retest. |
The update doesn't work either. |
@henrikbrixandersen If there's no clean way to get this to work there, can I suggest skipping this test |
Add support for the J-Link Commander "-NoGui 1" command line parameter in the West J-Link runner. This command line parameter suppresses GUI dialogs (except for license dialogs) in J-Link Commander starting from v6.80. Signed-off-by: Henrik Brix Andersen <hebad@vestas.com>
24a93aa
to
2cd2cfa
Compare
@mbolivar-nordic Thank. I have added such a check. Without access to a Windows-based development PC, it's a bit difficult for me to debug what is going on. I suggest leaving the check in and hopefully someone (on Windows) will step up and submit a PR for making the check work on that platform as well. |
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.
@henrikbrixandersen If there's no clean way to get this to work there, can I suggest skipping this test
if platform.system() == 'Windows'
?@mbolivar-nordic Thank. I have added such a check. Without access to a Windows-based development PC, it's a bit difficult for me to debug what is going on. I suggest leaving the check in and hopefully someone (on Windows) will step up and submit a PR for making the check work on that platform as well.
Sorry I wasn't particularly helpful and haven't much time this week to debug. You can assign an enhancement issue to me and I'll take a closer look later.
Add support for the J-Link Commander "-NoGui 1" command line parameter in the West J-Link runner.
This command line parameter suppresses GUI dialogs (except for license dialogs) in J-Link Commander starting from v6.80.
Signed-off-by: Henrik Brix Andersen hebad@vestas.com