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

robot_state_publisher in Humble breaks with version="2" attribute specified in robot tag #937

Closed
smith-doug opened this issue Sep 12, 2023 · 4 comments

Comments

@smith-doug
Copy link

smith-doug commented Sep 12, 2023

In Humble, they added a check to urdf_parser for <robot version=""> that will break if the version is anything besides "1.0". https://github.com/ros/urdfdom/blob/humble/urdf_parser/src/model.cpp#L129 The version tag is already defined as a string.

I need to use version="2" to load collision geometry from the urdf without it being turned into a convex mesh.
robot_state_publisher uses urdf_parser and breaks when it sees this tag.

I think it makes sense to rename the tag that tesseract_urdf::parseURDFString looks for.

@Levi-Armstrong
Copy link
Contributor

Levi-Armstrong commented Sep 15, 2023

I think that makes sense and doubt very many are using the version for switching functionality at the moment.

@rjoomen
Copy link
Contributor

rjoomen commented Oct 11, 2023

I'm using version 2.0, as I am loading some collision meshes in full detail to be able to perform convex decomposition. If this functionality got dropped, though, I could always use the visual mesh as a source. But I think the real issue is the collision mesh tags in what Tesseract calls version 1 being silently converted to convex hulls, which deviates from the URDF specs.

Dropping the version tag, and making never convert would be a breaking change with older Tesseract URDFs, where users might expect collision meshes to be converted. This does make Tesseract respect the URDF standard again, though, where works as expected and conversion is done using the Tesseract-specific <convex_mesh> type.

@marrts
Copy link
Contributor

marrts commented Jan 12, 2024

What if we added a tag to where the mesh was defined and make it default to detailed mesh?
So you go from

<geometry>
  <mesh filename="package://file/path/to/mesh.stl" />
</geometry>

to

<geometry>
  <mesh filename="package://file/path/to/mesh.stl" type="convex"/>
</geometry>

And we can default collision to convex hull for now and then enforce the version 2.0 like ROS2 is doing now. Or we can not enforce it and give a deprecation warning.

This wouldn't throw any errors that I can find with the URDF parser that @smith-doug pointed to.

You could even take it a step further and allow for more control

<geometry> <!-- this might take a while at launch though-->
  <mesh filename="package://file/path/to/mesh.stl" type="convex_decomposition"> 
    <params max="64" resolution="400000" ... />
  </mesh>
</geometry>

or

<geometry>
  <mesh filename="package://file/path/to/mesh.stl" type="octomap">
    <params resolution="0.01" shape="box" />
  </mesh>
</geometry>

@Levi-Armstrong
Copy link
Contributor

Addressed by #979

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

4 participants