Skip to content

Add Capsule primitives#24

Closed
jmirabel wants to merge 1 commit into
ros:masterfrom
jmirabel:master
Closed

Add Capsule primitives#24
jmirabel wants to merge 1 commit into
ros:masterfrom
jmirabel:master

Conversation

@jmirabel

@jmirabel jmirabel commented Apr 9, 2016

Copy link
Copy Markdown

Collision checking with capsules is faster than with cylinder. As FCL has this primitives, I guess it shouldn't be a problem to add them in the URDF format ?
I have made changes in urdfdom packages as well.

@jmirabel

jmirabel commented Apr 9, 2016

Copy link
Copy Markdown
Author

Parsing of capsules: ros/urdfdom#85

@scpeters

Copy link
Copy Markdown
Contributor

The code looks pretty straight-forward since capsules are so similar to cylinders.

My only concern is that adding new fields to urdf could lead to problems when trying to parse new files with an old parser, since the format doesn't have any way of declaring a version.

@jmirabel

Copy link
Copy Markdown
Author

Tinyxml can parse a version number in the xml tag.
From now on, the old parser could give a warning when version number is set and too high and the new parser could fail when version number is not what it expects. But an update of the old parser would be required...

@scpeters

Copy link
Copy Markdown
Contributor

That's the main problem, that the old parser would need to be updated.

@florent-lamiraux

florent-lamiraux commented May 6, 2016

Copy link
Copy Markdown

There are two separate issues :

  1. modifying the URDF language that is a standard,
  2. modifying the parser (urdfdom, urdfdom_headers) so that it parses new primitives.
    I understand that you do not want to do 1, since old versions of the parser will ignore capsules. This pull request however addresses 2. I would then recommend to accept the pull request and to upgrade the language when
    1. new versions of the parser check the version of the files parsed,
    2. old versions without this feature are not supported anymore.

@olivier-stasse

Copy link
Copy Markdown

Dear Joseph and Florent,
Collada accepts capsule as an analytical shape (https://www.khronos.org/files/collada_spec_1_5.pdf).
As collision accepts also link to collada files will this be acceptable ?
It would avoid any modification of the current and past parser.

@jmirabel

jmirabel commented Oct 5, 2016

Copy link
Copy Markdown
Author

This solution is really heavy in terms of development because analytical shapes are not parsed by assimp. I guess there would be some backward compatibility issues in other places in ROS.

@sbarthelemy

Copy link
Copy Markdown

I would also use capsule shapes if it was available, and - by the way - a "rounded box shape" too (or alternatively a "convex hull of spheres shape").

@scpeters could you detail the issue you foresee with old parsers?

AFAICT, when encountering a capsule, the old parser will fail to parse the file with those error logs:
"Unknown geometry type '%s'"
"Could not parse collision element for Link [%s]"

That seems quite sane to me. Is it a blocker?

@wxmerkt

wxmerkt commented Nov 17, 2017

Copy link
Copy Markdown

Hey at all -
I'd like to understand what is currently blocking adding capsules to newer versions of urdfdom as the current parser will already fail and warn if it encounters an unknown tag. Would it be possible to make a new sub-version of URDF to allow capsules? Is there any way we could address these blocking/open points?

Happy to contribute, thank you very much,
Wolfgang

@wxmerkt

wxmerkt commented Apr 3, 2018

Copy link
Copy Markdown

Hey,
I wanted to ping this thread - any input on how we may add Capsules? E.g. by having a higher version number in parser or targeting future release? What could unblock this?

Thank you very much for your input,
Wolfgang

Edit Drake also supports parsing capsule shapes

@sbarthelemy

Copy link
Copy Markdown

Hello, I'd like to see topic move forward too.

FYI: the bullet URDF parser does parse capsule shapes.

@davetcoleman

Copy link
Copy Markdown
Contributor

This seems like a great idea, and something we should add to ROS Melodic. ROS2 is an even easier place to target this.

@sbarthelemy's point about the existing error messages seems compelling enough. @scpeters what is remaining to merge this in?

@jmirabel

Copy link
Copy Markdown
Author

@davetcoleman there is a need to either:

  • make the old parser fail when it encounters a file with a capsule. Maybe, it is sufficient to well document the fact that developers should add "urdfdom >= ..." to their package.xml when using capsules.
  • drop backward compatibility of old parsers.

@traversaro

Copy link
Copy Markdown
Contributor

This seems like a great idea, and something we should add to ROS Melodic. ROS2 is an even easier place to target this.

Note that at the moment, urdfdom_headers and urdfdom for ROS1 distributions are provided as upstream Ubuntu packages, and as long as this is the case, almost any new feature needs to wait for the next Ubuntu LTS release to be used.

make the old parser fail when it encounters a file with a capsule. Maybe, it is sufficient to well document the fact that developers should add "urdfdom >= ..." to their package.xml when using capsules.

Related discussion on URDF format versioning: ros/urdfdom#123 .

@jmirabel

Copy link
Copy Markdown
Author

@traversaro thanks for the insight.

Following discussion in ros/urdfdom#123, is it practicable to define URDF v2 by changing the name of the robot tag into another tag, like robot2 (obviously, this is a silly name to get me understood) such that:

  • the new parser would be able to read tags robot and robot2 correctly
  • the old parser would fail to read tag robot2

@jmirabel

Copy link
Copy Markdown
Author

Tag robot2 could include an attribute version to allow versioning.

@jmirabel

jmirabel commented Jun 7, 2019

Copy link
Copy Markdown
Author

Dear @scpeters,
do you think it is possible to add a tag (e.g. urdf or device) that would be the successor of tag robot ?

@Levi-Armstrong

Copy link
Copy Markdown

What is left to be done to get this merged? I may have resources to address the remaining items as we are interested in adding several new types shown in PR #54 #55 #56 #57.

@davetcoleman

Copy link
Copy Markdown
Contributor

Note that at the moment, urdfdom_headers and urdfdom for ROS1 distributions are provided as upstream Ubuntu packages, and as long as this is the case, almost any new feature needs to wait for the next Ubuntu LTS release to be used.

Yes but this has been the argument for the last ~8 years, so it just remains unchanged. It would still be nice to make changes for the next LTS.

You also are hinting at the possibility of adding urdfdom back into the OSRF-hosted build farm / PPA, which I support.

@davetcoleman davetcoleman mentioned this pull request Aug 22, 2019
@jmirabel

jmirabel commented Apr 3, 2020

Copy link
Copy Markdown
Author

I didn't follow closely. From #59, it seems that versioning has been added somewhere. Any knows what should be done to update this PR to get it accepted ?

@Aetherbase

Copy link
Copy Markdown

Hello, is there a reason on why is this not merged yet?

@jmirabel

Copy link
Copy Markdown
Author

Closing this since there are very little chances this gets merge as is after 6 years.

@jmirabel jmirabel closed this Aug 17, 2022
@Levi-Armstrong

Copy link
Copy Markdown

If anyone is interested Tesseract has a urdf parser which includes additional shapes like convex mesh, cone, capsule, octomap, etc. and also supports Quaternion for origin tag and acceleration limits.

https://github.com/tesseract-robotics/tesseract/tree/master/tesseract_urdf

https://tesseract-docs.readthedocs.io/en/latest/_source/core/packages/tesseract_urdf_doc.html

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.

10 participants