Skip to content

Commit 5317f76

Browse files
Marti Bolivarnashif
authored andcommitted
scripts: west: introduce common runner configuration
Continue better integration of the runner subpackage into west by moving the common runner configuration options into the command core. This allows commands like "west flash -h" to display help for common overrides like --kernel-hex. Adjustments needed to make this happen are: - Change the build system to separate common configuration values from runner-specific options and arguments - Prepare the runner core by defining a new RunnerConfig class that represents the common configuration, and accepting that from a new create() method, which replaces create_from_args(). - Convert all concrete runner classes to use the new style of argument parsing and initialization. - Group the command options appropriately for help output readability There's still a bit of tool-specific stuff in the common configuration (gdb and openocd configuration in particular); a more generic way to deal with that will be necessary to better support things like non-GDB debuggers, but that's out of scope of this patch. All the runner-specific options are still in the runner packge, which currently prevents them from being included in "west flash -h" etc. Fixing that is also out of scope of this patch. This has the ancillary benefit of getting rid of the legacy 'debug' argument to ZephyrBinaryRunner, which is no longer appropriate since verbose debug logging is handled by log.py in west. Signed-off-by: Marti Bolivar <[email protected]>
1 parent c9dfbfa commit 5317f76

File tree

15 files changed

+315
-225
lines changed

15 files changed

+315
-225
lines changed

cmake/flash/CMakeLists.txt

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,6 @@ assert_not(DEBUG_SCRIPT "DEBUG_SCRIPT has been removed; use BOARD_DEBUG_RUNNER")
33

44
get_property(RUNNERS GLOBAL PROPERTY ZEPHYR_RUNNERS)
55

6-
# These arguments are common to all runners.
7-
set(RUNNER_ARGS_COMMON
8-
# Required:
9-
"--board-dir=${BOARD_DIR}"
10-
"--kernel-elf=${PROJECT_BINARY_DIR}/${KERNEL_ELF_NAME}"
11-
"--kernel-hex=${PROJECT_BINARY_DIR}/${KERNEL_HEX_NAME}"
12-
"--kernel-bin=${PROJECT_BINARY_DIR}/${KERNEL_BIN_NAME}"
13-
# Optional, but so often needed that they're provided by default:
14-
# (TODO: revisit whether we really want these here)
15-
"--gdb=${CMAKE_GDB}"
16-
"--openocd=${OPENOCD}"
17-
"--openocd-search=${OPENOCD_DEFAULT_PATH}"
18-
)
19-
206
# Enable verbose output, if requested.
217
if(CMAKE_VERBOSE_MAKEFILE)
228
set(RUNNER_VERBOSE "--verbose")
@@ -33,8 +19,37 @@ endif()
3319
# configuration if the board files change.
3420
if(RUNNERS)
3521
set(ZEPHYR_RUNNERS ${RUNNERS} CACHE INTERNAL "Available runners")
36-
set(ZEPHYR_RUNNER_ARGS_COMMON ${RUNNER_ARGS_COMMON} CACHE STRING
37-
"Common arguments to all runners" FORCE)
22+
23+
# Runner configuration. This is provided to all runners, and is
24+
# distinct from the free-form arguments provided by e.g.
25+
# board_runner_args().
26+
#
27+
# Always applicable:
28+
set(ZEPHYR_RUNNER_CONFIG_BOARD_DIR "${BOARD_DIR}"
29+
CACHE STRING "Board definition directory" FORCE)
30+
set(ZEPHYR_RUNNER_CONFIG_KERNEL_ELF "${PROJECT_BINARY_DIR}/${KERNEL_ELF_NAME}"
31+
CACHE STRING "Path to kernel image in ELF format" FORCE)
32+
set(ZEPHYR_RUNNER_CONFIG_KERNEL_HEX "${PROJECT_BINARY_DIR}/${KERNEL_HEX_NAME}"
33+
CACHE STRING "Path to kernel image in Intel Hex format" FORCE)
34+
set(ZEPHYR_RUNNER_CONFIG_KERNEL_BIN "${PROJECT_BINARY_DIR}/${KERNEL_BIN_NAME}"
35+
CACHE STRING "Path to kernel image as raw binary" FORCE)
36+
# Not always applicable, but so often needed that they're provided
37+
# by default: (TODO: clean this up)
38+
if(DEFINED CMAKE_GDB)
39+
set(ZEPHYR_RUNNER_CONFIG_GDB "${CMAKE_GDB}"
40+
CACHE STRING "Path to GDB binary, if applicable" FORCE)
41+
endif()
42+
if(DEFINED OPENOCD)
43+
set(ZEPHYR_RUNNER_CONFIG_OPENOCD "${OPENOCD}"
44+
CACHE STRING "Path to openocd binary, if applicable" FORCE)
45+
endif()
46+
if(DEFINED OPENOCD_DEFAULT_PATH)
47+
set(ZEPHYR_RUNNER_CONFIG_OPENOCD_SEARCH "${OPENOCD_DEFAULT_PATH}"
48+
CACHE STRING "Path to add to openocd search path, if applicable" FORCE)
49+
endif()
50+
51+
# Runner-specific command line arguments obtained from the board's
52+
# build scripts, the application's scripts, etc.
3853
foreach(runner ${RUNNERS})
3954
string(MAKE_C_IDENTIFIER ${runner} runner_id)
4055
# E.g. args = BOARD_RUNNER_ARGS_openocd, BOARD_RUNNER_ARGS_dfu_util, etc.

scripts/meta/west/cmd/run_common.py

Lines changed: 110 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from .. import cmake
1414
from .. import log
1515
from ..runner import get_runner_cls
16+
from ..runner.core import RunnerConfig
1617
from . import CommandContextError
1718

1819

@@ -22,24 +23,56 @@ def add_parser_common(parser_adder, command):
2223
formatter_class=argparse.RawDescriptionHelpFormatter,
2324
description=command.description)
2425

25-
parser.add_argument('-d', '--build-dir',
26-
help='''Build directory to obtain runner information
27-
from; default is the current working directory.''')
28-
parser.add_argument('-c', '--cmake-cache', default=cmake.DEFAULT_CACHE,
29-
help='''Path to CMake cache file containing runner
30-
configuration (this is generated by the Zephyr
31-
build system when compiling binaries);
32-
default: {}.
33-
34-
If this is a relative path, it is assumed relative to
35-
the build directory. An absolute path can also be
36-
given instead.'''.format(cmake.DEFAULT_CACHE))
37-
parser.add_argument('-r', '--runner',
38-
help='''If given, overrides any cached {}
39-
runner.'''.format(command.name))
40-
parser.add_argument('--skip-rebuild', action='store_true',
41-
help='''If given, do not rebuild the application
42-
before running {} commands.'''.format(command.name))
26+
group = parser.add_argument_group(title='General Options')
27+
group.add_argument('-d', '--build-dir',
28+
help='''Build directory to obtain runner information
29+
from; default is the current working directory.''')
30+
group.add_argument('-c', '--cmake-cache', default=cmake.DEFAULT_CACHE,
31+
help='''Path to CMake cache file containing runner
32+
configuration (this is generated by the Zephyr
33+
build system when compiling binaries);
34+
default: {}.
35+
36+
If this is a relative path, it is assumed relative to
37+
the build directory. An absolute path can also be
38+
given instead.'''.format(cmake.DEFAULT_CACHE))
39+
group.add_argument('-r', '--runner',
40+
help='''If given, overrides any cached {}
41+
runner.'''.format(command.name))
42+
group.add_argument('--skip-rebuild', action='store_true',
43+
help='''If given, do not rebuild the application
44+
before running {} commands.'''.format(command.name))
45+
46+
group = parser.add_argument_group(
47+
title='Configuration overrides',
48+
description=dedent('''\
49+
These values usually come from the Zephyr build system itself
50+
as stored in the CMake cache; providing these options
51+
overrides those settings.'''))
52+
53+
# Important:
54+
#
55+
# 1. The destination variables of these options must match
56+
# the RunnerConfig slots.
57+
# 2. The default values for all of these must be None.
58+
#
59+
# This is how we detect if the user provided them or not when
60+
# overriding values from the cached configuration.
61+
group.add_argument('--board-dir',
62+
help='Zephyr board directory')
63+
group.add_argument('--kernel-elf',
64+
help='Path to kernel binary in .elf format')
65+
group.add_argument('--kernel-hex',
66+
help='Path to kernel binary in .hex format')
67+
group.add_argument('--kernel-bin',
68+
help='Path to kernel binary in .bin format')
69+
group.add_argument('--gdb',
70+
help='Path to GDB, if applicable')
71+
group.add_argument('--openocd',
72+
help='Path to OpenOCD, if applicable')
73+
group.add_argument(
74+
'--openocd-search',
75+
help='Path to add to OpenOCD search path, if applicable')
4376

4477
return parser
4578

@@ -57,6 +90,30 @@ def desc_common(command_name):
5790
'''.format(**{'command': command_name}))
5891

5992

93+
def cached_runner_config(cache):
94+
'''Parse the RunnerConfig from a CMake Cache.'''
95+
board_dir = cache['ZEPHYR_RUNNER_CONFIG_BOARD_DIR']
96+
kernel_elf = cache['ZEPHYR_RUNNER_CONFIG_KERNEL_ELF']
97+
kernel_hex = cache['ZEPHYR_RUNNER_CONFIG_KERNEL_HEX']
98+
kernel_bin = cache['ZEPHYR_RUNNER_CONFIG_KERNEL_BIN']
99+
gdb = cache.get('ZEPHYR_RUNNER_CONFIG_GDB')
100+
openocd = cache.get('ZEPHYR_RUNNER_CONFIG_OPENOCD')
101+
openocd_search = cache.get('ZEPHYR_RUNNER_CONFIG_OPENOCD_SEARCH')
102+
103+
return RunnerConfig(board_dir, kernel_elf, kernel_hex, kernel_bin,
104+
gdb=gdb, openocd=openocd,
105+
openocd_search=openocd_search)
106+
107+
108+
def _override_config_from_namespace(cfg, namespace):
109+
'''Override a RunnerConfig's contents with command-line values.'''
110+
for var in cfg.__slots__:
111+
if var in namespace:
112+
val = getattr(namespace, var)
113+
if val is not None:
114+
setattr(cfg, var, val)
115+
116+
60117
def do_run_common(command, args, runner_args, cached_runner_var):
61118
command_name = command.name
62119
build_dir = args.build_dir or getcwd()
@@ -79,6 +136,12 @@ def do_run_common(command, args, runner_args, cached_runner_var):
79136
# places.
80137
chdir(build_dir)
81138

139+
# Runner creation, phase 1.
140+
#
141+
# Get the default runner name from the cache, allowing a command
142+
# line override. Get the ZephyrBinaryRunner class by name, and
143+
# make sure it supports the command.
144+
82145
# TODO: build this by joining with build_dir once the above chdir
83146
# goes away.
84147
cache_file = args.cmake_cache
@@ -104,22 +167,38 @@ def do_run_common(command, args, runner_args, cached_runner_var):
104167
log.die('Runner {} does not support command {}'.format(
105168
runner, command_name))
106169

107-
cached_common_args = cache.get_list('ZEPHYR_RUNNER_ARGS_COMMON')
170+
# Runner creation, phase 2.
171+
#
172+
# At this point, the common options above are already parsed in
173+
# 'args', and unrecognized arguments are in 'runner_args'.
174+
#
175+
# - Pull the RunnerConfig out of the cache
176+
# - Override cached values with applicable command-line options
177+
178+
cfg = cached_runner_config(cache)
179+
_override_config_from_namespace(cfg, args)
180+
181+
# Runner creation, phase 3.
182+
#
183+
# - Pull out cached runner arguments, and append command-line
184+
# values (which should override the cache)
185+
# - Construct a runner-specific argument parser to handle cached
186+
# values plus overrides given in runner_args
187+
# - Parse arguments and create runner instance from final
188+
# RunnerConfig and parsed arguments.
189+
108190
cached_runner_args = cache.get_list(
109191
'ZEPHYR_RUNNER_ARGS_{}'.format(cmake.make_c_identifier(runner)))
110-
# Construct the final command line arguments, create a
111-
# runner-specific parser to handle them, and run the command.
112192
assert isinstance(runner_args, list), runner_args
113-
final_runner_args = cached_common_args + cached_runner_args + runner_args
114-
115-
# Having the runners themselves be the place where their argument
116-
# parsing is handled is hackish; it's an artifact of the time
117-
# before the runner package was part of west.
118-
#
119-
# TODO: refactor runner argument parsing higher up into west.
193+
# If the user passed -- to force the parent argument parser to stop
194+
# parsing, it will show up here, and needs to be filtered out.
195+
runner_args = [arg for arg in runner_args if arg != '--']
196+
final_runner_args = cached_runner_args + runner_args
120197
parser = argparse.ArgumentParser(prog=runner)
121198
runner_cls.add_parser(parser)
122-
parsed_args = parser.parse_args(args=final_runner_args)
123-
parsed_args.verbose = args.verbose
124-
runner = runner_cls.create_from_args(parsed_args)
199+
parsed_args, unknown = parser.parse_known_args(args=final_runner_args)
200+
if unknown:
201+
raise CommandContextError('Runner', runner,
202+
'received unknown arguments', unknown)
203+
runner = runner_cls.create(cfg, parsed_args)
125204
runner.run(command_name)

scripts/meta/west/runner/arc.py

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,16 @@ class EmStarterKitBinaryRunner(ZephyrBinaryRunner):
2424
# client to execute the application.
2525
#
2626

27-
def __init__(self, board_dir, elf, gdb,
28-
openocd='openocd', search=None,
27+
def __init__(self, cfg,
2928
tui=False, tcl_port=DEFAULT_ARC_TCL_PORT,
3029
telnet_port=DEFAULT_ARC_TELNET_PORT,
31-
gdb_port=DEFAULT_ARC_GDB_PORT, debug=False):
32-
super(EmStarterKitBinaryRunner, self).__init__(debug=debug)
33-
self.board_dir = board_dir
34-
self.elf = elf
35-
self.gdb_cmd = [gdb] + (['-tui'] if tui else [])
30+
gdb_port=DEFAULT_ARC_GDB_PORT):
31+
super(EmStarterKitBinaryRunner, self).__init__(cfg)
32+
self.gdb_cmd = [cfg.gdb] + (['-tui'] if tui else [])
3633
search_args = []
37-
if search is not None:
38-
search_args = ['-s', search]
39-
self.openocd_cmd = [openocd] + search_args
34+
if cfg.openocd_search is not None:
35+
search_args = ['-s', cfg.openocd_search]
36+
self.openocd_cmd = [cfg.openocd or 'openocd'] + search_args
4037
self.tcl_port = tcl_port
4138
self.telnet_port = telnet_port
4239
self.gdb_port = gdb_port
@@ -57,18 +54,17 @@ def do_add_parser(cls, parser):
5754
help='openocd gdb port, defaults to 3333')
5855

5956
@classmethod
60-
def create_from_args(cls, args):
61-
if args.gdb is None:
57+
def create(cls, cfg, args):
58+
if cfg.gdb is None:
6259
raise ValueError('--gdb not provided at command line')
6360

6461
return EmStarterKitBinaryRunner(
65-
args.board_dir, args.kernel_elf, args.gdb,
66-
openocd=args.openocd, search=args.openocd_search,
62+
cfg,
6763
tui=args.tui, tcl_port=args.tcl_port, telnet_port=args.telnet_port,
68-
gdb_port=args.gdb_port, debug=args.verbose)
64+
gdb_port=args.gdb_port)
6965

7066
def do_run(self, command, **kwargs):
71-
kwargs['openocd-cfg'] = path.join(self.board_dir, 'support',
67+
kwargs['openocd-cfg'] = path.join(self.cfg.board_dir, 'support',
7268
'openocd.cfg')
7369

7470
if command in {'flash', 'debug'}:
@@ -97,7 +93,7 @@ def flash_debug(self, command, **kwargs):
9793
['-ex', 'target remote :{}'.format(self.gdb_port),
9894
'-ex', 'load'] +
9995
continue_arg +
100-
[self.elf])
96+
[self.cfg.kernel_elf])
10197

10298
self.run_server_and_client(server_cmd, gdb_cmd)
10399

scripts/meta/west/runner/bossac.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,8 @@
1414
class BossacBinaryRunner(ZephyrBinaryRunner):
1515
'''Runner front-end for bossac.'''
1616

17-
def __init__(self, bin_name, bossac='bossac',
18-
port=DEFAULT_BOSSAC_PORT, debug=False):
19-
super(BossacBinaryRunner, self).__init__(debug=debug)
20-
self.bin_name = bin_name
17+
def __init__(self, cfg, bossac='bossac', port=DEFAULT_BOSSAC_PORT):
18+
super(BossacBinaryRunner, self).__init__(cfg)
2119
self.bossac = bossac
2220
self.port = port
2321

@@ -37,9 +35,9 @@ def do_add_parser(cls, parser):
3735
help='serial port to use, default is /dev/ttyACM0')
3836

3937
@classmethod
40-
def create_from_args(command, args):
41-
return BossacBinaryRunner(args.kernel_bin, bossac=args.bossac,
42-
port=args.bossac_port, debug=args.verbose)
38+
def create(cls, cfg, args):
39+
return BossacBinaryRunner(cfg, bossac=args.bossac,
40+
port=args.bossac_port)
4341

4442
def do_run(self, command, **kwargs):
4543
if platform.system() != 'Linux':
@@ -50,7 +48,7 @@ def do_run(self, command, **kwargs):
5048
'ospeed', '1200', 'cs8', '-cstopb', 'ignpar', 'eol', '255',
5149
'eof', '255']
5250
cmd_flash = [self.bossac, '-p', self.port, '-R', '-e', '-w', '-v',
53-
'-b', self.bin_name]
51+
'-b', self.cfg.kernel_bin]
5452

5553
self.check_call(cmd_stty)
5654
self.check_call(cmd_flash)

0 commit comments

Comments
 (0)