Fix problems when parsing a Command Substitution as a parameter value (#137)

Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Author: ivanpauno

launch_ros/launch_ros/actions/node.py

@@ -23,6 +23,7 @@
 from typing import Optional
 from typing import Text  # noqa: F401
 from typing import Tuple  # noqa: F401
+from typing import TYPE_CHECKING
 from typing import Union
 
 import warnings
@@ -32,11 +33,12 @@
 from launch.frontend import Entity
 from launch.frontend import expose_action
 from launch.frontend import Parser
+from launch.frontend.type_utils import get_data_type_from_identifier
+
 from launch.launch_context import LaunchContext
 import launch.logging
 from launch.some_substitutions_type import SomeSubstitutionsType
 from launch.substitutions import LocalSubstitution
-from launch.substitutions import TextSubstitution
 from launch.utilities import ensure_argument_type
 from launch.utilities import normalize_to_list_of_substitutions
 from launch.utilities import perform_substitutions
@@ -57,6 +59,9 @@
 
 import yaml
 
+if TYPE_CHECKING:
+    from ..descriptions import Parameter
+
 
 @expose_action('node')
 class Node(ExecuteProcess):
@@ -159,6 +164,7 @@ def __init__(
                     "Only use 'executable'"
                 )
             executable = node_executable
+
         if package is not None:
             cmd = [ExecutableInPackage(package=package, executable=executable)]
         else:
@@ -207,7 +213,7 @@ def __init__(
 
         self.__expanded_node_name = self.UNSPECIFIED_NODE_NAME
         self.__expanded_node_namespace = self.UNSPECIFIED_NODE_NAMESPACE
-        self.__expanded_parameter_files = None  # type: Optional[List[Text]]
+        self.__expanded_parameter_arguments = None  # type: Optional[List[Tuple[Text, bool]]]
         self.__final_node_name = None  # type: Optional[Text]
         self.__expanded_remappings = None  # type: Optional[List[Tuple[Text, Text]]]
 
@@ -218,27 +224,28 @@ def __init__(
     @staticmethod
     def parse_nested_parameters(params, parser):
         """Normalize parameters as expected by Node constructor argument."""
+        from ..descriptions import ParameterValue
+
         def get_nested_dictionary_from_nested_key_value_pairs(params):
             """Convert nested params in a nested dictionary."""
             param_dict = {}
             for param in params:
                 name = tuple(parser.parse_substitution(param.get_attr('name')))
-                value = param.get_attr('value', data_type=None, optional=True)
+                type_identifier = param.get_attr('type', data_type=None, optional=True)
+                data_type = None
+                if type_identifier is not None:
+                    data_type = get_data_type_from_identifier(type_identifier)
+                value = param.get_attr('value', data_type=data_type, optional=True)
                 nested_params = param.get_attr('param', data_type=List[Entity], optional=True)
                 if value is not None and nested_params:
-                    raise RuntimeError('param and value attributes are mutually exclusive')
+                    raise RuntimeError(
+                        'nested parameters and value attributes are mutually exclusive')
+                if data_type is not None and nested_params:
+                    raise RuntimeError(
+                        'nested parameters and type attributes are mutually exclusive')
                 elif value is not None:
-                    def normalize_scalar_value(value):
-                        if isinstance(value, str):
-                            value = parser.parse_substitution(value)
-                            if len(value) == 1 and isinstance(value[0], TextSubstitution):
-                                value = value[0].text  # python `str` are not converted like yaml
-                        return value
-                    if isinstance(value, list):
-                        value = [normalize_scalar_value(x) for x in value]
-                    else:
-                        value = normalize_scalar_value(value)
-                    param_dict[name] = value
+                    some_value = parser.parse_if_substitutions(value)
+                    param_dict[name] = ParameterValue(some_value, value_type=data_type)
                 elif nested_params:
                     param_dict.update({
                         name: get_nested_dictionary_from_nested_key_value_pairs(nested_params)
@@ -325,7 +332,13 @@ def _create_params_file_from_dict(self, params):
             yaml.dump(param_dict, h, default_flow_style=False)
             return param_file_path
 
+    def _get_parameter_rule(self, param: 'Parameter', context: LaunchContext):
+        name, value = param.evaluate(context)
+        return f'{name}:={yaml.dump(value)}'
+
     def _perform_substitutions(self, context: LaunchContext) -> None:
+        # Here to avoid cyclic import
+        from ..descriptions import Parameter
         try:
             if self.__substitutions_performed:
                 # This function may have already been called by a subclass' `execute`, for example.
@@ -365,31 +378,36 @@ def _perform_substitutions(self, context: LaunchContext) -> None:
         # so they can be overriden with specific parameters of this Node
         global_params = context.launch_configurations.get('ros_params', None)
         if global_params is not None or self.__parameters is not None:
-            self.__expanded_parameter_files = []
+            self.__expanded_parameter_arguments = []
         if global_params is not None:
             param_file_path = self._create_params_file_from_dict(global_params)
-            self.__expanded_parameter_files.append(param_file_path)
+            self.__expanded_parameter_arguments.append((param_file_path, True))
             cmd_extension = ['--params-file', f'{param_file_path}']
             self.cmd.extend([normalize_to_list_of_substitutions(x) for x in cmd_extension])
             assert os.path.isfile(param_file_path)
         # expand parameters too
         if self.__parameters is not None:
             evaluated_parameters = evaluate_parameters(context, self.__parameters)
-            for i, params in enumerate(evaluated_parameters):
+            for params in evaluated_parameters:
+                is_file = False
                 if isinstance(params, dict):
-                    param_file_path = self._create_params_file_from_dict(params)
-                    assert os.path.isfile(param_file_path)
+                    param_argument = self._create_params_file_from_dict(params)
+                    is_file = True
+                    assert os.path.isfile(param_argument)
                 elif isinstance(params, pathlib.Path):
-                    param_file_path = str(params)
+                    param_argument = str(params)
+                    is_file = True
+                elif isinstance(params, Parameter):
+                    param_argument = self._get_parameter_rule(params, context)
                 else:
                     raise RuntimeError('invalid normalized parameters {}'.format(repr(params)))
-                if not os.path.isfile(param_file_path):
+                if is_file and not os.path.isfile(param_argument):
                     self.__logger.warning(
-                        'Parameter file path is not a file: {}'.format(param_file_path),
+                        'Parameter file path is not a file: {}'.format(param_argument),
                     )
                     continue
-                self.__expanded_parameter_files.append(param_file_path)
-                cmd_extension = ['--params-file', f'{param_file_path}']
+                self.__expanded_parameter_arguments.append((param_argument, is_file))
+                cmd_extension = ['--params-file' if is_file else '-p', f'{param_argument}']
                 self.cmd.extend([normalize_to_list_of_substitutions(x) for x in cmd_extension])
         # expand remappings too
         global_remaps = context.launch_configurations.get('ros_remaps', None)

launch_ros/launch_ros/descriptions/__init__.py

@@ -15,7 +15,12 @@
 """descriptions Module."""
 
 from .composable_node import ComposableNode
+from ..parameter_descriptions import Parameter
+from ..parameter_descriptions import ParameterValue
+
 
 __all__ = [
     'ComposableNode',
+    'Parameter',
+    'ParameterValue',
 ]

launch_ros/launch_ros/parameter_descriptions.py

@@ -0,0 +1,156 @@
+# Copyright 2020 Open Source Robotics Foundation, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Module for a description of a Parameter."""
+
+from typing import List
+from typing import Optional
+from typing import Text
+from typing import Tuple
+from typing import TYPE_CHECKING
+from typing import Union
+
+from launch import LaunchContext
+from launch import SomeSubstitutionsType
+from launch import SomeSubstitutionsType_types_tuple
+from launch.substitution import Substitution
+from launch.utilities import ensure_argument_type
+from launch.utilities import normalize_to_list_of_substitutions
+from launch.utilities import perform_substitutions
+from launch.utilities.type_utils import AllowedTypesType
+from launch.utilities.type_utils import normalize_typed_substitution
+from launch.utilities.type_utils import perform_typed_substitution
+from launch.utilities.type_utils import SomeValueType
+
+if TYPE_CHECKING:
+    from .parameters_type import EvaluatedParameterValue
+
+
+class ParameterValue:
+    """Describes a ROS parameter value."""
+
+    def __init__(
+        self,
+        value: SomeValueType,
+        *,
+        value_type: Optional[AllowedTypesType] = None
+    ) -> None:
+        """
+        Construct a parameter value description.
+
+        :param value: Value or substitution that can be resolved to a value.
+        :param value_type: Used when `value` is a substitution, to coerce the result.
+            Can be one of:
+                - Scalar types: int, bool, float, str
+                - List types: List[int], List[bool], List[float], List[str]
+                - None: will use yaml rules, and check that the result matches one of the above.
+        """
+        self.__value = normalize_typed_substitution(value, value_type)
+        self.__value_type = value_type
+        self.__evaluated_parameter_value: Optional['EvaluatedParameterValue'] = None
+
+    @property
+    def value(self) -> SomeValueType:
+        """Getter for parameter value."""
+        if self.__evaluated_parameter_value is not None:
+            return self.__evaluated_parameter_value
+        return self.__value
+
+    @property
+    def value_type(self) -> AllowedTypesType:
+        """Getter for parameter value type."""
+        return self.__value_type
+
+    def __str__(self) -> Text:
+        return (
+            'launch_ros.description.ParameterValue'
+            f'(value={self.value}, value_type={self.value_type})'
+        )
+
+    def evaluate(self, context: LaunchContext) -> 'EvaluatedParameterValue':
+        """Evaluate and return parameter rule."""
+        self.__evaluated_parameter_value = perform_typed_substitution(
+            context, self.value, self.value_type)
+        return self.__evaluated_parameter_value
+
+
+class Parameter:
+    """Describes a ROS parameter."""
+
+    def __init__(
+        self,
+        name: SomeSubstitutionsType,
+        value: SomeValueType,
+        *,
+        value_type: Optional[AllowedTypesType] = None
+    ) -> None:
+        """
+        Construct a parameter description.
+
+        :param name: Name of the parameter.
+        :param value: Value of the parameter.
+        :param value_type: Used when `value` is a substitution, to coerce the result.
+            Can be one of:
+                - A scalar type: `int`, `str`, `float`, `bool`.
+                  `bool` are written like in `yaml`.
+                  Both `1` and `1.` are valid floats.
+                - An uniform list: `List[int]`, `List[str]`, `List[float]`, `List[bool]`.
+                  Lists are written like in `yaml`.
+                - `None`, which means that yaml rules will be used.
+                  The result of the convertion must be one of the above types,
+                  if not `ValueError` is raised.
+            If value is not a substitution and this parameter is provided,
+            it will be used to check `value` type.
+        """
+        ensure_argument_type(name, SomeSubstitutionsType_types_tuple, 'name')
+
+        self.__name = normalize_to_list_of_substitutions(name)
+        self.__parameter_value = ParameterValue(value, value_type=value_type)
+        self.__evaluated_parameter_name: Optional[Text] = None
+        self.__evaluated_parameter_rule: Optional[Tuple[Text, 'EvaluatedParameterValue']] = None
+
+    @property
+    def name(self) -> Union[List[Substitution], Text]:
+        """Getter for parameter name."""
+        if self.__evaluated_parameter_name is not None:
+            return self.__evaluated_parameter_name
+        return self.__name
+
+    @property
+    def value(self) -> SomeValueType:
+        """Getter for parameter value."""
+        return self.__parameter_value.value
+
+    @property
+    def value_type(self) -> AllowedTypesType:
+        """Getter for parameter value type."""
+        return self.__parameter_value.value_type
+
+    def __str__(self) -> Text:
+        return (
+            'launch_ros.description.Parameter'
+            f'(name={self.name}, value={self.value}, value_type={self.value_type})'
+        )
+
+    def evaluate(self, context: LaunchContext) -> Tuple[Text, 'EvaluatedParameterValue']:
+        """Evaluate and return parameter rule."""
+        if self.__evaluated_parameter_rule is not None:
+            return self.__evaluated_parameter_rule
+
+        name = perform_substitutions(context, self.name)
+        value = self.__parameter_value.evaluate(context)
+
+        self.__evaluated_parameter_name = name
+        self.__evaluated_parameter_rule = (name, value)
+        return (name, value)

launch_ros/launch_ros/parameters_type.py

@@ -23,42 +23,59 @@
 from typing import Union
 
 from launch.some_substitutions_type import SomeSubstitutionsType
+from launch.some_substitutions_type import SomeSubstitutionsType_types_tuple
 from launch.substitution import Substitution
 
+from .parameter_descriptions import Parameter as ParameterDescription
+from .parameter_descriptions import ParameterValue as ParameterValueDescription
+
 
 # Parameter value types used below
+_SingleValueType_types_tuple = (str, int, float, bool)
 _SingleValueType = Union[str, int, float, bool]
 _MultiValueType = Union[
     Sequence[str], Sequence[int], Sequence[float], Sequence[bool], bytes]
 
 SomeParameterFile = Union[SomeSubstitutionsType, pathlib.Path]
 SomeParameterName = Sequence[Union[Substitution, str]]
-SomeParameterValue = Union[SomeSubstitutionsType,
-                           Sequence[SomeSubstitutionsType],
-                           _SingleValueType,
-                           _MultiValueType]
+SomeParameterValue = Union[
+    ParameterValueDescription,
+    SomeSubstitutionsType,
+    Sequence[SomeSubstitutionsType],
+    _SingleValueType,
+    _MultiValueType
+]
+SomeParameterValue_types_tuple = (
+    SomeSubstitutionsType_types_tuple +
+    _SingleValueType_types_tuple +
+    (bytes,)
+)
 
 # TODO(sloretz) Recursive type when mypy supports them python/mypy#731
 _SomeParametersDict = Mapping[SomeParameterName, Any]
 SomeParametersDict = Mapping[SomeParameterName, Union[SomeParameterValue, _SomeParametersDict]]
 
 # Paths to yaml files with parameters, or dictionaries of parameters, or pairs of
 # parameter names and values
-SomeParameters = Sequence[Union[SomeParameterFile, Mapping[SomeParameterName, SomeParameterValue]]]
+SomeParameters = Sequence[Union[SomeParameterFile, ParameterDescription, SomeParametersDict]]
 
 ParameterFile = Sequence[Substitution]
 ParameterName = Sequence[Substitution]
-ParameterValue = Union[Sequence[Substitution],
-                       Sequence[Sequence[Substitution]],
-                       _SingleValueType,
-                       _MultiValueType]
+ParameterValue = Union[
+    Sequence[Substitution],
+    Sequence[Sequence[Substitution]],
+    _SingleValueType,
+    _MultiValueType,
+    ParameterValueDescription]
 
 # Normalized (flattened to avoid having a recursive type) parameter dict
 ParametersDict = Dict[ParameterName, ParameterValue]
 
 # Normalized parameters
-Parameters = Sequence[Union[ParameterFile, ParametersDict]]
+Parameters = Sequence[Union[ParameterFile, ParametersDict, ParameterDescription]]
 
 EvaluatedParameterValue = Union[_SingleValueType, _MultiValueType]
 # Evaluated parameters: filenames or dictionary after substitutions have been evaluated
-EvaluatedParameters = Sequence[Union[pathlib.Path, Dict[str, EvaluatedParameterValue]]]
+EvaluatedParameters = Sequence[
+    Union[pathlib.Path, ParameterDescription, Dict[str, EvaluatedParameterValue]]
+]