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

Evaluate parameters exception loading URDF #214

Closed
v-lopez opened this issue Jan 20, 2021 · 9 comments
Closed

Evaluate parameters exception loading URDF #214

v-lopez opened this issue Jan 20, 2021 · 9 comments
Assignees

Comments

@v-lopez
Copy link

v-lopez commented Jan 20, 2021

Bug report

Required Info:

  • Operating System: Ubuntu 20.04
  • Installation type: Binaries
  • Version or commit hash: 0.11.1-1focal.20210106.015448
  • DDS implementation: Fast-RTPS

Steps to reproduce issue

A full example is going to take a few files, and the issue is simple enough.

I am trying to load a URDF file into a robot_description argument via Substitutions.

The urdf is generated correctly from the xacro, but it crashes when parsing before writing it to the param file.
The reason seems to be the same as was discussed here: #136

If I try to load this file: https://github.com/pal-robotics/pmb2_robot/blob/kinetic-devel/pmb2_description/gazebo/gazebo.urdf.xacro#L45

The semicolon in the comment causes it to fail with the error:

  File "/opt/ros/foxy/lib/python3.8/site-packages/launch_ros/actions/node.py", line 377, in _perform_substitutions
    evaluated_parameters = evaluate_parameters(context, self.__parameters)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch_ros/utilities/evaluate_parameters.py", line 145, in evaluate_parameters
    output_params.append(evaluate_parameter_dict(context, param))
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch_ros/utilities/evaluate_parameters.py", line 70, in evaluate_parameter_dict
    yaml_evaluated_value = yaml.safe_load(evaluated_value)
  File "/usr/lib/python3/dist-packages/yaml/__init__.py", line 162, in safe_load
    return load(stream, SafeLoader)
  File "/usr/lib/python3/dist-packages/yaml/__init__.py", line 114, in load
    return loader.get_single_data()
  File "/usr/lib/python3/dist-packages/yaml/constructor.py", line 49, in get_single_data
    node = self.get_single_node()
  File "/usr/lib/python3/dist-packages/yaml/composer.py", line 36, in get_single_node
    document = self.compose_document()
  File "/usr/lib/python3/dist-packages/yaml/composer.py", line 58, in compose_document
    self.get_event()
  File "/usr/lib/python3/dist-packages/yaml/parser.py", line 118, in get_event
    self.current_event = self.state()
  File "/usr/lib/python3/dist-packages/yaml/parser.py", line 193, in parse_document_end
    token = self.peek_token()
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 129, in peek_token
    self.fetch_more_tokens()
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 223, in fetch_more_tokens
    return self.fetch_value()
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 577, in fetch_value
    raise ScannerError(None, None,
yaml.scanner.ScannerError: mapping values are not allowed here
  in "<unicode string>", line 33, column 14:
        <!-- todo: this is working with gazebo 1. ... 

Is there an equivalent way of escaping the robot_description text so it's always parsed as string?

@djchopp
Copy link

djchopp commented May 4, 2021

Posting to hopefully help the next person. This issue looks to be resolved with #137 however that has not made it into the foxy branch/release. In the meantime, building from source on foxy using the master branch for both launch and launch_ros allows you to do the following for the python launch system:

#!/usr/bin/env python3

import os

from ament_index_python.packages import get_package_share_directory

from launch import LaunchDescription
from launch.actions import DeclareLaunchArgument
from launch.substitutions import LaunchConfiguration, Command
from launch_ros.actions import Node
from launch_ros.descriptions import ParameterValue  # Need master or Galactic branch for this feature

def generate_launch_description():
  use_sim_time = LaunchConfiguration('use_sim_time')
  model_path = LaunchConfiguration('model_path')
  

  return LaunchDescription([

    DeclareLaunchArgument(
      'use_sim_time',
      default_value="false",
      description='Flag to use simulation time'),   

    DeclareLaunchArgument(
      'model_path',
      default_value=os.path.join(get_package_share_directory('my_package'),'urdf','test.urdf'),
      description='path to urdf'),

    Node(
      package='robot_state_publisher',
      executable='robot_state_publisher',
      name='robot_state_publisher',
      output='screen',
      parameters=[{
        'use_sim_time': use_sim_time,
        'robot_description': ParameterValue(Command(['cat ',model_path]), value_type=str)
      }])
  ])

A xacro could be loaded using ParameterValue(Command(['xacro ',model_path]), value_type=str) as well if you point model_path to the xacro instead.

@shonigmann
Copy link

Just thought I'd share a workaround that works in Foxy now, without the need to checkout galactic/rolling versions of packages, using OpaqueFunction and the xacro python library:

import os
import xacro

from ament_index_python.packages import get_package_share_directory

from launch import LaunchDescription
from launch.actions import DeclareLaunchArgument
from launch.actions import OpaqueFunction
from launch.substitutions import LaunchConfiguration
from launch_ros.actions import Node


# evaluates LaunchConfigurations in context for use with xacro.process_file(). Returns a list of launch actions to be included in launch description
def evaluate_xacro(context, *args, **kwargs):
    use_sim_time = LaunchConfiguration('use_sim_time').perform(context)
    model_path = LaunchConfiguration('model_path').perform(context)
    xacro_prefix = LaunchConfiguration('xacro_prefix').perform(context)
  
    robot_state_publisher_node = Node(
      package='robot_state_publisher',
      executable='robot_state_publisher',
      name='robot_state_publisher',
      output='screen',
      parameters=[{
        'use_sim_time': use_sim_time,
        'robot_description': xacro.process_file(model_path, mappings={'prefix': xacro_prefix}).toxml()
      }])

    return [robot_state_publisher_node]

def generate_launch_description():
  return LaunchDescription([

    DeclareLaunchArgument(
      'use_sim_time',
      default_value="false",
      description='Flag to use simulation time'),   

    DeclareLaunchArgument(
      'model_path',
      default_value=os.path.join(get_package_share_directory('my_package'),'urdf','test.urdf'),
      description='path to urdf'),

    DeclareLaunchArgument(
      'xacro_prefix',
      default_value="my_robot_",
      description='prefix argument input to xacro file'),   
    
    OpaqueFunction(function=evaluate_xacro)  # add OpaqueFunction to evaluate xacro file in context and pass to any nodes that need it
  ])

While it is admittedly a bit harder to read than a typical launch file, it checks all the boxes for my use cases, anyways; namely it:

  • supports xacro argument inputs when parsing a file
  • supports dynamically set paths using LaunchConfigurations
  • supports URDF containing special characters (though I haven't tested this exhaustively)

@jacobperron
Copy link
Member

@v-lopez Can you confirm if this is fixed with the latest changes on the foxy branch for launch and launch_ros?

@v-lopez
Copy link
Author

v-lopez commented Nov 18, 2021

Still getting that error:

Traceback (most recent call last):
  File "/home/user/pal_robots/install/launch/lib/python3.8/site-packages/launch/launch_service.py", line 228, in _process_one_event
    await self.__process_event(next_event)
  File "/home/user/pal_robots/install/launch/lib/python3.8/site-packages/launch/launch_service.py", line 248, in __process_event
    visit_all_entities_and_collect_futures(entity, self.__context))
  File "/home/user/pal_robots/install/launch/lib/python3.8/site-packages/launch/utilities/visit_all_entities_and_collect_futures_impl.py", line 45, in visit_all_entities_and_collect_futures
    futures_to_return += visit_all_entities_and_collect_futures(sub_entity, context)
  File "/home/user/pal_robots/install/launch/lib/python3.8/site-packages/launch/utilities/visit_all_entities_and_collect_futures_impl.py", line 45, in visit_all_entities_and_collect_futures
    futures_to_return += visit_all_entities_and_collect_futures(sub_entity, context)
  File "/home/user/pal_robots/install/launch/lib/python3.8/site-packages/launch/utilities/visit_all_entities_and_collect_futures_impl.py", line 45, in visit_all_entities_and_collect_futures
    futures_to_return += visit_all_entities_and_collect_futures(sub_entity, context)
  [Previous line repeated 3 more times]
  File "/home/user/pal_robots/install/launch/lib/python3.8/site-packages/launch/utilities/visit_all_entities_and_collect_futures_impl.py", line 38, in visit_all_entities_and_collect_futures
    sub_entities = entity.visit(context)
  File "/home/user/pal_robots/install/launch/lib/python3.8/site-packages/launch/action.py", line 108, in visit
    return self.execute(context)
  File "/home/user/pal_robots/install/launch_ros/lib/python3.8/site-packages/launch_ros/actions/node.py", line 440, in execute
    self._perform_substitutions(context)
  File "/home/user/pal_robots/install/launch_ros/lib/python3.8/site-packages/launch_ros/actions/node.py", line 395, in _perform_substitutions
    evaluated_parameters = evaluate_parameters(context, self.__parameters)
  File "/home/user/pal_robots/install/launch_ros/lib/python3.8/site-packages/launch_ros/utilities/evaluate_parameters.py", line 151, in evaluate_parameters
    output_params.append(evaluate_parameter_dict(context, param))
  File "/home/user/pal_robots/install/launch_ros/lib/python3.8/site-packages/launch_ros/utilities/evaluate_parameters.py", line 72, in evaluate_parameter_dict
    yaml_evaluated_value = yaml.safe_load(evaluated_value)
  File "/usr/lib/python3/dist-packages/yaml/__init__.py", line 162, in safe_load
    return load(stream, SafeLoader)
  File "/usr/lib/python3/dist-packages/yaml/__init__.py", line 114, in load
    return loader.get_single_data()
  File "/usr/lib/python3/dist-packages/yaml/constructor.py", line 49, in get_single_data
    node = self.get_single_node()
  File "/usr/lib/python3/dist-packages/yaml/composer.py", line 36, in get_single_node
    document = self.compose_document()
  File "/usr/lib/python3/dist-packages/yaml/composer.py", line 58, in compose_document
    self.get_event()
  File "/usr/lib/python3/dist-packages/yaml/parser.py", line 118, in get_event
    self.current_event = self.state()
  File "/usr/lib/python3/dist-packages/yaml/parser.py", line 193, in parse_document_end
    token = self.peek_token()
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 129, in peek_token
    self.fetch_more_tokens()
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 223, in fetch_more_tokens
    return self.fetch_value()
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 577, in fetch_value
    raise ScannerError(None, None,
yaml.scanner.ScannerError: mapping values are not allowed here
  in "<unicode string>", line 33, column 14:
        <!-- todo: this is working with gazebo 1. ... 
                 ^

With launch_ros at tag 0.11.5 and launch 0.10.7

@jacobperron
Copy link
Member

@v-lopez Can you share the launch file to reproduce the error? I think you should explicitly tell launch that the parameter type is a string, e.g. in Python or in XML you can use the type attribute, e.g.

  <param name="foo" value="bar" type="str" />

@DLu
Copy link
Contributor

DLu commented Dec 6, 2021

I've run into this problem too, and added better error messages in this PR: #275

@DLu
Copy link
Contributor

DLu commented Dec 6, 2021

(and we discussed different ways to fix the problem here #268 but ultimately decided to just force people to explicitly state that its a string.

@jacobperron
Copy link
Member

@v-lopez Friendly ping. Let me know if this is still an issue after trying to explicitly set the type to string. Otherwise, I'm inclined to close this ticket.

@jacobperron
Copy link
Member

Closing, since I believe this issue as been addressed. Please comment or re-open if I'm mistaken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants