ros / urdfdom

URDF parser
http://ros.org/wiki/urdf
Other
96 stars 132 forks source link

Move transmission parsing into urdfdom? #3

Open davetcoleman opened 11 years ago

davetcoleman commented 11 years ago

As suggested by @mikeferguson, it might be more useful to move the transmission parsing code into urdfdom so that the xml standard for urdf is centralized.

isucan commented 11 years ago

+1

mikeferguson commented 11 years ago

+1

isucan commented 11 years ago

I looked at the XML for the transmission stuff. Has this changed at all from the format we had a while back? There are two options as I see it:

  1. Put this XML in the urdf and use the parser Dave pointed to for loading it. The regular urdf parser ignores unknown tags. This does not need to modify the URDF format or parser, but allows keeping things in the same place.
  2. Modify the URDF format (and consequently, the parser) to parse transmissions. If you think we are at a stable XML representation that is unlikely to change in the next year, for instance, we could probably add it to the URDF. But I'd like to offer the community the option to state their opinion on this. This means we will add a wiki page describing the proposed changes, advertise on ros-users and allow for people to comment on the proposed changes. Depending on the results of that, we should have a meeting and discuss changes to be made (if any).

I expect (2) will take a while (at least a few weeks), so my suggestion would be to do (1) for now. If the XML format for transmissions is stable, we can start on (2) as well.

davetcoleman commented 11 years ago

To clarify, option 1 is basically do nothing and leave things the way they are. We are already using tags for using controllers in Gazebo through this method, see tutorial

Just today there was some discussion on the controls mailing list about modifying the transmission format to allow multiple hardware interfaces to be supported. So I guess we aren't completely stable yet. However, in the current parser I made it load the unrecognized elements into a tinyxml variable so that its easy for custom tags to be added for more complex transmissions.

On this note, I was looking into the integration of my transmission parser into urdfdom and urdfdom_headers and I noticed in urdfdom_headers that there is currently no tinyxml dependency. Would it be ok to add this so that the data structures can store the custom tags as I just mentioned?

Overall, I think we should be planning to do option (2) as soon as the discussions on transmission format settles down.

adolfo-rt commented 11 years ago

+1, but not just yet. Urdfdom is a stable package (version 1.7.1), and I'd like to see the transmission parsing code get used a bit more before commiting to an API and having to tick-tock any changes.

I'm currently working with this code, and in fact I might propose (minor) changes that affect both the public API and the XML representation. These are changes that I had not foreseen in previous design discussions, but only became apparent when getting down and dirty with the implementation.

jbohren commented 10 years ago

Meanwhile, if you try to parse an urdf with urdf_parser_py it throws an error if your <transmission><joint>.. tags don't have a type attribute. #36