Skip to content

Update to make URDF usable in ROS2 - #2

Open
RMichaelSwan wants to merge 2 commits into
nasa-jpl:mainfrom
RMichaelSwan:ros2
Open

Update to make URDF usable in ROS2#2
RMichaelSwan wants to merge 2 commits into
nasa-jpl:mainfrom
RMichaelSwan:ros2

Conversation

@RMichaelSwan

@RMichaelSwan RMichaelSwan commented Aug 21, 2024

Copy link
Copy Markdown

Fixed a number of issues with existing URDFs such as:

  • Relative paths. Replaced with package relative paths (package:// instead of ./). Not sure if this would cause issues with other tools, but it does work with rviz2, robot_state_publisher, and joint_state_publisher in the context of a ROS2 workspace (and should work with ROS1 also).
  • Incorrect/bad limits (replaced huge numbers with more sensible limits, though not verified with actuals).
  • Incorrect joint type for drive joints (wheels, propellers). Changed from revolute to continuous.
  • Tabs are not able to be ingested by robot_state_publisher, replaced with spaces.
  • Colons inside comments cause issues with robot_state_publisher, replaced with dashes.

Also converted into ROS2 compatible package and provided a few examples for launching with rviz2.

@gkjohnson

Copy link
Copy Markdown
Collaborator

Hi @RMichaelSwan - sorry I missed this PR. There are a lot of changes here and I'd prefer not to add too much ROS-specific logic here if we can avoid it. Do you mind splitting this PR up into two separate ones? These changes I think are good changes to make:

  • Incorrect/bad limits (replaced huge numbers with more sensible limits, though not verified with actuals).
  • Incorrect joint type for drive joints (wheels, propellers). Changed from revolute to continuous.
  • (☝️ lets change the huge joint limits to continuous)
  • Colons inside comments cause issues with robot_state_publisher, replaced with dashes.

Incorrect/bad limits (replaced huge numbers with more sensible limits, though not verified with actuals).

Can you explain what you mean by "actuals" in this case?

Tabs are not able to be ingested by robot_state_publisher, replaced with spaces.

To be honest I would report this as a bug to ROS. Tabs are considered valid white space in XML. ROS not supporting them is objectively wrong.

Relative paths. Replaced with package relative paths (package:// instead of ./).

Can you explain the need for this? Do relative paths really not work in ROS?

@RMichaelSwan

RMichaelSwan commented Feb 20, 2025

Copy link
Copy Markdown
Author
  1. Limits:

Can you explain what you mean by "actuals" in this case?

By "actuals" I'm referring to what the rover joint limits would actually be. In this case I set limits that prevent wheels and arms from phasing through part of the body, which is why I opted to set them to revolute in those cases.

  1. Tabs vs spaces

To be honest I would report this as a bug to ROS. Tabs are considered valid white space in XML. ROS not supporting them is objectively wrong.

While I agree that tabs are to spec and rviz2 / ROS2 should handle them correctly, this may affect more than one tool (perhaps also the robot_state_publisher) and I'm not super interested in putting in multiple issues / PRs for them for this one URDF.

  1. Relative paths

Can you explain the need for this? Do relative paths really not work in ROS?

I believe I was having issues with rviz2 not being able to use the relative paths in some cases since the URDF folder context and the content thereof may be loaded into a rosparam. Not sure of a better way to handle this.

  1. Splitting PR:

Where would the split be? Basically a ROS2 adaptation PR and a PR for the URDF changes? One idea could be to have a ros2 branch in your repo if you don't want to change the main branch for some of these things.

@gkjohnson

Copy link
Copy Markdown
Collaborator

I'm not super interested in putting in multiple issues / PRs for them for this one URDF.

I agree it's a pain to track down everywhere this might be broken but one should be made for at least one of the cases you've seen. In the mean time we can change from tabs to spaces.

Perhaps I should give up on hoping on seeing any progress in the ROS ecosystem but these kind of poor software practices make it a chore to work with and limit the interoperation with broader ecosystem tools. It's unfortunate that file format specifications seem to mean nothing in ROS...

I believe I was having issues with rviz2 not being able to use the relative paths in some cases since the URDF folder context and the content thereof may be loaded into a rosparam. Not sure of a better way to handle this.

Looking around it looks like relative paths are generally not supported in ROS, either, which is also disappointing to see 😅 This change will break other tools and demos so I'll need to think about this a bit more.

Where would the split be? Basically a ROS2 adaptation PR and a PR for the URDF changes? One idea could be to have a ros2 branch in your repo if you don't want to change the main branch for some of these things.

Maybe a ros2 branch makes most sense for now? I think we can make some of the non-breaking, non-ROS-ecosystem-specific URDF changes in the main branch to keep the files as close as possible. Eg in the main branch:

  • Incorrect/bad limits (replaced huge numbers with more sensible limits)
  • Incorrect joint type for drive joints (wheels, propellers). Changed from revolute to continuous.
  • Tabs are not able to be ingested by robot_state_publisher, replaced with spaces.
  • Colons inside comments cause issues with robot_state_publisher, replaced with dashes.

And in the ros2 branch:

  • package:// prefix usage
  • Other ros build and startup files

I appreciate the changes here, by the way. Thanks for the contribution!

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.

2 participants