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

[Bug] String parameter parsed as boolean (when passed as a dictionary to a Node in a launch file) #410

Open
beltransen opened this issue Sep 5, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@beltransen
Copy link

Bug report

Required Info:

  • Operating System:
    • Ubuntu 22.04
  • Installation type:
    • Binaries
  • Version or commit hash:
    • 0.19.7-2jammy.20240728.221802
  • DDS implementation:
    • N/A
  • Client library (if applicable):
    • N/A

Steps to reproduce issue

When launching a node using a python launch file passing parameters as a dict, there is a problem parsing the string values 'y' and 'n', as they are interpreted as boolean values.

For instance, the following launch file my_launch.py tries to spawn a node from the pcl_ros package that can receive a string parameter filter_field_name:

from launch import LaunchDescription
from launch_ros.actions import Node

def generate_launch_description():
    return LaunchDescription([
        Node(
		package='pcl_ros',
		executable='filter_passthrough_node',
		name='passthrough_y',
		output='screen',
		parameters=[{
		    'filter_field_name': 'y'
		}]
	    )
    ])

If the value of the parameter is any other than 'y' or 'n', the node starts correctly. However, if any of those values is passed, the following error arises:
[INFO] [filter_passthrough_node-1]: process started with pid [25858] [filter_passthrough_node-1] terminate called after throwing an instance of 'rclcpp::exceptions::InvalidParameterTypeException' [filter_passthrough_node-1] what(): parameter 'filter_field_name' has invalid type: Wrong parameter type, parameter {filter_field_name} is of type {string}, setting it to {bool} is not allowed. [ERROR] [filter_passthrough_node-1]: process has died [pid 25858, exit code -6, cmd '/home/beltransen/ros2_ws/install/pcl_ros/lib/pcl_ros/filter_passthrough_node --ros-args -r __node:=passthrough_y --params-file /tmp/launch_params_lirux2gp'].

The content of the temporary .yaml file (/tmp/launch_params_lirux2gp) generated from the dictionary of parameters and passed to the node is:

/**:
  ros__parameters:
    filter_field_name: y

The problem here is that, during the conversion, the 'y' and 'n' strings are inserted into the .yaml file without single quotes, unlike any other string value, which preserves the quotes and, therefore, can be correctly parsed.

If I manually create a .yaml file keeping the single quotes:

/**:
   ros__parameters:
    filter_field_name: 'y'

And adapt the launch file to read the parameters from the .yaml file instead of passing them as a dictionary:

from launch import LaunchDescription
from launch_ros.actions import Node

def generate_launch_description():
    return LaunchDescription([
        Node(
		package='pcl_ros',
		executable='filter_passthrough_node',
		name='passthrough_y',
		output='screen',
		parameters=["/path/to/config.yaml"]
	    )
    ])

The node starts correctly.

Expected behavior

The temporary .yaml file created from parameters passed as a dictionary should preserve the quotes of all string parameters.

Actual behavior

The temporary .yaml file created from parameters passed as a dictionary removes the quotes of string parameters when taking 'y' or 'n' values. Afterwards, when the .yaml file is read, those values are interpreted as booleans.

@pierrelouismelin
Copy link

Hi,

I also stumbled on this problem, and found a fix.

First, I created a new substitution class: TextsSubstitution. It takes an array of Text-like types and substitutions to produce a string concatenating all passed parameters.

from typing import List, Union, Text, Iterable
import collections.abc

from launch.substitutions import TextSubstitution
from launch.substitution import Substitution
from launch.some_substitutions_type import SomeSubstitutionsType
from launch.utilities import ensure_argument_type
from launch.utilities.class_tools_impl import is_a_subclass
from launch.launch_context import LaunchContext

class TextsSubstitution(Substitution):
  """Substitution that concatenates a list of string text and substitutions."""

  def __init__(self, *, texts: SomeSubstitutionsType) -> None:
    """Create a TextsSubstitution."""
    super().__init__()

    ensure_argument_type(
        texts,
        (str, Substitution, collections.abc.Iterable),
        'texts',
        'TextsSubstitution')

    from launch.utilities import normalize_to_list_of_substitutions
    self.__texts = normalize_to_list_of_substitutions(texts)

  @classmethod
  def parse(cls, data: Iterable[SomeSubstitutionsType]):
    """Parse `TextsSubstitution` substitution."""
    if len(data) != 1:
      raise TypeError('eval substitution expects 1 argument')
    return cls, {'expression': data[0]}

  @property
  def texts(self) -> List[Substitution]:
    """Getter for expression."""
    return self.__texts

  def describe(self) -> Text:
    """Return a description of this substitution as a string."""
    return 'Texts({})'.format(' + '.join([sub.describe() for sub in self.texts]))

  def perform(self, context: LaunchContext) -> Text:
    """Perform the substitution by concatenating the texts."""
    from launch.utilities import perform_substitutions
    return perform_substitutions(context, self.texts)

In our case, a simpler substitution can be created that just ensures single quotes are surrounding the given text.

class RawTextSubstitution(Substitution):
  """Substitution that produces a single-quoted string from the given text."""

  def __init__(self, *, text: Union[Text, Substitution]) -> None:
    """Create a RawTextSubstitution."""
    super().__init__()

    ensure_argument_type(
        text,
        (str, Substitution),
        'text',
        'RawTextSubstitution')

    if isinstance(text, str):
      self.__text = TextSubstitution(text=text)
    elif is_a_subclass(text, Substitution):
      self.__text = text
    else:
      raise TypeError(
          "Failed to normalize given item of type '{}', when only "
          "'str' or 'launch.Substitution' were expected.".format(type(text)))

  @property
  def text(self) -> Substitution:
    """Getter for expression."""
    return self.__text

  def describe(self) -> Text:
    """Return a description of this substitution as a string."""
    return 'Texts({})'.format(' + '.join(self.text.describe()))

  def perform(self, context: LaunchContext) -> Text:
    """Perform the substitution by concatenating the texts."""
    from launch.utilities import perform_substitutions
    quote_sub = TextSubstitution(text="'")
    return perform_substitutions(context, [quote_sub, self.text, quote_sub])

Having this substitution, you can easily use it to declare safely a string in parameters of a node:

def generate_launch_description():
  node_name = LaunchConfiguration('node_name')
  node_name_arg = DeclareLaunchArgument(
      'node_name',
      default_value='a_name'
  )
  a_string = LaunchConfiguration('a_string ')
  a_string_arg= DeclareLaunchArgument(
      'a_string ',
      default_value='yes'
  )

  node = Node(
      package="my_package",
      executable="my_node_exe",
      name=TextsSubstitution(texts=["my_prefix_", node_name]),
      parameters=[{
              "first_param": RawTextSubstitution(text='false'),
              "second_param": RawTextSubstitution(text=a_string),
      }],
  )

  return LaunchDescription(
      [node_name_arg, a_string_arg, node])

And then, to launch it:

ros2 launch my_package my_launch.py node_name:=hello a_string:=no

I think many default substitutions classes could be added to the ROS2 launch library to facilitate the use of launch files.

I hope this helps.

@mjcarroll mjcarroll added the bug Something isn't working label Nov 8, 2024
@MichaelOrlov
Copy link

@wjwwood a friendly ping to try to make a preliminary analysis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants