From f8e8e9229d4ba9d11ae0888d3d52f5b45dfc955c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=AD=20Bol=C3=ADvar?= Date: Tue, 23 Jun 2020 13:35:52 -0700 Subject: [PATCH] runners: enforce RunnerCaps via create() indirection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Require all implementations to provide a do_create(), a new ZephyrBinaryRunner abstract class method, and make create() itself concrete. This allows us to enforce common conventions related to individual runner capabilities as each runner provides to the core via RunnerCaps. For now, just enforce that: - common options related to capabilities are always added, so runners can't reuse them for different ends - common options provided for runners which don't support them emit sensible error messages that should be easy to diagnose and support Signed-off-by: Martí Bolívar --- scripts/west_commands/run_common.py | 2 +- .../west_commands/runners/blackmagicprobe.py | 2 +- scripts/west_commands/runners/bossac.py | 2 +- .../west_commands/runners/canopen_program.py | 2 +- scripts/west_commands/runners/core.py | 36 ++++++++++++++++--- scripts/west_commands/runners/dediprog.py | 2 +- scripts/west_commands/runners/dfu.py | 2 +- scripts/west_commands/runners/esp32.py | 2 +- scripts/west_commands/runners/hifive1.py | 2 +- scripts/west_commands/runners/intel_s1000.py | 2 +- scripts/west_commands/runners/jlink.py | 2 +- scripts/west_commands/runners/mdb.py | 2 +- scripts/west_commands/runners/misc.py | 2 +- scripts/west_commands/runners/nios2.py | 2 +- scripts/west_commands/runners/nrfjprog.py | 2 +- scripts/west_commands/runners/nsim.py | 2 +- scripts/west_commands/runners/openocd.py | 2 +- scripts/west_commands/runners/pyocd.py | 2 +- scripts/west_commands/runners/qemu.py | 2 +- scripts/west_commands/runners/stm32flash.py | 2 +- scripts/west_commands/runners/xtensa.py | 2 +- 21 files changed, 52 insertions(+), 24 deletions(-) diff --git a/scripts/west_commands/run_common.py b/scripts/west_commands/run_common.py index 8f6eec159c7..d1ae9c47951 100644 --- a/scripts/west_commands/run_common.py +++ b/scripts/west_commands/run_common.py @@ -210,8 +210,8 @@ def do_run_common(command, user_args, user_runner_args): # # Use that RunnerConfig to create the ZephyrBinaryRunner instance # and call its run(). - runner = runner_cls.create(runner_cfg_from_args(args, build_dir), args) try: + runner = runner_cls.create(runner_cfg_from_args(args, build_dir), args) runner.run(command_name) except ValueError as ve: log.err(str(ve), fatal=True) diff --git a/scripts/west_commands/runners/blackmagicprobe.py b/scripts/west_commands/runners/blackmagicprobe.py index 1544d9737b7..74e5859a765 100644 --- a/scripts/west_commands/runners/blackmagicprobe.py +++ b/scripts/west_commands/runners/blackmagicprobe.py @@ -27,7 +27,7 @@ class BlackMagicProbeRunner(ZephyrBinaryRunner): return RunnerCaps(commands={'flash', 'debug', 'attach'}) @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): return BlackMagicProbeRunner(cfg, args.gdb_serial) @classmethod diff --git a/scripts/west_commands/runners/bossac.py b/scripts/west_commands/runners/bossac.py index 6f0062b1afa..e5b8d57febe 100644 --- a/scripts/west_commands/runners/bossac.py +++ b/scripts/west_commands/runners/bossac.py @@ -41,7 +41,7 @@ class BossacBinaryRunner(ZephyrBinaryRunner): help='serial port to use, default is /dev/ttyACM0') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): return BossacBinaryRunner(cfg, bossac=args.bossac, port=args.bossac_port, offset=args.offset) diff --git a/scripts/west_commands/runners/canopen_program.py b/scripts/west_commands/runners/canopen_program.py index 3dc02e0a67f..13431e0466a 100644 --- a/scripts/west_commands/runners/canopen_program.py +++ b/scripts/west_commands/runners/canopen_program.py @@ -87,7 +87,7 @@ class CANopenBinaryRunner(ZephyrBinaryRunner): parser.set_defaults(confirm=True) @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): return CANopenBinaryRunner(cfg, int(args.node_id), can_context=args.can_context, program_number=int(args.program_number), diff --git a/scripts/west_commands/runners/core.py b/scripts/west_commands/runners/core.py index 2fd9a7a6a99..891b493341c 100644 --- a/scripts/west_commands/runners/core.py +++ b/scripts/west_commands/runners/core.py @@ -199,6 +199,16 @@ class RunnerCaps: self.commands, self.flash_addr) +def _missing_cap(cls, option): + # Helper function that's called when an option was given on the + # command line that corresponds to a missing capability. + # + # 'cls' is a ZephyrBinaryRunner subclass; 'option' is an option + # that can't be supported due to missing capability. + + raise ValueError(f"{cls.name()} doesn't support {option} option") + + class RunnerConfig: '''Runner execution-time configuration. @@ -260,7 +270,9 @@ class _DTFlashAction(argparse.Action): class ZephyrBinaryRunner(abc.ABC): '''Abstract superclass for binary runners (flashers, debuggers). - **Note**: these APIs are still evolving, and will change! + **Note**: this class's API has changed relatively rarely since it + as added, but it is not considered a stable Zephyr API, and may change + without notice. With some exceptions, boards supported by Zephyr must provide generic means to be flashed (have a Zephyr firmware binary @@ -389,13 +401,20 @@ class ZephyrBinaryRunner(abc.ABC): Runner-specific options are added through the do_add_parser() hook.''' - # Common options that depend on runner capabilities. - if cls.capabilities().flash_addr: + # Common options that depend on runner capabilities. If a + # capability is not supported, the option string or strings + # are added anyway, to prevent an individual runner class from + # using them to mean something else. + caps = cls.capabilities() + + if caps.flash_addr: parser.add_argument('--dt-flash', default='n', choices=_YN_CHOICES, action=_DTFlashAction, help='''If 'yes', use configuration generated by device tree (DT) to compute flash addresses.''') + else: + parser.add_argument('--dt-flash', help=argparse.SUPPRESS) # Runner-specific options. cls.do_add_parser(parser) @@ -406,13 +425,22 @@ class ZephyrBinaryRunner(abc.ABC): '''Hook for adding runner-specific options.''' @classmethod - @abc.abstractmethod def create(cls, cfg, args): '''Create an instance from command-line arguments. - ``cfg``: RunnerConfig instance (pass to superclass __init__) - ``args``: runner-specific argument namespace parsed from execution environment, as specified by ``add_parser()``.''' + caps = cls.capabilities() + if args.dt_flash and not caps.flash_addr: + _missing_cap(cls, '--dt-flash') + + return cls.do_create(cfg, args) + + @classmethod + @abc.abstractmethod + def do_create(cls, cfg, args): + '''Hook for instance creation from command line arguments.''' @classmethod def get_flash_address(cls, args, build_conf, default=0x0): diff --git a/scripts/west_commands/runners/dediprog.py b/scripts/west_commands/runners/dediprog.py index ed438c46d9a..4c514efca27 100644 --- a/scripts/west_commands/runners/dediprog.py +++ b/scripts/west_commands/runners/dediprog.py @@ -40,7 +40,7 @@ class DediProgBinaryRunner(ZephyrBinaryRunner): help='Number of retries (default 5)') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): return DediProgBinaryRunner(cfg, spi_image=args.spi_image, vcc=args.vcc, diff --git a/scripts/west_commands/runners/dfu.py b/scripts/west_commands/runners/dfu.py index 6a43e33f259..fbd2816be96 100644 --- a/scripts/west_commands/runners/dfu.py +++ b/scripts/west_commands/runners/dfu.py @@ -74,7 +74,7 @@ class DfuUtilBinaryRunner(ZephyrBinaryRunner): help='dfu-util executable; defaults to "dfu-util"') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): if args.img is None: args.img = cfg.bin_file diff --git a/scripts/west_commands/runners/esp32.py b/scripts/west_commands/runners/esp32.py index 6b1d062f8dd..9d17970c91f 100644 --- a/scripts/west_commands/runners/esp32.py +++ b/scripts/west_commands/runners/esp32.py @@ -63,7 +63,7 @@ class Esp32BinaryRunner(ZephyrBinaryRunner): help='Partition table to flash') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): if args.esp_tool: espidf = args.esp_tool else: diff --git a/scripts/west_commands/runners/hifive1.py b/scripts/west_commands/runners/hifive1.py index 5457cd8918e..1a72d3018e1 100644 --- a/scripts/west_commands/runners/hifive1.py +++ b/scripts/west_commands/runners/hifive1.py @@ -29,7 +29,7 @@ class HiFive1BinaryRunner(ZephyrBinaryRunner): pass @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): if cfg.gdb is None: raise ValueError('--gdb not provided at command line') diff --git a/scripts/west_commands/runners/intel_s1000.py b/scripts/west_commands/runners/intel_s1000.py index 14b716b00ff..3c17c50abdc 100644 --- a/scripts/west_commands/runners/intel_s1000.py +++ b/scripts/west_commands/runners/intel_s1000.py @@ -53,7 +53,7 @@ class IntelS1000BinaryRunner(ZephyrBinaryRunner): help='xt-gdb port, defaults to 20000') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): return IntelS1000BinaryRunner( cfg, args.xt_ocd_dir, args.ocd_topology, args.ocd_jtag_instr, args.gdb_flash_file, diff --git a/scripts/west_commands/runners/jlink.py b/scripts/west_commands/runners/jlink.py index 8a5fd36522b..5c21cf1d5e2 100644 --- a/scripts/west_commands/runners/jlink.py +++ b/scripts/west_commands/runners/jlink.py @@ -90,7 +90,7 @@ class JLinkBinaryRunner(ZephyrBinaryRunner): parser.set_defaults(reset_after_load=False) @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): build_conf = BuildConfiguration(cfg.build_dir) flash_addr = cls.get_flash_address(args, build_conf) return JLinkBinaryRunner(cfg, args.device, diff --git a/scripts/west_commands/runners/mdb.py b/scripts/west_commands/runners/mdb.py index be53a24ff37..669ef63fbff 100644 --- a/scripts/west_commands/runners/mdb.py +++ b/scripts/west_commands/runners/mdb.py @@ -58,7 +58,7 @@ class MdbBinaryRunner(ZephyrBinaryRunner): targets are connected''') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): return MdbBinaryRunner( cfg, cores=args.cores, diff --git a/scripts/west_commands/runners/misc.py b/scripts/west_commands/runners/misc.py index e429b39cb4b..814bb0a961e 100644 --- a/scripts/west_commands/runners/misc.py +++ b/scripts/west_commands/runners/misc.py @@ -43,7 +43,7 @@ class MiscFlasher(ZephyrBinaryRunner): directory''') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): return MiscFlasher(cfg, args.cmd, args.args) def do_run(self, *args, **kwargs): diff --git a/scripts/west_commands/runners/nios2.py b/scripts/west_commands/runners/nios2.py index 8aa3bc1c07f..5a6616e795c 100644 --- a/scripts/west_commands/runners/nios2.py +++ b/scripts/west_commands/runners/nios2.py @@ -41,7 +41,7 @@ class Nios2BinaryRunner(ZephyrBinaryRunner): help='if given, GDB uses -tui') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): return Nios2BinaryRunner(cfg, quartus_py=args.quartus_flash, cpu_sof=args.cpu_sof, diff --git a/scripts/west_commands/runners/nrfjprog.py b/scripts/west_commands/runners/nrfjprog.py index 5eca671911d..9394f79e7fe 100644 --- a/scripts/west_commands/runners/nrfjprog.py +++ b/scripts/west_commands/runners/nrfjprog.py @@ -53,7 +53,7 @@ class NrfJprogBinaryRunner(ZephyrBinaryRunner): e.g. "--recover"''') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): return NrfJprogBinaryRunner(cfg, args.nrf_family, args.softreset, args.snr, erase=args.erase, tool_opt=args.tool_opt) diff --git a/scripts/west_commands/runners/nsim.py b/scripts/west_commands/runners/nsim.py index 7d242e387b7..002e2dac0c1 100644 --- a/scripts/west_commands/runners/nsim.py +++ b/scripts/west_commands/runners/nsim.py @@ -45,7 +45,7 @@ class NsimBinaryRunner(ZephyrBinaryRunner): help='nsim props file, defaults to nsim.props') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): if cfg.gdb is None: raise ValueError('--gdb not provided at command line') diff --git a/scripts/west_commands/runners/openocd.py b/scripts/west_commands/runners/openocd.py index 35a15079b28..a3049be24b5 100644 --- a/scripts/west_commands/runners/openocd.py +++ b/scripts/west_commands/runners/openocd.py @@ -98,7 +98,7 @@ class OpenOcdBinaryRunner(ZephyrBinaryRunner): help='openocd gdb port, defaults to 3333') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): return OpenOcdBinaryRunner( cfg, pre_init=args.cmd_pre_init, diff --git a/scripts/west_commands/runners/pyocd.py b/scripts/west_commands/runners/pyocd.py index 429698aab63..34138def16b 100644 --- a/scripts/west_commands/runners/pyocd.py +++ b/scripts/west_commands/runners/pyocd.py @@ -95,7 +95,7 @@ class PyOcdBinaryRunner(ZephyrBinaryRunner): e.g. \'--script=user.py\' ''') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): build_conf = BuildConfiguration(cfg.build_dir) flash_addr = cls.get_flash_address(args, build_conf) diff --git a/scripts/west_commands/runners/qemu.py b/scripts/west_commands/runners/qemu.py index de417dcbe89..7609cbb48bc 100644 --- a/scripts/west_commands/runners/qemu.py +++ b/scripts/west_commands/runners/qemu.py @@ -24,7 +24,7 @@ class QemuBinaryRunner(ZephyrBinaryRunner): pass # Nothing to do. @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): return QemuBinaryRunner(cfg) def do_run(self, command, **kwargs): diff --git a/scripts/west_commands/runners/stm32flash.py b/scripts/west_commands/runners/stm32flash.py index b281ffb2f38..e8eb4407178 100644 --- a/scripts/west_commands/runners/stm32flash.py +++ b/scripts/west_commands/runners/stm32flash.py @@ -79,7 +79,7 @@ class Stm32flashBinaryRunner(ZephyrBinaryRunner): help='verify writes, default False') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): return Stm32flashBinaryRunner(cfg, device=args.device, action=args.action, baud=args.baud_rate, force_binary=args.force_binary, start_addr=args.start_addr, exec_addr=args.execution_addr, diff --git a/scripts/west_commands/runners/xtensa.py b/scripts/west_commands/runners/xtensa.py index d77b18e0f66..3fcf17d6ac1 100644 --- a/scripts/west_commands/runners/xtensa.py +++ b/scripts/west_commands/runners/xtensa.py @@ -26,7 +26,7 @@ class XtensaBinaryRunner(ZephyrBinaryRunner): help='path to XTensa tools') @classmethod - def create(cls, cfg, args): + def do_create(cls, cfg, args): # Override any GDB with the one provided by the XTensa tools. cfg.gdb = path.join(args.xcc_tools, 'bin', 'xt-gdb') return XtensaBinaryRunner(cfg)