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 passing parameters as a dictionary #138

Merged
merged 17 commits into from
Sep 21, 2018
Merged
126 changes: 109 additions & 17 deletions launch_ros/launch_ros/actions/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@
import logging
import os
import pathlib
from tempfile import NamedTemporaryFile
from typing import Dict # noqa: F401
from typing import Iterable
from typing import List
from typing import Optional
from typing import Text # noqa: F401
from typing import Tuple

from launch import Substitution
from launch.action import Action
from launch.actions import ExecuteProcess
from launch.launch_context import LaunchContext
Expand All @@ -39,6 +41,8 @@
from rclpy.validate_namespace import validate_namespace
from rclpy.validate_node_name import validate_node_name

import yaml

_logger = logging.getLogger(name='launch_ros')


Expand Down Expand Up @@ -87,11 +91,28 @@ def __init__(
If no node_namespace is given, then the default namespace `/` is
assumed.

The parameters are passed as a list, with each element either a yaml
file that contains parameter rules (string or pathlib.Path to the full
path of the file), or a dictionary that specifies parameter rules.
Keys of the dictionary can be strings or an iterable of Substitutions
that will be expanded to a string.
Values in the dictionary can be strings, integers, floats, or tuples
of Substitutions that will be expanded to a string.
Additionally, values in the dictionary can be lists of the
aforementioned types, or another dictionary with the same properties.
A yaml file with the resulting parameters from the dictionary will be
written to a temporary file, the path to which will be passed to the
node.
Multiple dictionaries/files can be passed: each file path will be
passed in in order to the node (where the last definition of a
parameter takes effect).

:param: package the package in which the node executable can be found
:param: node_executable the name of the executable to find
:param: node_name the name of the node
:param: node_namespace the ros namespace for this Node
:param: parameters list of names of yaml files with parameter rules
:param: parameters list of names of yaml files with parameter rules,
or dictionaries of parameters.
:param: remappings ordered list of 'to' and 'from' string pairs to be
passed to the node as ROS remapping rules
:param: arguments list of extra arguments for the node
Expand All @@ -111,13 +132,16 @@ def __init__(
ros_args_index += 1
if parameters is not None:
ensure_argument_type(parameters, (list), 'parameters', 'Node')
# All arguments are paths to files with parameters (or substitutions that evaluate
# to paths).
# TODO(dhood): add support for parameter dicts.
parameter_types = list(SomeSubstitutionsType_types_tuple) + [pathlib.Path]
# All elements in the list are paths to files with parameters (or substitutions that
# evaluate to paths), or dictionaries of parameters (fields can be substitutions).
parameter_types = list(SomeSubstitutionsType_types_tuple) + [pathlib.Path, dict]
i = 0
for param in parameters:
ensure_argument_type(param, parameter_types, 'parameters[{}]'.format(i), 'Node')
if isinstance(param, dict) and node_name is None:
raise RuntimeError(
'If a dictionary of parameters is specified, the node name must also be '
'specified. See https://github.com/ros2/launch/issues/139')
i += 1
cmd += [LocalSubstitution(
'ros_specific_arguments[{}]'.format(ros_args_index),
Expand Down Expand Up @@ -146,7 +170,7 @@ def __init__(
self.__expanded_node_name = '<node_name_unspecified>'
self.__expanded_node_namespace = '/'
self.__final_node_name = None # type: Optional[Text]
self.__expanded_parameters = None # type: Optional[List[Text]]
self.__expanded_parameter_files = None # type: Optional[List[Text]]
self.__expanded_remappings = None # type: Optional[List[Tuple[Text, Text]]]

self.__substitutions_performed = False
Expand All @@ -158,6 +182,70 @@ def node_name(self):
raise RuntimeError("cannot access 'node_name' before executing action")
return self.__final_node_name

def _create_params_file_from_dict(self, context, params):
with NamedTemporaryFile(mode='w', prefix='launch_params_', delete=False) as h:
param_file_path = h.name
# TODO(dhood): clean up generated parameter files.
Copy link
Member

Choose a reason for hiding this comment

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

Is the intention to merge this and follow up with a pull request to clean up the files or do you think we can do without it?

I know we talked about it before and I said it would be ok to not do that for now, but now I'm thinking we should really clean up after ourselves. What are your thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that it's important also, was planning to leave it as followup to simplify reviewing of this PR. The tests are relying on the files being around atm so was going to also have an option to disable the cleanup.


def perform_substitution_if_applicable(context, var):
if isinstance(var, (int, float, str)):
# No substitution necessary.
return var
if isinstance(var, Substitution):
return perform_substitutions(context, normalize_to_list_of_substitutions(var))
if isinstance(var, tuple):
try:
return perform_substitutions(
context, normalize_to_list_of_substitutions(var))
except TypeError as e:
raise TypeError(
'Invalid element received in parameters dictionary '
'(not all tuple elements are Substitutions): {}'.format(var))
else:
raise TypeError(
'Unsupported type received in parameters dictionary: {}'
.format(type(var)))

def expand_dict(input_dict):
expanded_dict = {}
for k, v in input_dict.items():
# Key (parameter/group name) can only be a string/Substitutions that evaluates
# to a string.
expanded_key = perform_substitutions(
context, normalize_to_list_of_substitutions(k))
if isinstance(v, dict):
# Expand the nested dict.
expanded_value = expand_dict(v)
elif isinstance(v, list):
# Expand each element.
expanded_value = []
for e in v:
if isinstance(e, list):
raise TypeError(
'Nested lists are not supported for parameters: {} found in {}'
.format(e, v))
expanded_value.append(perform_substitution_if_applicable(context, e))
# Tuples are treated as Substitution(s) to be concatenated.
elif isinstance(v, tuple):
for e in v:
ensure_argument_type(
e, SomeSubstitutionsType_types_tuple,
'parameter dictionary tuple entry', 'Node')
expanded_value = perform_substitutions(
context, normalize_to_list_of_substitutions(v))
else:
expanded_value = perform_substitution_if_applicable(context, v)
expanded_dict[expanded_key] = expanded_value
return expanded_dict

expanded_dict = expand_dict(params)
param_dict = {
self.__expanded_node_name: {'ros__parameters': expanded_dict}}
if self.__expanded_node_namespace:
param_dict = {self.__expanded_node_namespace: param_dict}
yaml.dump(param_dict, h, default_flow_style=False)
return param_file_path

def _perform_substitutions(self, context: LaunchContext) -> None:
try:
if self.__substitutions_performed:
Expand Down Expand Up @@ -192,18 +280,22 @@ def _perform_substitutions(self, context: LaunchContext) -> None:
self.__final_node_name += '/' + self.__expanded_node_name
# expand parameters too
if self.__parameters is not None:
self.__expanded_parameters = []
for param_file_path in self.__parameters:
if isinstance(param_file_path, pathlib.Path):
param_file_path = str(param_file_path)
expanded_param_file_path = perform_substitutions(
context, normalize_to_list_of_substitutions(param_file_path))
if not os.path.isfile(expanded_param_file_path):
self.__expanded_parameter_files = []
for params in self.__parameters:
if isinstance(params, dict):
param_file_path = self._create_params_file_from_dict(context, params)
else:
if isinstance(params, pathlib.Path):
param_file_path = str(params)
else:
param_file_path = perform_substitutions(
context, normalize_to_list_of_substitutions(params))
if not os.path.isfile(param_file_path):
_logger.warn(
'Parameter file path is not a file: {}'.format(expanded_param_file_path))
'Parameter file path is not a file: {}'.format(param_file_path))
# Don't skip adding the file to the parameter list since space has been
# reserved for it in the ros_specific_arguments.
self.__expanded_parameters.append(expanded_param_file_path)
self.__expanded_parameter_files.append(param_file_path)
# expand remappings too
if self.__remappings is not None:
self.__expanded_remappings = []
Expand All @@ -226,8 +318,8 @@ def execute(self, context: LaunchContext) -> Optional[List[Action]]:
ros_specific_arguments.append('__node:={}'.format(self.__expanded_node_name))
if self.__node_namespace is not None:
ros_specific_arguments.append('__ns:={}'.format(self.__expanded_node_namespace))
if self.__expanded_parameters is not None:
for param_file_path in self.__expanded_parameters:
if self.__expanded_parameter_files is not None:
for param_file_path in self.__expanded_parameter_files:
ros_specific_arguments.append('__params:={}'.format(param_file_path))
if self.__expanded_remappings is not None:
for remapping_from, remapping_to in self.__expanded_remappings:
Expand Down
1 change: 1 addition & 0 deletions launch_ros/package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
<depend>lifecycle_msgs</depend>
<depend>osrf_pycommon</depend>
<depend>rclpy</depend>
<depend>python3-yaml</depend>

<test_depend>ament_copyright</test_depend>
<test_depend>ament_flake8</test_depend>
Expand Down
1 change: 1 addition & 0 deletions launch_ros/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
'ament_index_python',
'launch',
'osrf_pycommon',
'pyyaml',
],
zip_safe=True,
author='William Woodall',
Expand Down
1 change: 1 addition & 0 deletions test_launch_ros/package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<test_depend>demo_nodes_py</test_depend>
<test_depend>launch_ros</test_depend>
<test_depend>python3-pytest</test_depend>
<test_depend>python3-yaml</test_depend>

<export>
<build_type>ament_python</build_type>
Expand Down
1 change: 1 addition & 0 deletions test_launch_ros/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
'setuptools',
'demo_nodes_py',
'launch_ros',
'pyyaml',
],
zip_safe=True,
author='William Woodall',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
talker:
ros__parameters:
some_int: 42
a_string: "Hello world"
some_list:
sub_list:
some_integers: [1, 2, 3, 4]
some_doubles : [3.14, 2.718]
/my_ns:
my_node:
ros__parameters:
some_int: 42
a_string: "Hello world"
some_list:
sub_list:
some_integers: [1, 2, 3, 4]
some_doubles : [3.14, 2.718]
Loading