-
Notifications
You must be signed in to change notification settings - Fork 79
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
Forking URDFDom? #59
Comments
There are two different issues to be discussed:
I cannot help for point 2. I can give my two cents on point 1 and summarize what has been said in #24 .
I fully align with @scpeters points: An old parser should fail verbosely on a file with a too recent format version, without requiring to update the old parser. This was the blocking point for #24. Some months (or years) after opening the PR, I proposed a solution which consists in:
I insist on this proposition because I prefer a consensus more than a fork. |
Let me first apologize for not responding to these pull requests that add features to urdfdom. Let me try to communicate my thoughts on some of these challenges: I've spoken before about the issues with old parsers not recognizing new fields (#24 (comment)). There is a Another challenge to adding features to For sdformat, we are discussing the use of xml namespaces for custom elements and prefixes: |
I think we fully understand the constraints of maintaining such a key package. However, we should enter discussions how to address and overcome these concerns to allow some progress and avoid alternative forks. Hopefully, we can start this process at ROSCon. |
I personally fully agree with @davetcoleman suggestion that the URDF format should be allowed to evolve, and in particular with @jmirabel proposal. On the top of that, I think some points that should be addressed in the discussion are the following:
There are been a few related discussions in the past that I think could be interesting for people reading this thread: |
Thanks for all these very quick thoughts and feedback! Lots of good actionables here, that I agree should be discussed at ROSCon
This seems easily fixable by providing a Melodic & future Noetic release in the ROS 1 build farm, right? Just a matter of someone making the release... |
Making the releases is relatively easy, knowing what exactly to release is the hard part. Even just jumping it into a ROS package will run into issues of colliding symbols and breaking existing implementations that rely on the system packages. There's definitely space to accelerate the rate of development. But we do need to develop the migration plan with versioning etc that can be implemented into the specification to enable the migration path so that maintenance updates can be pushed. There's significant value in being upstream in Ubuntu, but it does slow down the release cadence possiblities and require significant planning ahead for major changes. I'm quite hesitant to support custom extensions for various elements. That is a way that you can get to the point that they all talk URDF, but nothing is fully compatible because they all use different custom extensions. Getting the major stakeholder together at ROSCon sounds like a good idea. |
Was this discussed at ROSCon ? I couldn't attend. |
Yes, we had a meeting about it at ROSCon. We're going to send out some additional details this week, but the short of it is that we are going to attempt not to fork URDF, and see if we can make a bit more progress with it. There is also some talk of switching to a different file format, but we still have to finish discussing that and have a follow-up meeting. |
@clalancette: what's the current status on adding the version nr to the format? |
It's been done in ros/urdf_parser_py#52 and ros/urdfdom#133. I've also released a new version of urdfdom_py into Melodic. I think what is left to do is:
|
Nice 👍 Seems I need to Watch even more repositories to not miss this sort of thing ;) Should there be a |
@clalancette There is any plan of propagating this change in the fork of urdfdom used in ROS2, i.e. https://github.com/ros2/urdfdom ? The fact that ROS2 install its own urdfdom that shadows the one used by ROS1/Gazebo is already quite error prone (see robotology-legacy/gym-ignition#118 (comment)), and if the two versions are not in sync that will get even worse. |
Ug, I didn't even realize that. OK, I'll update the list above, we definitely need to get the change into the ROS2 repository as well. |
Regarding this point:
I had the same issue with SDFormat. As part of TRI-sponsored work, we (@azeey, @scpeters, and I) considered REP / PEP-style proposals, but are just keeping it simple for now: To clarify my position on URDF (at the possibly of being cast as a villain :P): Otherwise, if both formats continue to be co-developed, we will continue to have potentially duplicate effort between the two. (Maybe that's good? Maybe it's not?) I know that this is a completely non-trivial undertaking that's been discussed for years, and nowhere near possible in the near future, but just want to state my position. |
One proposal is that, at least for the time being, we somehow make URDF and SDFormat use the same proposal processes, possibly even the same technology (e.g. same XML parser, similar conventions for I know that this is an Ignition vs. ROS thing, but it would be lovely to see less forking between those technologies when possible? |
Related discussion on ROS Discourse: Why are ROS and Ignition/Gazebo diverging (or not converging)?. You're not the only community member wondering about the apparent divergence. |
Hehe, yeah. I had done a briefly survey on some modeling formats, and came across this (of many) discussions: I'd be happy to take this (kinda crappily written) survey doc and post it on a Gist if it's at all useful. (I'm sure this is the nth survey out of n^2.) |
We also discussed it at the last ROSCon. While there does seem to be some support for switching to another official format, its not entirely clear which format would be chosen. There are some industrial formats that have some benefits, there is COLLADA, there is SDF. There was some lukewarm support for switching to SDF at ROSCon. I don't have strong opinions on the topic. However, I think there are two things that any proposal for a new format needs to deal with:
|
I have recently been using Ignition along with SDF format and I think with some additions it would be a great improvement over the URDF format. I think if we stuck with URDF we would just be replicating a lot of what is already in SDF format. Also, with using the same format it would provide a seamless transition between ROS and Ignition Simulation. I vote for moving to the SDF format. |
@Levi-Armstrong along the lines of your comment, I proposed making SDFormat files first-class citizens in Out of curiosity, what additions were you thinking of form SDFormat to make it an improvement over URDF? |
If it does not exists and if it possible, a converter from URDF to SDF ? |
@jmirabel: Gazebo already comes with one. |
Thanks for your reply. I think it shouldn't be bound to gazebo. |
Related issue: gazebosim/sdformat#85 . TL;DR: There is now a conversion tool directly in sdformat, but unfortunately it depends on Ruby: https://github.com/osrf/sdformat/blob/sdf9/src/cmd/cmdsdformat.rb.in . |
Does sdformat also handles SRDF ? |
SRDF is MoveIt's "semantic" kinematic chain / collision space addition to URDF, and wholly separate from SDFormat. URDF parsers will also not recognize SRDF tags. The term "URDF" is overloaded to mean both a file format ( The URDF data representation |
@jmirabel In addition to Ian's answer, SDFormat 1.7 (libsdformat 9.0.0) also better supports custom tags (which would be ideal for application- or library-specific uses): For example, some form of SRDF's information could be parsed from an
@traversaro I've filed gazebosim/sdformat#274. Would love to hear more (as we on the Drake side have felt this as well), but in a separate thread. |
The reason, I prefer it over the current URDF format is that it appears to be designed around optimizing the physics engine. Most of the collision checking libraries I am using are wrappers around physics engines with the exception of FCL, but they still apply. The model concept prevents duplicate collision geometries being create at the physic engine which we could take an advantage of in collision checking libraries. It also has a static flag for objects. This allows the physics engine to use an optimized data structure because it is not moving to improve performance which collision checking would also take advantage of. |
Related discussion on "forking" URDF: tesseract-robotics/tesseract#184 (comment) . |
There are many desired changes to the URDF spec that are needed for projects like MoveIt and Tesseract that have not been merged into this repo. This has been an issue for many years - the spec is basically unchanged while the SDF format goes through many many iterations. This is hurting the ROS project. A good discussion to get a sense of this issue is here
Based on today's MoveIt Maintainer meeting, we are considering forking this URDF project so that new features can be added. This is not an ideal outcome because we don't want to fracture the ROS community and have code duplication. As I understand it, ROS-Industrial already has forked it.
In order to prevent a fork, we believe URDFDom needs to be released within ROS, not Ubuntu Universe, so that changes can be released more frequently. We also believe we need a more aggressive policy in breaking things, especially between ROS versions, so forward progress can be made.
I may have missed some key points here as I don't follow every detail of the URDF project, but I've been following this project loosely for years and am trying to sum up my observations and the feedback I've heard from others.
How can we get the long list of PRs/improvements in this repo merged without the fear of breaking something?
@scpeters @jmirabel @rhaschke @tfoote @Levi-Armstrong
The text was updated successfully, but these errors were encountered: