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

Ros2 #203

Closed
wants to merge 9 commits into from
Closed

Ros2 #203

wants to merge 9 commits into from

Conversation

vandanamandlik
Copy link

@vandanamandlik vandanamandlik commented Dec 19, 2018

Ported xacro package to ROS2 and tested it on ROS2 version bouncy and crystal.
@rhaschke, Could you please review it and give your feedback ?

- Moved contents from "src/xacro/" folder to "xacro/" folder.
- Renamed scripts folder to resource folder.
- Modifed package.xml and setup.py according to ROS2 rules.
- Deleted CMakeLists.txt
- Modified all python scripts as per ros2 coding guidelines.
- Added substitution_args.py file, replacement for roslaunch's resolve_args() method,
  as it is not available in ROS2's ros2launch package.
  - Updated shebang line in all python scripts to use python3.
  - Resolved errors where some test cases were failing to find out some .xml files. (eg. include1.xml)
  - Added temporary workaround for rosgraph.names.load_mappings because rosgraph is not migrated to ROS2.
- Added some more details in README.md file.
@darkdragon-001
Copy link

Is there any chance this is reviewed and released soon?

@rhaschke
Copy link
Contributor

rhaschke commented Jul 5, 2019

Sorry for being unresponsive on this. I am too busy with other projects and I didn't familiarize myself with ROS2 yet. Hopefully, I can find some time during my summer holidays, i.e. during the next 4 weeks to work on this.
To facilitate the review and merge process, I suggest to split this large PR into individual commits as follows:

  • ROS-version agnostic, i.e. general improvements of xacro
    I already have put some effort into making xacro python2/3 agnostic as well as to use it without ROS at all. These properties should be maintained.
    • cosmetics like flake, pep
  • ROS2-specific changes

Would be great, if you, @vandanamandlik or @kazuki0824, could prepare this.

@schornakj
Copy link

Is there an example somewhere showing how to parse a URDF xacro file in ROS 2, for example from within a .launch.py script?

@mogumbo
Copy link

mogumbo commented Jul 29, 2019

I would also like to see an example of using a xacro file in a launch.py script. I have been trying to use it in dashing like this:
xacro_name = os.path.join(get_package_share_directory('my_package'), 'urdf/my_robot.xacro')
urdf = xacro.process_file(xacro_name)

This errors on a call to get_path() in substitution_args.py. I can get past that by importing rospkg and uncommenting some code in _get_rospack(). But then I'm just stuck on another error.

@schornakj
Copy link

I would also like to see an example of using a xacro file in a launch.py script. I have been trying to use it in dashing like this:
xacro_name = os.path.join(get_package_share_directory('my_package'), 'urdf/my_robot.xacro')
urdf = xacro.process_file(xacro_name)

This errors on a call to get_path() in substitution_args.py. I can get past that by importing rospkg and uncommenting some code in _get_rospack(). But then I'm just stuck on another error.

Your method should work, but the underlying issue (as @rhaschke mentioned) is that the version of substitution_args.py included in this PR isn't really ROS2 compatible. Most robot .urdf.xacro files heavily rely on substitution to find things like collision meshes in support packages, and since substitution_args.py is still using Catkin methods in some places it makes sense that it wouldn't work in a ROS2 system.

@gonzodepedro
Copy link

Changes on #211

@rhaschke rhaschke mentioned this pull request Sep 25, 2019
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

Successfully merging this pull request may close these issues.

6 participants