diff --git a/better_launch/declarative.py b/better_launch/declarative.py index 5e4b79c..11d286e 100644 --- a/better_launch/declarative.py +++ b/better_launch/declarative.py @@ -111,7 +111,10 @@ def exec_request(key: str, req: dict) -> Any: with res: if isinstance(children, list): - children = {f"{key}.children.{idx}": child for idx, child in enumerate(children)} + children = { + f"{key}.children.{idx}": child + for idx, child in enumerate(children) + } if not isinstance(children, dict): raise ValueError( @@ -174,6 +177,7 @@ def _get_toml_args(toml: dict) -> list[DeclaredArg]: def launch_toml( path: str, launch_args: dict[str, str] = None, + extra_args: list[str] = None, *, # These should largely mirror the launch_this decorator ui: bool = None, @@ -204,7 +208,7 @@ def launch_toml( executable = "my-node" name = "${name}" - Substitutions are also possible and use a similar syntax as in ROS1 (as shown for the `name` launch argument above). `if` and `unless` conditions can be added as well. + Substitutions are also possible and use a similar syntax as in ROS1 (as shown for the `name` launch argument above). `if` and `unless` conditions can be added as well. All parameters below can be set through the launch file by declaring them on the global scope with a `bl_` prefix (i.e. `ui` becomes `bl_ui`). @@ -310,6 +314,11 @@ def launch_toml( if arg is not None: argv.extend([f"--{key}", str(arg)]) + if extra_args: + if argv is None: + argv = [] + argv.extend(extra_args) + def launch_func(*args, **kwargs): _execute_toml(toml, eval_mode=eval_mode, **kwargs) diff --git a/better_launch/launcher.py b/better_launch/launcher.py index 2d02af0..24df39c 100644 --- a/better_launch/launcher.py +++ b/better_launch/launcher.py @@ -117,6 +117,7 @@ class BetterLaunch(metaclass=BetterLaunchMeta): _launchfile: str = None _launch_func_args: dict[str, Any] = {} + _node_param_overrides: dict[str, dict[str, Any]] = {} def __init__( self, @@ -224,6 +225,9 @@ def hello(self) -> None: # We don't want to log this print(msg) + def set_node_param_overrides(self, overrides: dict[str, dict[str, Any]]) -> None: + BetterLaunch._node_param_overrides = overrides or {} + def spin(self, exit_with_last_node: bool = True) -> None: """Join the BetterLaunch thread until it terminates. You do **not** need to call this if you're using the [launch_this][] wrapper or the TUI. @@ -1506,13 +1510,26 @@ def node( group = self.group_tip namespace = group.assemble_namespace() + # Merge CLI overrides (CLI takes precedence) + resolved_params: dict[str, Any] + if isinstance(params, str): + resolved_params = self.load_params(configfile=params) + elif params is None: + resolved_params = {} + else: + resolved_params = params + + cli_overrides = self._node_param_overrides.get(name, {}) + if cli_overrides: + resolved_params = {**resolved_params, **cli_overrides} + node = Node( package, executable, name, namespace, remaps=remaps, - params=params, + params=resolved_params, cmd_args=cmd_args, env=env, isolate_env=isolate_env, diff --git a/better_launch/utils/better_logging.py b/better_launch/utils/better_logging.py index 831e851..79e7473 100644 --- a/better_launch/utils/better_logging.py +++ b/better_launch/utils/better_logging.py @@ -13,7 +13,7 @@ # Log format string for ROS so that we can identify and reformat its log messages. ROSLOG_PATTERN_ROS = "%%{severity}%%{time}%%{message}" -# Regular expression matching ROSLOG_PATTERN_ROS. The named groups will be matched to +# Regular expression matching ROSLOG_PATTERN_ROS. The named groups will be matched to # logging.LogRecord attributes via their group names. ROSLOG_PATTERN_BL = r"%%(?P\w+)%%(?P[\d.]+)%%(?P[\s\S]*)" @@ -221,15 +221,11 @@ def format(self, record: logging.LogRecord) -> str: record.levelno ) - msg = record.getMessage() - if self.max_message_length > 0 and len(msg) > self.max_message_length: - msg = msg[: self.max_message_length] + "..." - record.msg = msg - # The message has already been formatted with its arguments above. - # Clearing record.args prevents the next formatter from attempting - # a second '%' substitution on the truncated text, which could crash - # if any format placeholders were removed during truncation. - record.args = None + if self.max_message_length > 0: + msg = record.getMessage() + if msg > self.max_message_length: + record.msg = msg[: self.max_message_length] + "…" + return super().format(record) @@ -302,26 +298,25 @@ def configure_logger( screen_formatter: logging.Formatter = None, file_formatter: logging.Formatter = None, ) -> None: - """Initialize the logging framework. - """ + """Initialize the logging framework.""" # TODO proper docstring if output: - if isinstance(output, Iterable) and not isinstance(output, str): - output = [output] + if isinstance(output, str) or not isinstance(output, Iterable): + sinks = [output] + else: + sinks = list(output) - for idx, sink in output: - if isinstance(sink, str): - output[idx] = LogSink[output.upper()] - - output = set(output) + output_sinks = set( + LogSink[sink.upper()] if isinstance(sink, str) else sink for sink in sinks + ) else: - output = {LogSink.SCREEN} + output_sinks = {LogSink.SCREEN} config = Settings() screen_filter = LevelFilter(config.screen_log_level) file_filter = LevelFilter(config.file_log_level) - for sink in output: + for sink in output_sinks: if sink == LogSink.SCREEN: screen_handler = roslog.launch_config.get_screen_handler() if screen_handler not in logger.handlers: diff --git a/better_launch/utils/click.py b/better_launch/utils/click.py index 7277d51..9331372 100644 --- a/better_launch/utils/click.py +++ b/better_launch/utils/click.py @@ -1,4 +1,5 @@ from typing import Any, Type, Iterable, Callable +import json from dataclasses import dataclass import click @@ -53,20 +54,46 @@ def get_click_bl_options(expose: bool = False) -> list[click.Option]: list[click.Option] _description_ """ + def update_value(ctx: click.Context, param: click.Parameter, value: Any): key = param.name[3:].replace("-", "_") - _update_settings(**{key: value}) + + if value is not None: + _update_settings(**{key: value}) + + if key == "node_param_override" and value: + ctx.command.allow_extra_args = True + ctx.allow_extra_args = True + + def _bool_param_type(value: str) -> bool: + """Convert string to boolean for Click options.""" + if isinstance(value, bool): + return value + val_lower = value.lower() + if val_lower in ("true", "1", "yes", "on"): + return True + if val_lower in ("false", "0", "no", "off"): + return False + raise click.BadParameter(f"Invalid boolean value: {value}") # XXX always keep these synchronized with our Settings class options = [ click.Option( ["--bl-ui"], - type=bool, + type=_bool_param_type, default=None, help="Enforce or prevent starting the TUI", expose_value=expose, # not passed to our run method callback=update_value, ), + click.Option( + ["--bl-node-param-override"], + type=_bool_param_type, + default=None, + help="Allow overriding node parameters from the command line", + expose_value=expose, + callback=update_value, + ), click.Option( ["--bl-colormode"], type=click.types.Choice([c.name for c in Colormode], case_sensitive=False), @@ -138,11 +165,88 @@ def get_click_launch_command( allow_kwargs: bool = False, ) -> click.Command: click_cmd = click.Command( - cmd_name, callback=launch_func, params=options, help=cmd_help + cmd_name, + callback=launch_func, + params=options, + help=cmd_help, + context_settings={"ignore_unknown_options": True}, ) - if allow_kwargs: - click_cmd.allow_extra_args = True - click_cmd.ignore_unknown_options = True + click_cmd.allow_extra_args = allow_kwargs + click_cmd.ignore_unknown_options = True return click_cmd + + +def args_to_dict(args: list[str]) -> dict[str, Any]: + """Convert a list of CLI arguments to a dictionary. + + Parameters + ---------- + args : list[str] + List of arguments, e.g. ["--foo", "bar", "--baz", "1.0"] + + Returns + ------- + dict[str, Any] + Dictionary of arguments, e.g. {"foo": "bar", "baz": 1.0} + + Raises + ------ + ValueError + If an argument does not start with "-" or if a value is missing. + """ + result = {} + it = iter(args) + for arg in it: + if not arg.startswith("-"): + raise ValueError(f"Argument '{arg}' does not start with '-'") + + key = arg.lstrip("-") + try: + value = next(it) + except StopIteration: + raise ValueError(f"Missing value for argument '{arg}'") + + try: + value = json.loads(value) + except (json.JSONDecodeError, TypeError): + pass + + result[key] = value + + return result + + +def parse_node_params( + kwargs: dict[str, Any], +) -> tuple[dict[str, Any], dict[str, dict[str, Any]]]: + """Separate node parameters from other keyword arguments. + + Node parameters are identified by containing a dot in their key. The part + before the first dot is considered the node name, the part after the dot + is the parameter name. + + Parameters + ---------- + kwargs : dict[str, Any] + Dictionary of keyword arguments. + + Returns + ------- + tuple[dict[str, Any], dict[str, dict[str, Any]]] + Remaining keyword arguments and a dictionary of node parameters. + """ + remaining_kwargs = {} + node_params = {} + + for key, value in kwargs.items(): + if "." in key: + node_name, param_name = key.split(".", 1) + if node_name not in node_params: + node_params[node_name] = {} + node_params[node_name][param_name] = value + else: + remaining_kwargs[key] = value + + return remaining_kwargs, node_params diff --git a/better_launch/utils/settings.py b/better_launch/utils/settings.py index 28abfd0..25128cd 100644 --- a/better_launch/utils/settings.py +++ b/better_launch/utils/settings.py @@ -46,6 +46,7 @@ def severity_to_loglevel(severity: str) -> int: @dataclass(frozen=True) class _Settings: ui: bool = False + node_param_override: bool = False colormode: Colormode = Colormode.DEFAULT print_limit: int = 0 screen_log_level: int = logging.INFO @@ -139,15 +140,15 @@ def get_env_variables(self) -> dict[str, Any]: return vars def as_dict(self) -> dict[str, Any]: - """Returns the settings as a dict. - """ + """Returns the settings as a dict.""" # A bit more comfortable than having to import dataclasses.asdict each time return asdict(self) + def _update_settings(**overrides) -> None: """Replace the _SETTINGS object with a new instance with updated values. Only non-None values are applied. - This should only be called right after the launch process has started. + This should only be called right after the launch process has started. Parameters ---------- diff --git a/better_launch/wrapper.py b/better_launch/wrapper.py index 48175c7..4c7f2ea 100644 --- a/better_launch/wrapper.py +++ b/better_launch/wrapper.py @@ -15,11 +15,19 @@ _bl_singleton_instance, _bl_include_args, ) -from better_launch.utils.settings import Colormode, Settings, _update_settings, default_screen_format, default_file_format +from better_launch.utils.settings import ( + Colormode, + Settings, + _update_settings, + default_screen_format, + default_file_format, +) from better_launch.utils.better_logging import init_logging from better_launch.utils.introspection import find_calling_frame from better_launch.utils.click import ( DeclaredArg, + parse_node_params, + args_to_dict, get_click_options, get_click_bl_options, get_click_launch_command, @@ -96,7 +104,7 @@ def launch_this( def decoration_helper(func): sig = inspect.signature(func) declared_args = _get_declared_args(sig, func.__doc__) - + func_doc = doc.parse(func.__doc__) argspec = inspect.getfullargspec(func) @@ -144,7 +152,9 @@ def sigterm_handler(sig, frame): signal.signal(signal.SIGQUIT, sigterm_handler) -def _get_declared_args(signature: inspect.Signature, docstring: str = None) -> list[DeclaredArg]: +def _get_declared_args( + signature: inspect.Signature, docstring: str = None +) -> list[DeclaredArg]: # Extract more fine-grained information from the docstring param_docstrings = {} if docstring: @@ -157,7 +167,7 @@ def _get_declared_args(signature: inspect.Signature, docstring: str = None) -> l for param in signature.parameters.values(): ptype = None default = DeclaredArg._undefined - + if param.annotation is not param.empty: ptype = param.annotation if ptype and isinstance(ptype, str): @@ -213,7 +223,11 @@ def _exec_launch_func( include_args: dict = glob[_bl_include_args] bl.logger.info(f"Including launch file: {includefile} (args={include_args})") - call_kw = {a.name: a.default for a in declared_args if a.default != DeclaredArg._undefined} + call_kw = { + a.name: a.default + for a in declared_args + if a.default != DeclaredArg._undefined + } for key, val in include_args.items(): if allow_kwargs or key in call_kw: @@ -222,7 +236,7 @@ def _exec_launch_func( launch_func(**call_kw) return - + # Get the filename of the original launchfile # At this point we know that we are the main launch file if launchfile: @@ -271,33 +285,30 @@ def _exec_launch_func( @click.pass_context def run(ctx: click.Context, *args, **kwargs): - init_logging(roslog.launch_config) + import sys - if allow_kwargs: - # If the launch func defines a **kwarg we can pass all extra arguments to it, with - # the caveat that these extra args need to be defined as `-[-] val` tuples. - assert ( - len(ctx.args) % 2 == 0 - ), f"extra arguments need to be '-- ' tuples ({ctx.args})" + init_logging(roslog.launch_config) - for i in range(0, len(ctx.args), 2): - key = ctx.args[i] - if not key.startswith("-"): - raise ValueError("Extra argument keys must start with a dash") + extra_args_dict = args_to_dict(ctx.args) + remaining_kwargs, node_overrides = parse_node_params(extra_args_dict) - val = ctx.args[i + 1] - try: - val = literal_eval(val) - except Exception: - # Keep val as a string - pass + if Settings().node_param_override and node_overrides: + BetterLaunch._node_param_overrides = node_overrides - kwargs[key.strip("-")] = val + # Handle remaining kwargs if allow_kwargs is enabled + if allow_kwargs and remaining_kwargs: + for key, value in remaining_kwargs.items(): + if isinstance(value, str): + try: + value = literal_eval(value) + except Exception: + pass + kwargs[key] = value # By default BetterLaunch has access to all arguments from its launch function BetterLaunch._launch_func_args = dict(kwargs) - # Wrap the launch function so we can do some preparation and cleanup tasks. + # Wrap the launch function so we can do some preparation and cleanup tasks. def launch_func_wrapper(): try: # Execute the launch function! @@ -339,9 +350,8 @@ def launch_func_wrapper(): allow_kwargs=allow_kwargs, ) - if allow_kwargs: + if allow_kwargs or Settings().node_param_override: click_cmd.allow_extra_args = True - click_cmd.ignore_unknown_options = True try: click_cmd.main(_argv) @@ -350,7 +360,9 @@ def launch_func_wrapper(): raise -def _expose_ros2_launch_function(launch_func: Callable, declared_args: list[DeclaredArg]): +def _expose_ros2_launch_function( + launch_func: Callable, declared_args: list[DeclaredArg] +): """Helper function that exposes a function decorated by launch_this so that it can be included by a regular ROS2 launch file. We achieve this by generating a `generate_launch_description` function and adding it to the module globals where the launch function is defined. Parameters diff --git a/bin/bl b/bin/bl index 0c794cd..af43146 100755 --- a/bin/bl +++ b/bin/bl @@ -147,11 +147,25 @@ class BetterLaunchPython(LaunchFile): flush=True, ) + # Filter out bl_* settings - these are BetterLaunch options, not launch args + launch_args = {k: v for k, v in kwargs.items() if not k.startswith("bl_")} + + # Pass current bl_* settings as CLI args to child process + from better_launch.utils.settings import Settings + + current_settings = Settings() + # First argument becomes argv[0], so should be the program name args = ["python3", self.filepath] - for key, arg in kwargs.items(): + for key, arg in launch_args.items(): if arg is not None: - args.extend([f"--{key}", arg]) + args.extend([f"--{key}", str(arg)]) + + if current_settings.node_param_override: + args.extend(["--bl-node-param-override", "true"]) + + # Pass through extra args (e.g., --node.param value for node overrides) + args.extend(ctx.args) # This does NOT return and will replace our executable with the new executable in the same process os.execvp("python3", [str(a) for a in args]) @@ -191,11 +205,13 @@ class BetterLaunchToml(LaunchFile): from better_launch.declarative import launch_toml + # Filter out bl_* settings - these are BetterLaunch options, not launch args + launch_args = {k: v for k, v in kwargs.items() if not k.startswith("bl_")} + # Since the toml files are not executable, we need something else to run them. We # could create another script with a main function, but I don't think there's much # of a point to this. - # TODO pass overrides? - launch_toml(self.filepath, launch_args=kwargs) + launch_toml(self.filepath, launch_args=launch_args, extra_args=ctx.args) class Ros2LaunchFile(LaunchFile): diff --git a/examples/13_node_param_overrides.launch.py b/examples/13_node_param_overrides.launch.py new file mode 100644 index 0000000..c2f1e67 --- /dev/null +++ b/examples/13_node_param_overrides.launch.py @@ -0,0 +1,33 @@ +#!/usr/bin/env python3 +""" +Example launch file demonstrating node parameter overrides from command line. + +By default, better_launch allows you to pass any arguments from the command line. +If you use the --bl-node-param-override flag, you can also override parameters +of specific nodes by using the . syntax. + +The node in this example declares 'period' parameter with declare_parameter(), so it can be +overridden via -p period:= when launching. + +Example usage: +-------------- +# Default period is 1.0s +bl better_launch 13_node_param_overrides.launch.py --bl-node-param-override true + +# Override period to 0.2s (fast publishing) +bl better_launch 13_node_param_overrides.launch.py --bl-node-param-override true --my_timer.period 0.2 +""" + +from better_launch import BetterLaunch, launch_this +import os + + +@launch_this +def node_param_overrides(): + bl = BetterLaunch() + script_dir = os.path.dirname(os.path.abspath(__file__)) + bl.node( + package=".", + executable=os.path.join(script_dir, "scripts", "timed_talker.py"), + name="my_timer", + ) diff --git a/examples/scripts/timed_talker.py b/examples/scripts/timed_talker.py new file mode 100755 index 0000000..00883b2 --- /dev/null +++ b/examples/scripts/timed_talker.py @@ -0,0 +1,38 @@ +#!/usr/bin/env python3 +"""Simple timed talker that outputs at a configurable rate.""" + +import rclpy +from rclpy.node import Node +from std_msgs.msg import String + + +class TimedTalker(Node): + def __init__(self): + super().__init__("my_timer") + self.declare_parameter("period", 1.0) + self._period = self.get_parameter("period").value + self.get_logger().info(f"Publishing with period={self._period}s") + self._pub = self.create_publisher(String, "chatter", 10) + self._timer = self.create_timer(self._period, self._callback) + self._count = 0 + + def _callback(self): + msg = String() + msg.data = f"Hello World: {self._count}" + self.get_logger().info(f'Publishing: "{msg.data}"') + self._pub.publish(msg) + self._count += 1 + + +def main(args=None): + rclpy.init(args=args) + try: + rclpy.spin(TimedTalker()) + except KeyboardInterrupt: + pass + finally: + rclpy.shutdown() + + +if __name__ == "__main__": + main() diff --git a/tests/test_click_utils.py b/tests/test_click_utils.py new file mode 100644 index 0000000..6a03d0b --- /dev/null +++ b/tests/test_click_utils.py @@ -0,0 +1,155 @@ +import sys +from types import ModuleType +from unittest.mock import MagicMock + + +# Mock rclpy and other ROS2 dependencies before importing better_launch +def mock_package(name): + m = ModuleType(name) + m.__path__ = [] + sys.modules[name] = m + return m + + +mock_rclpy = mock_package("rclpy") +mock_rclpy.Parameter = MagicMock() +sys.modules["rclpy.node"] = MagicMock() +sys.modules["rclpy.qos"] = MagicMock() +sys.modules["rclpy.action"] = MagicMock() +sys.modules["rclpy.parameter"] = MagicMock() +sys.modules["rclpy.executors"] = MagicMock() +sys.modules["rclpy.logging"] = MagicMock() +sys.modules["rclpy.signals"] = MagicMock() +sys.modules["rclpy.context"] = MagicMock() + +mock_package("ament_index_python") +sys.modules["ament_index_python.packages"] = MagicMock() + +mock_package("launch") +sys.modules["launch.actions"] = MagicMock() +sys.modules["launch.launch_description_sources"] = MagicMock() +sys.modules["launch.substitutions"] = MagicMock() +sys.modules["launch.conditions"] = MagicMock() +sys.modules["launch.event_handlers"] = MagicMock() +sys.modules["launch.events"] = MagicMock() + +mock_package("launch_ros") +sys.modules["launch_ros.actions"] = MagicMock() +sys.modules["launch_ros.substitutions"] = MagicMock() + +mock_package("lifecycle_msgs") +sys.modules["lifecycle_msgs.msg"] = MagicMock() +sys.modules["lifecycle_msgs.srv"] = MagicMock() + +mock_package("rcl_interfaces") +sys.modules["rcl_interfaces.msg"] = MagicMock() +sys.modules["rcl_interfaces.srv"] = MagicMock() + +mock_package("std_msgs") +sys.modules["std_msgs.msg"] = MagicMock() + +mock_package("std_srvs") +sys.modules["std_srvs.srv"] = MagicMock() + +mock_package("ros2param") +sys.modules["ros2param.api"] = MagicMock() + +mock_package("osrf_pycommon") +sys.modules["osrf_pycommon.process_utils"] = MagicMock() + +mock_click = mock_package("click") +mock_click.Option = MagicMock() +mock_click.Context = MagicMock() +mock_click.Parameter = MagicMock() +mock_click.Command = MagicMock() +mock_click.types = MagicMock() +mock_click.pass_context = lambda x: x + +mock_doc = mock_package("docstring_parser") +mock_doc.parse = MagicMock() + +import pytest +from better_launch.utils.click import parse_node_params, args_to_dict + + +def test_parse_node_params(): + # empty kwargs + assert parse_node_params({}) == ({}, {}) + + # no node params + kwargs = {"foo": "bar", "baz": 1} + assert parse_node_params(kwargs) == (kwargs, {}) + + # single node param + kwargs = {"node1.param1": "val1", "foo": "bar"} + assert parse_node_params(kwargs) == ({"foo": "bar"}, {"node1": {"param1": "val1"}}) + + # multiple params same node + kwargs = {"node1.param1": "val1", "node1.param2": 2} + assert parse_node_params(kwargs) == ({}, {"node1": {"param1": "val1", "param2": 2}}) + + # multiple nodes + kwargs = {"node1.p1": 1, "node2.p2": 2} + assert parse_node_params(kwargs) == ({}, {"node1": {"p1": 1}, "node2": {"p2": 2}}) + + # mixed kwargs + kwargs = {"node1.p1": 1, "foo": "bar", "node2.p2": 2, "baz": 3} + assert parse_node_params(kwargs) == ( + {"foo": "bar", "baz": 3}, + {"node1": {"p1": 1}, "node2": {"p2": 2}}, + ) + + # various value types + kwargs = { + "n.s": "string", + "n.f": 1.5, + "n.b": True, + "n.l": [1, 2, 3], + "n.d": {"a": 1}, + } + assert parse_node_params(kwargs) == ( + {}, + {"n": {"s": "string", "f": 1.5, "b": True, "l": [1, 2, 3], "d": {"a": 1}}}, + ) + + # nested param names + kwargs = {"node1.param.subparam": "val"} + assert parse_node_params(kwargs) == ({}, {"node1": {"param.subparam": "val"}}) + + +def test_args_to_dict(): + # basic parsing + args = ["--foo", "bar", "--baz", "1.0"] + assert args_to_dict(args) == {"foo": "bar", "baz": 1.0} + + # JSON type inference + args = [ + "--s", + "str", + "--i", + "1", + "--f", + "1.5", + "--b", + "true", + "--l", + "[1, 2]", + "--d", + '{"a": 1}', + ] + assert args_to_dict(args) == { + "s": "str", + "i": 1, + "f": 1.5, + "b": True, + "l": [1, 2], + "d": {"a": 1}, + } + + # missing value error + with pytest.raises(ValueError, match="Missing value for argument '--foo'"): + args_to_dict(["--foo"]) + + # no-dash-prefix error + with pytest.raises(ValueError, match="Argument 'foo' does not start with '-'"): + args_to_dict(["foo", "bar"])