From 0a67ab95de5edb1ed83202af6d79f1fac79f0e8f Mon Sep 17 00:00:00 2001 From: Victor Lopez Date: Fri, 19 Feb 2021 11:05:24 +0100 Subject: [PATCH 1/4] Add wildcard loading to ros2 param load Signed-off-by: Victor Lopez --- ros2param/ros2param/api/__init__.py | 26 ++++++---- ros2param/ros2param/verb/load.py | 8 ++- ros2param/test/test_verb_load.py | 76 ++++++++++++++++++++++++++++- 3 files changed, 96 insertions(+), 14 deletions(-) diff --git a/ros2param/ros2param/api/__init__.py b/ros2param/ros2param/api/__init__.py index 565849887..4acef0842 100644 --- a/ros2param/ros2param/api/__init__.py +++ b/ros2param/ros2param/api/__init__.py @@ -137,21 +137,27 @@ def load_parameter_dict(*, node, node_name, parameter_dict): print(msg, file=sys.stderr) -def load_parameter_file(*, node, node_name, parameter_file): +def load_parameter_file(*, node, node_name, parameter_file, use_wildcard): # Remove leading slash and namespaces with open(parameter_file, 'r') as f: param_file = yaml.safe_load(f) - if node_name not in param_file: + param_keys = [] + if use_wildcard and '/**' in param_file: + param_keys.append('/**') + if node_name in param_file: + param_keys.append(node_name) + + if param_keys == []: raise RuntimeError('Param file does not contain parameters for {}, ' ' only for nodes: {}' .format(node_name, param_file.keys())) - - value = param_file[node_name] - if type(value) != dict or 'ros__parameters' not in value: - raise RuntimeError('Invalid structure of parameter file for node {}' - 'expected same format as provided by ros2 param dump' - .format(node_name)) - load_parameter_dict(node=node, node_name=node_name, - parameter_dict=value['ros__parameters']) + for k in param_keys: + value = param_file[k] + if type(value) != dict or 'ros__parameters' not in value: + raise RuntimeError('Invalid structure of parameter file for node {}' + 'expected same format as provided by ros2 param dump' + .format(k)) + load_parameter_dict(node=node, node_name=node_name, + parameter_dict=value['ros__parameters']) def call_describe_parameters(*, node, node_name, parameter_names=None): diff --git a/ros2param/ros2param/verb/load.py b/ros2param/ros2param/verb/load.py index 72a72824e..a9fd8d422 100644 --- a/ros2param/ros2param/verb/load.py +++ b/ros2param/ros2param/verb/load.py @@ -34,8 +34,11 @@ def add_arguments(self, parser, cli_name): # noqa: D102 parser.add_argument( '--include-hidden-nodes', action='store_true', help='Consider hidden nodes as well') - arg = parser.add_argument( + parser.add_argument( 'parameter_file', help='Parameter file') + parser.add_argument( + '--use-wildcard', action='store_true', + help='Load parameters in the \'/**\' namespace into the node') def main(self, *, args): # noqa: D102 with NodeStrategy(args) as node: @@ -47,4 +50,5 @@ def main(self, *, args): # noqa: D102 return 'Node not found' with DirectNode(args) as node: - load_parameter_file(node=node, node_name=node_name, parameter_file=args.parameter_file) + load_parameter_file(node=node, node_name=node_name, parameter_file=args.parameter_file, + use_wildcard=args.use_wildcard) diff --git a/ros2param/test/test_verb_load.py b/ros2param/test/test_verb_load.py index 50c89e1b3..2296ec82a 100644 --- a/ros2param/test/test_verb_load.py +++ b/ros2param/test/test_verb_load.py @@ -19,6 +19,7 @@ import time import unittest import xmlrpc +import yaml from launch import LaunchDescription from launch.actions import ExecuteProcess @@ -71,6 +72,17 @@ ' str_param: Bye World\n' ' use_sim_time: false\n' ) +INPUT_WILDCARD_PARAMETER_FILE = ( + f'/**:\n' + ' ros__parameters:\n' + ' str_param: Wildcard\n' + ' int_param: 12345\n' +) +INPUT_NODE_OVERLAY_PARAMETER_FILE = ( + f'{TEST_NAMESPACE}/{TEST_NODE}:\n' + ' ros__parameters:\n' + ' str_param: Override\n' +) # Skip cli tests on Windows while they exhibit pathological behavior # https://github.com/ros2/build_farmer/issues/248 @@ -191,10 +203,10 @@ def setUp(self): if timed_out: self.fail(f'CLI daemon failed to find test node after {TEST_TIMEOUT} seconds') - def _write_param_file(self, tmpdir, filename): + def _write_param_file(self, tmpdir, filename, contents=INPUT_PARAMETER_FILE): yaml_path = os.path.join(tmpdir, filename) with open(yaml_path, 'w') as f: - f.write(INPUT_PARAMETER_FILE) + f.write(contents) return yaml_path def _output_file(self): @@ -280,3 +292,63 @@ def test_verb_load(self): text=param_dump_command.output, strict=True ) + + def test_verb_load_wildcard(self): + with tempfile.TemporaryDirectory() as tmpdir: + # Try param file with only wildcard + filepath = self._write_param_file(tmpdir, 'params.yaml', INPUT_WILDCARD_PARAMETER_FILE) + with self.launch_param_load_command( + arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', filepath] + ) as param_load_command: + assert param_load_command.wait_for_shutdown(timeout=TEST_TIMEOUT) + assert param_load_command.exit_code != launch_testing.asserts.EXIT_OK + assert launch_testing.tools.expect_output( + expected_lines=['Param file does not contain parameters for ' + f'{TEST_NAMESPACE}/{TEST_NODE}'], + text=param_load_command.output, + strict=False + ) + + with self.launch_param_load_command( + arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', filepath, + '--use-wildcard'] + ) as param_load_command: + assert param_load_command.wait_for_shutdown(timeout=TEST_TIMEOUT) + assert param_load_command.exit_code == launch_testing.asserts.EXIT_OK + assert launch_testing.tools.expect_output( + expected_lines=[''], + text=param_load_command.output, + strict=True + ) + # Dump with ros2 param and check that wildcard parameters are loaded + with self.launch_param_dump_command( + arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', '--print'] + ) as param_dump_command: + assert param_dump_command.wait_for_shutdown(timeout=TEST_TIMEOUT) + assert param_dump_command.exit_code == launch_testing.asserts.EXIT_OK + loaded_params = yaml.safe_load(param_dump_command.output) + params = loaded_params[f'{TEST_NAMESPACE}/{TEST_NODE}']['ros__parameters'] + assert params['str_param'] == 'Wildcard' + assert params['int_param'] == 12345 + + # Concatenate wildcard + some overlays + filepath = self._write_param_file(tmpdir, 'params.yaml', + INPUT_WILDCARD_PARAMETER_FILE + '\n' + + INPUT_NODE_OVERLAY_PARAMETER_FILE) + with self.launch_param_load_command( + arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', filepath, + '--use-wildcard'] + ) as param_load_command: + assert param_load_command.wait_for_shutdown(timeout=TEST_TIMEOUT) + assert param_load_command.exit_code == launch_testing.asserts.EXIT_OK + + # Dump and check that wildcard parameters were overriden if in node namespace + with self.launch_param_dump_command( + arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', '--print'] + ) as param_dump_command: + assert param_dump_command.wait_for_shutdown(timeout=TEST_TIMEOUT) + assert param_dump_command.exit_code == launch_testing.asserts.EXIT_OK + loaded_params = yaml.safe_load(param_dump_command.output) + params = loaded_params[f'{TEST_NAMESPACE}/{TEST_NODE}']['ros__parameters'] + assert params['str_param'] == 'Override' # Overriden + assert params['int_param'] == 12345 # Wildcard namespace From 50768ef2fa92e720c40ea307db3232fd4fae753d Mon Sep 17 00:00:00 2001 From: Victor Lopez <3469405+v-lopez@users.noreply.github.com> Date: Tue, 23 Feb 2021 14:35:01 +0100 Subject: [PATCH 2/4] Update ros2param/ros2param/api/__init__.py Co-authored-by: Ivan Santiago Paunovic Signed-off-by: Victor Lopez --- ros2param/ros2param/api/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ros2param/ros2param/api/__init__.py b/ros2param/ros2param/api/__init__.py index 4acef0842..6a1fb51cc 100644 --- a/ros2param/ros2param/api/__init__.py +++ b/ros2param/ros2param/api/__init__.py @@ -150,14 +150,15 @@ def load_parameter_file(*, node, node_name, parameter_file, use_wildcard): if param_keys == []: raise RuntimeError('Param file does not contain parameters for {}, ' ' only for nodes: {}' .format(node_name, param_file.keys())) + param_dict = {} for k in param_keys: value = param_file[k] if type(value) != dict or 'ros__parameters' not in value: raise RuntimeError('Invalid structure of parameter file for node {}' 'expected same format as provided by ros2 param dump' .format(k)) - load_parameter_dict(node=node, node_name=node_name, - parameter_dict=value['ros__parameters']) + param_dict.update(value['ros__parameters']) + load_parameter_dict(node=node, node_name=node_name, parameter_dict=param_dict) def call_describe_parameters(*, node, node_name, parameter_names=None): From 4212449812728f88a621ef0dd87fd289f8d7816a Mon Sep 17 00:00:00 2001 From: Victor Lopez Date: Tue, 23 Feb 2021 14:41:08 +0100 Subject: [PATCH 3/4] Replace use-wildcard with no-use-wildcard Signed-off-by: Victor Lopez --- ros2param/ros2param/verb/load.py | 6 +++--- ros2param/test/test_verb_load.py | 9 ++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/ros2param/ros2param/verb/load.py b/ros2param/ros2param/verb/load.py index a9fd8d422..9edf76a65 100644 --- a/ros2param/ros2param/verb/load.py +++ b/ros2param/ros2param/verb/load.py @@ -37,8 +37,8 @@ def add_arguments(self, parser, cli_name): # noqa: D102 parser.add_argument( 'parameter_file', help='Parameter file') parser.add_argument( - '--use-wildcard', action='store_true', - help='Load parameters in the \'/**\' namespace into the node') + '--no-use-wildcard', action='store_true', + help='Do not load parameters in the \'/**\' namespace into the node') def main(self, *, args): # noqa: D102 with NodeStrategy(args) as node: @@ -51,4 +51,4 @@ def main(self, *, args): # noqa: D102 with DirectNode(args) as node: load_parameter_file(node=node, node_name=node_name, parameter_file=args.parameter_file, - use_wildcard=args.use_wildcard) + use_wildcard=not args.no_use_wildcard) diff --git a/ros2param/test/test_verb_load.py b/ros2param/test/test_verb_load.py index 2296ec82a..30142400f 100644 --- a/ros2param/test/test_verb_load.py +++ b/ros2param/test/test_verb_load.py @@ -298,7 +298,8 @@ def test_verb_load_wildcard(self): # Try param file with only wildcard filepath = self._write_param_file(tmpdir, 'params.yaml', INPUT_WILDCARD_PARAMETER_FILE) with self.launch_param_load_command( - arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', filepath] + arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', filepath, + '--no-use-wildcard'] ) as param_load_command: assert param_load_command.wait_for_shutdown(timeout=TEST_TIMEOUT) assert param_load_command.exit_code != launch_testing.asserts.EXIT_OK @@ -310,8 +311,7 @@ def test_verb_load_wildcard(self): ) with self.launch_param_load_command( - arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', filepath, - '--use-wildcard'] + arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', filepath] ) as param_load_command: assert param_load_command.wait_for_shutdown(timeout=TEST_TIMEOUT) assert param_load_command.exit_code == launch_testing.asserts.EXIT_OK @@ -336,8 +336,7 @@ def test_verb_load_wildcard(self): INPUT_WILDCARD_PARAMETER_FILE + '\n' + INPUT_NODE_OVERLAY_PARAMETER_FILE) with self.launch_param_load_command( - arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', filepath, - '--use-wildcard'] + arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', filepath] ) as param_load_command: assert param_load_command.wait_for_shutdown(timeout=TEST_TIMEOUT) assert param_load_command.exit_code == launch_testing.asserts.EXIT_OK From 2608c464446bf8af4ac1fc596ff2f464af5f873f Mon Sep 17 00:00:00 2001 From: Victor Lopez Date: Tue, 23 Feb 2021 15:19:49 +0100 Subject: [PATCH 4/4] Apply linter fixes Signed-off-by: Victor Lopez --- ros2param/ros2param/verb/load.py | 2 +- ros2param/test/test_verb_load.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ros2param/ros2param/verb/load.py b/ros2param/ros2param/verb/load.py index 9edf76a65..e260f81f6 100644 --- a/ros2param/ros2param/verb/load.py +++ b/ros2param/ros2param/verb/load.py @@ -38,7 +38,7 @@ def add_arguments(self, parser, cli_name): # noqa: D102 'parameter_file', help='Parameter file') parser.add_argument( '--no-use-wildcard', action='store_true', - help='Do not load parameters in the \'/**\' namespace into the node') + help="Do not load parameters in the '/**' namespace into the node") def main(self, *, args): # noqa: D102 with NodeStrategy(args) as node: diff --git a/ros2param/test/test_verb_load.py b/ros2param/test/test_verb_load.py index 30142400f..953262758 100644 --- a/ros2param/test/test_verb_load.py +++ b/ros2param/test/test_verb_load.py @@ -19,7 +19,6 @@ import time import unittest import xmlrpc -import yaml from launch import LaunchDescription from launch.actions import ExecuteProcess @@ -38,6 +37,8 @@ from ros2cli.node.strategy import NodeStrategy +import yaml + TEST_NODE = 'test_node' TEST_NAMESPACE = '/foo' @@ -73,7 +74,7 @@ ' use_sim_time: false\n' ) INPUT_WILDCARD_PARAMETER_FILE = ( - f'/**:\n' + '/**:\n' ' ros__parameters:\n' ' str_param: Wildcard\n' ' int_param: 12345\n'