Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support both parameter file configurations for composable nodes #259

Merged
merged 11 commits into from
Oct 23, 2021
8 changes: 5 additions & 3 deletions launch_ros/launch_ros/actions/load_composable_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ def get_composable_node_load_request(
composable_node_description: ComposableNode,
context: LaunchContext
):
"""Get the request that will be send to the composable node container."""
"""Get the request that will be sent to the composable node container."""
request = composition_interfaces.srv.LoadNode.Request()
request.package_name = perform_substitutions(
context, composable_node_description.package
Expand Down Expand Up @@ -283,15 +283,17 @@ def get_composable_node_load_request(
if parameters:
request.parameters = [
param.to_parameter_msg() for param in to_parameters_list(
context, evaluate_parameters(
context, request.node_name, expanded_ns,
evaluate_parameters(
context, parameters
)
)
]
if composable_node_description.extra_arguments is not None:
request.extra_arguments = [
param.to_parameter_msg() for param in to_parameters_list(
context, evaluate_parameters(
context, request.node_name, expanded_ns,
evaluate_parameters(
context, composable_node_description.extra_arguments
)
)
Expand Down
61 changes: 58 additions & 3 deletions launch_ros/launch_ros/utilities/to_parameters_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import pathlib
from typing import List
import warnings

from launch.launch_context import LaunchContext
import rclpy.parameter
Expand All @@ -28,26 +29,80 @@
from ..parameters_type import EvaluatedParameters


def __normalize_parameters_dict(dictionary):
"""
Combine namespaces and their node names into one key and remove the "ros__parameters" key.

Return an empty dict if the "ros__parameters" key is not found.
"""
# Recursive function
def normalize_parameters_dict(dictionary, keys, result_dict):
for key, value in dictionary.items():
# Base case: found 'ros__parameters' key
# Return
if key == 'ros__parameters':
node_name = '/'.join(keys)
result_dict[node_name] = value
return result_dict

if isinstance(value, dict):
keys.append(key.lstrip('/'))
result_dict = normalize_parameters_dict(value, keys, result_dict)
keys = []
jacobperron marked this conversation as resolved.
Show resolved Hide resolved

return result_dict

return normalize_parameters_dict(dictionary, [], {})


def to_parameters_list(
context: LaunchContext,
node_name: str,
namespace: str,
evaluated_parameters: EvaluatedParameters
) -> List[rclpy.parameter.Parameter]:
"""
Transform evaluated parameters into a list of rclpy.parameter.Parameter objects.

:param context: to carry out substitutions during normalization of parameter files.
See `normalize_parameters()` documentation for further reference.
:param node_name: node name
:param namespace: node namespace
:param parameters: parameters as either paths to parameter files or name/value pairs.
See `evaluate_parameters()` documentation for further reference.
:returns: a list of parameters
"""
parameters = [] # type: List[rclpy.parameter.Parameter]
node_name = node_name.lstrip('/')
if namespace:
namespace = namespace.strip('/')
node_name = f'{namespace}/{node_name}'

params_set = {}
warned_once = False
for params_set_or_path in evaluated_parameters:
if isinstance(params_set_or_path, pathlib.Path):
with open(str(params_set_or_path), 'r') as f:
params_set = evaluate_parameter_dict(
context, normalize_parameter_dict(yaml.safe_load(f))
)
param_dict = yaml.safe_load(f)
normalized_param_dict = __normalize_parameters_dict(param_dict)

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])
if not warned_once and not node_name:
warnings.warn(
'node name not provided to launch; parameter files will not apply',
UserWarning
)
warned_once = True

if param_dict:
params_set = evaluate_parameter_dict(
context, normalize_parameter_dict(param_dict)
)
else:
params_set = params_set_or_path
if not isinstance(params_set, dict):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/ns_1:
/node_1:
ros__parameters:
param_1: 1
param_2: 2

/**:
ros__parameters:
param_2: 22
param_3: 33

ns_2/node_2:
ros__parameters:
param_3: 3
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ns/test_node_name:
ros__parameters:
param: 1
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
some_int: 42
a_string: "Hello world"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/test_node_name:
ros__parameters:
param: 2
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/**:
ros__parameters:
param: "wildcard"
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

"""Tests for the LoadComposableNodes Action."""

import pathlib
import threading

from composition_interfaces.srv import LoadNode
Expand Down Expand Up @@ -214,6 +215,177 @@ def test_load_node_with_params(mock_component_container):
assert len(request.extra_arguments) == 0


def test_load_node_with_param_file(mock_component_container):
"""Test loading a node with with parameters specified in yaml files."""
parameters_file_dir = pathlib.Path(__file__).resolve().parent

# Case 1: no node name in yaml file
context = _assert_launch_no_errors([
_load_composable_node(
package='foo_package',
plugin='bar_plugin',
name='test_node_name',
namespace='test_node_namespace',
parameters=[
parameters_file_dir / 'example_parameters_no_name.yaml'
],
)
])
request = mock_component_container.requests[0]
assert get_node_name_count(context, '/test_node_namespace/test_node_name') == 1
assert request.node_name == 'test_node_name'
assert request.node_namespace == '/test_node_namespace'
assert len(request.parameters) == 2
assert request.parameters[0].name == 'some_int'
assert request.parameters[0].value.integer_value == 42
assert request.parameters[1].name == 'a_string'
assert request.parameters[1].value.string_value == 'Hello world'

# Case 2: node name with namespace
context = _assert_launch_no_errors([
_load_composable_node(
package='foo_package',
plugin='bar_plugin',
name='test_node_name',
namespace='ns',
parameters=[
parameters_file_dir / 'example_parameters_namespace.yaml'
],
)
])
request = mock_component_container.requests[1]
assert get_node_name_count(context, '/ns/test_node_name') == 1
assert request.node_name == 'test_node_name'
assert request.node_namespace == '/ns'
assert len(request.parameters) == 1
assert request.parameters[0].name == 'param'
assert request.parameters[0].value.integer_value == 1

# Case 3: nested node name with namespace
jacobperron marked this conversation as resolved.
Show resolved Hide resolved
context = _assert_launch_no_errors([
_load_composable_node(
package='foo_package',
plugin='bar_plugin',
name='my_node',
namespace='my_ns',
parameters=[
parameters_file_dir / 'example_parameters.yaml'
],
)
])
request = mock_component_container.requests[2]
assert get_node_name_count(context, '/my_ns/my_node') == 1
assert request.node_name == 'my_node'
assert request.node_namespace == '/my_ns'
assert len(request.parameters) == 5
assert request.parameters[0].name == 'some_int'
assert request.parameters[0].value.integer_value == 42
assert request.parameters[1].name == 'a_string'
assert request.parameters[1].value.string_value == 'Hello world'
assert request.parameters[2].value.string_value == ''

# Case 4: node name without namespace
context = _assert_launch_no_errors([
_load_composable_node(
package='foo_package',
plugin='bar_plugin',
name='test_node_name',
namespace='',
parameters=[
parameters_file_dir / 'example_parameters_no_namespace.yaml'
],
)
])
request = mock_component_container.requests[3]
assert get_node_name_count(context, '//test_node_name') == 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think two leading slashes should be a valid node name, I would have expected the name to be /test_node_name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think get_node_name_count() expects the name to be of the form /ns/node_name, so if the namespace is empty, it has to be //node_name 🤷 It doesn't work without the extra slash.

assert request.node_name == 'test_node_name'
assert request.node_namespace == '/'
assert len(request.parameters) == 1
assert request.parameters[0].name == 'param'
assert request.parameters[0].value.integer_value == 2

# Case 5: wildcard
context = _assert_launch_no_errors([
_load_composable_node(
package='foo_package',
plugin='bar_plugin',
name='my_node',
namespace='wildcard_ns',
parameters=[
parameters_file_dir / 'example_parameters_wildcard.yaml'
],
)
])
request = mock_component_container.requests[4]
assert get_node_name_count(context, '/wildcard_ns/my_node') == 1
assert request.node_name == 'my_node'
assert request.node_namespace == '/wildcard_ns'
assert len(request.parameters) == 1
assert request.parameters[0].name == 'param'
assert request.parameters[0].value.string_value == 'wildcard'

# Case 6: multiple entries (params with node name should take precedence over wildcard params)
context = _assert_launch_no_errors([
_load_composable_node(
package='foo_package',
plugin='bar_plugin',
name='node_1',
namespace='ns_1',
parameters=[
parameters_file_dir / 'example_parameters_multiple_entries.yaml'
],
),
_load_composable_node(
package='foo_package',
plugin='bar_plugin',
name='node_2',
namespace='ns_2',
parameters=[
parameters_file_dir / 'example_parameters_multiple_entries.yaml'
],
)
])
request = mock_component_container.requests[5]
assert get_node_name_count(context, '/ns_1/node_1') == 1
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

request = mock_component_container.requests[6]
assert get_node_name_count(context, '/ns_2/node_2') == 1
assert request.node_name == 'node_2'
assert request.node_namespace == '/ns_2'
assert len(request.parameters) == 2
assert request.parameters[0].name == 'param_2'
assert request.parameters[0].value.integer_value == 22
assert request.parameters[1].name == 'param_3'
assert request.parameters[1].value.integer_value == 3

# Case 7: node name not found
context = _assert_launch_no_errors([
_load_composable_node(
package='foo_package',
plugin='bar_plugin',
name='wrong_node_name',
namespace='ns',
parameters=[
parameters_file_dir / 'example_parameters_no_namespace.yaml'
],
)
])
request = mock_component_container.requests[7]
assert get_node_name_count(context, '/ns/wrong_node_name') == 1
assert request.node_name == 'wrong_node_name'
assert request.node_namespace == '/ns'
assert len(request.parameters) == 0


def test_load_node_with_global_remaps_in_group(mock_component_container):
"""Test loading a node with global remaps scoped to a group."""
context = _assert_launch_no_errors([
Expand Down