scripts: west_commands: runners: nrf: A couple of improvements #98018
Conversation
Until now, for historical reasons (see f42cef9 and 58e0e31), the use of the --erase-mode command-line switch was reserved for the nRF54L family. But in fact this can be used (instead of --erase) for any of the Nordic ICs. This patch extends the usage of this switch regardless of family. Signed-off-by: Carles Cufi <[email protected]>
| @@ -483,12 +485,15 @@ class ZephyrBinaryRunner(abc.ABC): | |||
| commands in its constructor. The actual command execution is | |||
| handled in the run() method.''' | |||
|
|
|||
| def __init__(self, cfg: RunnerConfig): | |||
| def __init__(self, cfg: RunnerConfig, dry_run: bool=_DRY_RUN): | |||
There was a problem hiding this comment.
I'm skeptical about this. The reason I made it just for debugging was that some runners are truly interactive and can't work without actually running their commands, because the output of one is used as an input for another.
I'd rather see this dropped from core.py. If you really need this for multiple runners, it would make more sense to me to have this added via RunnerCaps.
There was a problem hiding this comment.
While adding this via RunnerCaps makes sense, I would still need to modify core.py to do that, and I would also need to somewhat replace the _DRY_RUN global with something in the instance?
For now I will make this nRF-only while we decide.
There was a problem hiding this comment.
@mbolivar since we are very close to release I've opted to make this specific to a single runner. I may try to generalize this later if it's worth it, because I had to duplicate a tiny bit of code.
There was a problem hiding this comment.
@carlescufi right, it would still be in core.py but it would be done on a runner-specific basis and as far as I can tell would just end up being an additional common --dry-run argument in ZephyrBinaryRunner.add_parser(), which would be handled by the runners themselves. The debugging option could remain unchanged. What am I missing?
There was a problem hiding this comment.
Nothing at all, all good. I just wanted to make sure to get this in for 4.3.0 @mbolivar. Will follow-up next week.
carlescufi
force-pushed
the
nrf-runner-fixes
branch
from
cdb56ca to
05ff0f5
Compare
|
@pdgendt I realized I was missing parts of the implementation I had not committed. Can you please take another look? |
|
@pdgendt could you refresh your +1 here? thanks! |
In order to allow for users to invoke "west flash" without actual hardware connected but still running the logic and pregeneration of commands, specifically the json file. Signed-off-by: Carles Cufi <[email protected]>
carlescufi
force-pushed
the
nrf-runner-fixes
branch
from
05ff0f5 to
0cf8411
Compare
|
7 participants
See commit messages.