From 7307a5be8aabf892164912a6f262dcc0b5ac4bc5 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Thu, 13 Jul 2023 10:49:31 +0800 Subject: [PATCH 1/7] support wildcards(/**, /*) for the parameters file Signed-off-by: Chen Lihui --- .../utilities/to_parameters_list.py | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/launch_ros/launch_ros/utilities/to_parameters_list.py b/launch_ros/launch_ros/utilities/to_parameters_list.py index 6aad2032..e6a91e69 100644 --- a/launch_ros/launch_ros/utilities/to_parameters_list.py +++ b/launch_ros/launch_ros/utilities/to_parameters_list.py @@ -14,6 +14,7 @@ """Module with utility to transform evaluated parameters into parameter lists.""" +import re import pathlib from typing import List import warnings @@ -74,10 +75,13 @@ def to_parameters_list( :returns: a list of parameters """ parameters = [] # type: List[rclpy.parameter.Parameter] - node_name = node_name.lstrip('/') + + if node_name[0] != '/': + node_name = '/' + node_name + if namespace and namespace != '/': namespace = namespace.strip('/') - node_name = f'{namespace}/{node_name}' + node_name = f'/{namespace}{node_name}' params_set = {} warned_once = False @@ -89,10 +93,14 @@ def to_parameters_list( if normalized_param_dict: param_dict.clear() - if '**' in normalized_param_dict: - param_dict = normalized_param_dict['**'] - if node_name in normalized_param_dict: - param_dict.update(normalized_param_dict[node_name]) + + for key, value in normalized_param_dict.items(): + p1 = '/' + key + p1 = p1.replace('/*', '(/\\w+)') + match = re.match(p1, node_name) + if match: + param_dict.update(normalized_param_dict[key]) + if not warned_once and not node_name: warnings.warn( 'node name not provided to launch; parameter files will not apply', From aae8499dd9b57ec6eeebfdcfe42109e108c3f21a Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Thu, 13 Jul 2023 10:50:45 +0800 Subject: [PATCH 2/7] fix exist issue Signed-off-by: Chen Lihui --- launch_ros/launch_ros/utilities/to_parameters_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launch_ros/launch_ros/utilities/to_parameters_list.py b/launch_ros/launch_ros/utilities/to_parameters_list.py index e6a91e69..7c968105 100644 --- a/launch_ros/launch_ros/utilities/to_parameters_list.py +++ b/launch_ros/launch_ros/utilities/to_parameters_list.py @@ -101,7 +101,7 @@ def to_parameters_list( if match: param_dict.update(normalized_param_dict[key]) - if not warned_once and not node_name: + if not warned_once and not param_dict: warnings.warn( 'node name not provided to launch; parameter files will not apply', UserWarning From 97bd869b526cbc020ec3193b928a72ef341b9ff5 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Thu, 13 Jul 2023 11:19:59 +0800 Subject: [PATCH 3/7] address exception and flake8 Signed-off-by: Chen Lihui --- .../launch_ros/utilities/to_parameters_list.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/launch_ros/launch_ros/utilities/to_parameters_list.py b/launch_ros/launch_ros/utilities/to_parameters_list.py index 7c968105..b569c7e1 100644 --- a/launch_ros/launch_ros/utilities/to_parameters_list.py +++ b/launch_ros/launch_ros/utilities/to_parameters_list.py @@ -14,8 +14,8 @@ """Module with utility to transform evaluated parameters into parameter lists.""" -import re import pathlib +import re from typing import List import warnings @@ -97,9 +97,13 @@ def to_parameters_list( for key, value in normalized_param_dict.items(): p1 = '/' + key p1 = p1.replace('/*', '(/\\w+)') - match = re.match(p1, node_name) - if match: - param_dict.update(normalized_param_dict[key]) + try: + match = re.match(p1, node_name) + if match: + param_dict.update(normalized_param_dict[key]) + except re.error as e: + raise RuntimeError( + 'invalid yaml file {}, error: {}'.format(str(params_set_or_path), e)) if not warned_once and not param_dict: warnings.warn( From 1398208ed3c58685c92ef2ff750676e570aa2890 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Thu, 13 Jul 2023 13:03:41 +0800 Subject: [PATCH 4/7] update test as last one win in the yaml file and use fullmatch instead Signed-off-by: Chen Lihui --- .../launch_ros/utilities/to_parameters_list.py | 8 ++++---- .../actions/test_load_composable_nodes.py | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/launch_ros/launch_ros/utilities/to_parameters_list.py b/launch_ros/launch_ros/utilities/to_parameters_list.py index b569c7e1..c20240fb 100644 --- a/launch_ros/launch_ros/utilities/to_parameters_list.py +++ b/launch_ros/launch_ros/utilities/to_parameters_list.py @@ -95,12 +95,12 @@ def to_parameters_list( param_dict.clear() for key, value in normalized_param_dict.items(): - p1 = '/' + key - p1 = p1.replace('/*', '(/\\w+)') + pattern = key if key[0] == '/' else '/' + key + pattern = pattern.replace('/*', '(/\\w+)') try: - match = re.match(p1, node_name) + match = re.fullmatch(pattern, node_name) if match: - param_dict.update(normalized_param_dict[key]) + param_dict.update(value) except re.error as e: raise RuntimeError( 'invalid yaml file {}, error: {}'.format(str(params_set_or_path), e)) diff --git a/test_launch_ros/test/test_launch_ros/actions/test_load_composable_nodes.py b/test_launch_ros/test/test_launch_ros/actions/test_load_composable_nodes.py index c7b7f4df..81b59873 100644 --- a/test_launch_ros/test/test_launch_ros/actions/test_load_composable_nodes.py +++ b/test_launch_ros/test/test_launch_ros/actions/test_load_composable_nodes.py @@ -396,12 +396,12 @@ def test_load_node_with_param_file(mock_component_container): assert request.node_name == 'node_1' assert request.node_namespace == '/ns_1' assert len(request.parameters) == 3 - assert request.parameters[0].name == 'param_2' - assert request.parameters[0].value.integer_value == 2 - assert request.parameters[1].name == 'param_3' - assert request.parameters[1].value.integer_value == 33 - assert request.parameters[2].name == 'param_1' - assert request.parameters[2].value.integer_value == 1 + assert request.parameters[0].name == 'param_1' + assert request.parameters[0].value.integer_value == 1 + assert request.parameters[1].name == 'param_2' + assert request.parameters[1].value.integer_value == 22 + assert request.parameters[2].name == 'param_3' + assert request.parameters[2].value.integer_value == 33 request = mock_component_container.requests[-1] assert get_node_name_count(context, '/ns_2/node_2') == 1 From a06396b9d3981b6706784bf6a31363f738d06fdd Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Thu, 13 Jul 2023 13:19:03 +0800 Subject: [PATCH 5/7] Revert "fix exist issue" This reverts commit aae8499dd9b57ec6eeebfdcfe42109e108c3f21a. Signed-off-by: Chen Lihui --- launch_ros/launch_ros/utilities/to_parameters_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launch_ros/launch_ros/utilities/to_parameters_list.py b/launch_ros/launch_ros/utilities/to_parameters_list.py index c20240fb..909d1c6a 100644 --- a/launch_ros/launch_ros/utilities/to_parameters_list.py +++ b/launch_ros/launch_ros/utilities/to_parameters_list.py @@ -105,7 +105,7 @@ def to_parameters_list( raise RuntimeError( 'invalid yaml file {}, error: {}'.format(str(params_set_or_path), e)) - if not warned_once and not param_dict: + if not warned_once and not node_name: warnings.warn( 'node name not provided to launch; parameter files will not apply', UserWarning From 58f4fb4240acd1cd52dacde8211d8c528d61a6f6 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Thu, 13 Jul 2023 13:43:59 +0800 Subject: [PATCH 6/7] add a test Signed-off-by: Chen Lihui --- .../example_parameters_wildcard_mixed.yaml | 3 +++ .../actions/test_load_composable_nodes.py | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 test_launch_ros/test/test_launch_ros/actions/example_parameters_wildcard_mixed.yaml diff --git a/test_launch_ros/test/test_launch_ros/actions/example_parameters_wildcard_mixed.yaml b/test_launch_ros/test/test_launch_ros/actions/example_parameters_wildcard_mixed.yaml new file mode 100644 index 00000000..dc7f6d17 --- /dev/null +++ b/test_launch_ros/test/test_launch_ros/actions/example_parameters_wildcard_mixed.yaml @@ -0,0 +1,3 @@ +/*/aa/**/my_node: + ros__parameters: + param: "wildcard" diff --git a/test_launch_ros/test/test_launch_ros/actions/test_load_composable_nodes.py b/test_launch_ros/test/test_launch_ros/actions/test_load_composable_nodes.py index 81b59873..c5ebbffa 100644 --- a/test_launch_ros/test/test_launch_ros/actions/test_load_composable_nodes.py +++ b/test_launch_ros/test/test_launch_ros/actions/test_load_composable_nodes.py @@ -477,6 +477,26 @@ def test_load_node_with_param_file(mock_component_container): assert request.node_namespace == '/ns' assert len(request.parameters) == 0 + # Case 9: wildcard mixed + context = _assert_launch_no_errors([ + _load_composable_node( + package='foo_package', + plugin='bar_plugin', + name='my_node', + namespace='/wildcard_ns/aa/extra1/extra2', + parameters=[ + parameters_file_dir / 'example_parameters_wildcard_mixed.yaml' + ], + ) + ]) + request = mock_component_container.requests[-1] + assert get_node_name_count(context, '/wildcard_ns/aa/extra1/extra2/my_node') == 1 + assert request.node_name == 'my_node' + assert request.node_namespace == '/wildcard_ns/aa/extra1/extra2' + assert len(request.parameters) == 1 + assert request.parameters[0].name == 'param' + assert request.parameters[0].value.string_value == 'wildcard' + # Namespace not found context = _assert_launch_no_errors([ _load_composable_node( From ada035f5c8e261701c9aeb2a566648e91a5baabe Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Thu, 13 Jul 2023 17:24:06 +0800 Subject: [PATCH 7/7] add a test to test a wrong wildcard format Signed-off-by: Chen Lihui --- .../example_parameters_bad_wildcard.yaml | 3 +++ .../actions/test_set_parameters_from_file.py | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 test_launch_ros/test/test_launch_ros/actions/example_parameters_bad_wildcard.yaml diff --git a/test_launch_ros/test/test_launch_ros/actions/example_parameters_bad_wildcard.yaml b/test_launch_ros/test/test_launch_ros/actions/example_parameters_bad_wildcard.yaml new file mode 100644 index 00000000..d1e5b6c9 --- /dev/null +++ b/test_launch_ros/test/test_launch_ros/actions/example_parameters_bad_wildcard.yaml @@ -0,0 +1,3 @@ +/***: + ros__parameters: + param: "wildcard" diff --git a/test_launch_ros/test/test_launch_ros/actions/test_set_parameters_from_file.py b/test_launch_ros/test/test_launch_ros/actions/test_set_parameters_from_file.py index acb3a5f2..577459d4 100644 --- a/test_launch_ros/test/test_launch_ros/actions/test_set_parameters_from_file.py +++ b/test_launch_ros/test/test_launch_ros/actions/test_set_parameters_from_file.py @@ -27,6 +27,7 @@ from launch_ros.actions.load_composable_nodes import get_composable_node_load_request from launch_ros.descriptions import ComposableNode +import pytest import yaml @@ -176,3 +177,19 @@ def test_set_param_with_composable_node(): assert parameters[0].value.integer_value == 10 assert parameters[1].name == 'asd' assert parameters[1].value.string_value == 'bsd' + + +def test_set_bad_wildcard_param_with_composable_node(): + lc = MockContext() + node_description = ComposableNode( + package='asd', + plugin='my_plugin', + name='my_node', + namespace='my_ns' + ) + param_file_path = \ + os.path.dirname(os.path.abspath(__file__)) + '/example_parameters_bad_wildcard.yaml' + set_param_1 = SetParametersFromFile(param_file_path) + set_param_1.execute(lc) + with pytest.raises(RuntimeError): + get_composable_node_load_request(node_description, lc)