From 12b81f24cbc757306f29e77c835f7e0fdf3ce487 Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Wed, 3 Dec 2025 10:59:10 +0800 Subject: [PATCH 1/2] Fix parameter parsing for unspecified target nodes Signed-off-by: Barry Xu --- rclpy/rclpy/parameter.py | 21 +++++++++++++ rclpy/test/test_parameter.py | 57 +++++++++++++++++++++++++++--------- 2 files changed, 64 insertions(+), 14 deletions(-) diff --git a/rclpy/rclpy/parameter.py b/rclpy/rclpy/parameter.py index 1fd7dab76..50da49e1a 100644 --- a/rclpy/rclpy/parameter.py +++ b/rclpy/rclpy/parameter.py @@ -423,6 +423,27 @@ def parameter_dict_from_yaml_file( raise RuntimeError('YAML file is not a valid ROS parameter ' f'file for namespace {ns} node {node_basename}') param_dict.update(value['ros__parameters']) + else: + # Except the wildcard key, add all other keys + for k, v in param_file.items(): + if k == '/**' or k == '/*': + continue + if isinstance(v, dict): + # k is node_name or /node_name or /namespace/node_name + if 'ros__parameters' in v: + param_dict.update(v['ros__parameters']) + continue + # k is namespace + for node_name, node_value in v.items(): + if isinstance(node_value, dict) and 'ros__parameters' in node_value: + param_dict.update(node_value['ros__parameters']) + else: + raise RuntimeError( + f'YAML file is not a valid ROS parameter file for node \ + {k}/{node_name}') + else: + raise RuntimeError( + f'YAML file is not a valid ROS parameter file for node {k}') if not param_dict: raise RuntimeError('Param file does not contain any valid parameters') diff --git a/rclpy/test/test_parameter.py b/rclpy/test/test_parameter.py index c0101c9d6..7a1108439 100644 --- a/rclpy/test/test_parameter.py +++ b/rclpy/test/test_parameter.py @@ -268,26 +268,53 @@ def test_parameter_dict_from_yaml_file(self) -> None: base-nodename: true /foo/param_test_target: ros__parameters: - abs-ns-nodename: true + abs-foo-ns-nodename: true /foo: param_test_target: ros__parameters: - abs-ns-base-nodename: true + abs-foo-ns-base-nodename: true /bar/param_test_target: ros__parameters: - abs-ns-nodename: false + abs-bar-ns-nodename: false /bar: param_test_target: ros__parameters: - abs-ns-base-nodename: false + abs-bar-ns-base-nodename: false /**: ros__parameters: wildcard: true """ + # target nodes isn't specified with wildcard disabled + expected_no_target_node_no_wildcard = { + 'abs-bar-ns-base-nodename': Parameter( + 'abs-bar-ns-base-nodename', Parameter.Type.BOOL, False).to_parameter_msg(), + 'abs-bar-ns-nodename': Parameter( + 'abs-bar-ns-nodename', Parameter.Type.BOOL, False).to_parameter_msg(), + 'abs-foo-ns-base-nodename': Parameter( + 'abs-foo-ns-base-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), + 'abs-foo-ns-nodename': Parameter( + 'abs-foo-ns-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), + 'abs-nodename': Parameter( + 'abs-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), + 'base-nodename': Parameter( + 'base-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), + } - # target nodes is specified, so it should only parse wildcard - expected_no_target_node = { - 'wildcard': Parameter('wildcard', Parameter.Type.BOOL, True).to_parameter_msg(), + # target nodes isn't specified with wildcard enabled + expected_no_target_node_wildcard = { + 'abs-bar-ns-base-nodename': Parameter( + 'abs-bar-ns-base-nodename', Parameter.Type.BOOL, False).to_parameter_msg(), + 'abs-bar-ns-nodename': Parameter( + 'abs-bar-ns-nodename', Parameter.Type.BOOL, False).to_parameter_msg(), + 'abs-foo-ns-base-nodename': Parameter( + 'abs-foo-ns-base-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), + 'abs-foo-ns-nodename': Parameter( + 'abs-foo-ns-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), + 'abs-nodename': Parameter( + 'abs-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), + 'base-nodename': Parameter( + 'base-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), + 'wildcard': Parameter('wildcard', Parameter.Type.BOOL, True).to_parameter_msg(), } # target nodes is specified with wildcard enabled expected_target_node_wildcard = { @@ -308,10 +335,10 @@ def test_parameter_dict_from_yaml_file(self) -> None: # target nodes is specified with wildcard and namespace expected_target_node_ns = { 'wildcard': Parameter('wildcard', Parameter.Type.BOOL, True).to_parameter_msg(), - 'abs-ns-nodename': Parameter( - 'abs-ns-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), - 'abs-ns-base-nodename': Parameter( - 'abs-ns-base-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), + 'abs-foo-ns-nodename': Parameter( + 'abs-foo-ns-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), + 'abs-foo-ns-base-nodename': Parameter( + 'abs-foo-ns-base-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), } try: @@ -319,11 +346,13 @@ def test_parameter_dict_from_yaml_file(self) -> None: f.write(yaml_string) f.flush() f.close() + parameter_dict = parameter_dict_from_yaml_file(f.name, False) + assert parameter_dict == expected_no_target_node_no_wildcard parameter_dict = parameter_dict_from_yaml_file(f.name, True) - assert parameter_dict == expected_no_target_node + assert parameter_dict == expected_no_target_node_wildcard parameter_dict = parameter_dict_from_yaml_file( - f.name, True, target_nodes=['']) - assert parameter_dict == expected_no_target_node + f.name, True, target_nodes=[]) + assert parameter_dict == expected_no_target_node_wildcard parameter_dict = parameter_dict_from_yaml_file( f.name, True, target_nodes=['param_test_target']) assert parameter_dict == expected_target_node_wildcard From 419ee5e82e54fb9cbaad48e41abfcbbf009521d2 Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Wed, 3 Dec 2025 11:29:21 +0800 Subject: [PATCH 2/2] Address review comments from Copilot Signed-off-by: Barry Xu --- rclpy/rclpy/parameter.py | 4 ++-- rclpy/test/test_parameter.py | 30 +++++++++++++++--------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/rclpy/rclpy/parameter.py b/rclpy/rclpy/parameter.py index 50da49e1a..4b81ff2a8 100644 --- a/rclpy/rclpy/parameter.py +++ b/rclpy/rclpy/parameter.py @@ -439,8 +439,8 @@ def parameter_dict_from_yaml_file( param_dict.update(node_value['ros__parameters']) else: raise RuntimeError( - f'YAML file is not a valid ROS parameter file for node \ - {k}/{node_name}') + f'YAML file is not a valid ROS parameter file for node' + f'{k}/{node_name}') else: raise RuntimeError( f'YAML file is not a valid ROS parameter file for node {k}') diff --git a/rclpy/test/test_parameter.py b/rclpy/test/test_parameter.py index 7a1108439..c75f41db0 100644 --- a/rclpy/test/test_parameter.py +++ b/rclpy/test/test_parameter.py @@ -284,37 +284,37 @@ def test_parameter_dict_from_yaml_file(self) -> None: ros__parameters: wildcard: true """ - # target nodes isn't specified with wildcard disabled + # target nodes arn't specified with wildcard disabled expected_no_target_node_no_wildcard = { - 'abs-bar-ns-base-nodename': Parameter( + 'abs-bar-ns-base-nodename': Parameter( 'abs-bar-ns-base-nodename', Parameter.Type.BOOL, False).to_parameter_msg(), - 'abs-bar-ns-nodename': Parameter( + 'abs-bar-ns-nodename': Parameter( 'abs-bar-ns-nodename', Parameter.Type.BOOL, False).to_parameter_msg(), - 'abs-foo-ns-base-nodename': Parameter( + 'abs-foo-ns-base-nodename': Parameter( 'abs-foo-ns-base-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), - 'abs-foo-ns-nodename': Parameter( + 'abs-foo-ns-nodename': Parameter( 'abs-foo-ns-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), - 'abs-nodename': Parameter( + 'abs-nodename': Parameter( 'abs-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), - 'base-nodename': Parameter( + 'base-nodename': Parameter( 'base-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), } - # target nodes isn't specified with wildcard enabled + # target nodes aren't specified with wildcard enabled expected_no_target_node_wildcard = { - 'abs-bar-ns-base-nodename': Parameter( + 'abs-bar-ns-base-nodename': Parameter( 'abs-bar-ns-base-nodename', Parameter.Type.BOOL, False).to_parameter_msg(), - 'abs-bar-ns-nodename': Parameter( + 'abs-bar-ns-nodename': Parameter( 'abs-bar-ns-nodename', Parameter.Type.BOOL, False).to_parameter_msg(), - 'abs-foo-ns-base-nodename': Parameter( + 'abs-foo-ns-base-nodename': Parameter( 'abs-foo-ns-base-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), - 'abs-foo-ns-nodename': Parameter( + 'abs-foo-ns-nodename': Parameter( 'abs-foo-ns-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), - 'abs-nodename': Parameter( + 'abs-nodename': Parameter( 'abs-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), - 'base-nodename': Parameter( + 'base-nodename': Parameter( 'base-nodename', Parameter.Type.BOOL, True).to_parameter_msg(), - 'wildcard': Parameter('wildcard', Parameter.Type.BOOL, True).to_parameter_msg(), + 'wildcard': Parameter('wildcard', Parameter.Type.BOOL, True).to_parameter_msg(), } # target nodes is specified with wildcard enabled expected_target_node_wildcard = {