Proposer: John Hsu
Present at review:
- List reviewers
Question / concerns / comments
Adolfo Rodríguez Tsouroukdissian
The <sensor> tag has a very general name, but currently covers visual sensors only. If future extensions might include non-visual sensing elements like force-torque sensors, then the name is appropriate. If the scope will remain limited to visual sensors, a less general name should be considered.
John: The idea is that if we support <sensor> on the same level as <link> and <joint> elements. If we find sensor tags useful as an extension to URDF, to allow for different types of sensors inside of <sensor> block down the road, i.e.
<sensor> <ray> ... </ray> </sensor> <sensor> <imu> ... </imu> </sensor>.
- The scope of the URDF spec seems to be growing in directions where other specs, like Collada are already proficient in, like scenes and model states. Would it be possible to mention the rationale that supports extending URDF as opposed to embracing an existing spec?, or put otherwise, what would be difficult to achieve using "X" that will be much easier with the current proposal? (eg. robotics-specific stuff, like maybe including efforts in animations/model states?).
- John: I agree we should keep it simple, based on current proposal, existing spec will not change, we are simply extending (reluctantly) features that have been requested repeatedly by URDF users since we created the format. Current URDF users should not see any differences unless they want to use additional features. IMHO, COLLADA strives at being proficient in almost all areas of graphics + animation + physics + user extensions from the beginning, but URDF is designed to support only the most basic set of modeling descriptions needed by robotics. I do agree however, scenes belong outside of the URDF realm, hence I am removing the discussion for it outside of this API review. But on the other hand, I think model_state belongs as another extension similar to sensors inside of URDF, as model_state is very closely tied to URDF models. As Shaun mentioned, this will allow for things like setting the "home" position for URDF models.
I agree with adolfo. I thought the sensor tag was for something more general. I would propose a <ray_sensor>
- John: please see my response to adolfo.
I would like to see the <robot> root tag supported beyond feurte to groovy. We have many URDF's to update and it sounds like they will all break once groovy is released.
- John: will keep support for both tags at least through groovy, and for H-turtle if more people request it.
- I'm not sure this is addressed here, but I would like to see something that supports are "home" position for the urdf. Currently, the robot comes up in the "zero" position when not connected to physical hardware. I know the joint_state_publisher ultimately determines this, but there isn't a way to override the home position in that node.
John: <model_state> should support this.
- I don't really understand the need/use for scenes so I cannot speak to them.
- John: Scenes are removed from discussion.
It looks to me like switching the root tag from <robot> to <model> is intended to be more compatible with SDF. Is this true? If true, it does make some sense. However I don't know that it is any easier for that to change instead of SDF to change instead.
John: The change was unrelated to SDF, but motivated by considering we might want to model more than "robots" in our world. It has the nice side-effect of matching SDF
What is the value of adding scene info to URDF? Scene info is represented in SDF (see sdf_format), but outside of the URDF-like <model> tag.
In sdf_format I see a frustrating paragraph at the bottom of the page about how the format of a "model" inside of SDF matches that of an URDF "robot" except for a difference in semantics of coordinate frames. Basically the result seems to be that if you stick your robot's correct URDF into an SDF, it will not work right, and vice-versa, if you pull a working robot out of SDF and call it an URDF, it will not work right. It wouldn't be so bad if they had different syntaxes or tags, so you would get a parse error. This way there wouldn't even be any error messages, you would just see the robot with its limbs arranged incorrectly.
- My point here is that *that* seems like the kind of thing to be working on, rather than stirring in apparently redundant info and making the formats less compatible.
John: you do bring up a good point about potentially keeping <robot> for distinguishing URDF from SDF. I'll have to talk more with Nate to think about merging the two.
- Summary: The URDF is already a good description of robot structures. Functionality that better describes a robot's structure, like sensor information, is useful, but other sorts of information should live in its own separate location. Proposals for robot state and scene structure are very useful, but should remain separate from the URDF specification.
- Renaming from "robot" to "model" breaks existing code, and the benefits seem minimal.
- Cameras currently publish different images with different formats, but the new camera tag specifies a format for the camera. This proposal doesn't support things that the camera drivers already do (such as changing formats at runtime). Same for image width/height; the prosilica changes its image sizes on the fly.
- Real cameras don't have near and far clipping planes. These options are only useful in simulation. Do they belong in a generic robot description?
- update_rate is also determined at runtime by the driver for all sorts of sensors. Cameras, hokuyos, and kinects all change update rates on the fly. This field is also only useful for simulation, and doesn't belong in a generic description of the robot.
- model_state belongs somewhere else. It's not a description of a robot, it describes the state of a robot. The current state is extremely useful, but it should not pollute the URDF--the information belongs elsewhere.
- The scene and scene_state look useful, but they belong in their own places as well. No reason to add them to the urdf.
As a general comment, I think we should be very careful when adding to or modifying the URDF. The URDF tries to do only one thing, and do that very well. There are other specifications out there (e.g. Collada) that cover almost everything you can imagine.
- Sensors have the potential to become a good addition to the urdf library. Things to keep in mind:
- We should carefully design a description of sensors that is not just tailored towards simulation. For this I +1 Stu's comments above.
- Keep sensors as a separate xml description and a separate parser from the URDF, so we don't need to touch the URDF description/parser.
- Describe multiple use cases for each proposed specification.
- Is there an overlap between the Camera information and the data in a camera info message that is produced by every camera in ROS? How do we deal with a camera driver that can switch resolution, frame rate, etc on the fly? This currently gets reflected dynamically in the camera info message. How would the Camera urdf deal with this?
- Scene and scene_state seem useful, but outside the URDF scope. As mentioned by other reviewers, there might be an overlap with the SRDF. As a motivation for new descriptions, we should always add usecases -- these are still missing here.
- Model_state is definitely useful, but again it might be better in its own place, independent of the URDF. Here too we need use cases.
- Renaming 'robot' to 'model' offers a marginal benefit in clarity. I'm neutral to this change, as long as we provide both forward and backward compatibility. This means nobody will have to change any existing urdf file, and any new urdf file also works with old versions of the parser.
Full mail for the mailing-list available here: http://lists.ros.org/lurker/message/20120613.030231.df16f027.en.html
I am not against extending or changing URDF in itself, I think the PR2 specific extensions should be cleared and replaced by a generic mechanism, for instance. However, I am not sure we're going in the right direction here...
I suggest the following changes:
- keep URDF as simple as possible
- go toward making sure that URDF is a subset of SDF
- do not redefine what already exists in SDF, use it.
- write a spec file or a REP for URDF and SDF so that it can stay a coherent entity (i really don't want to maintain one SDF and one URDF for the same robot!)
- be nice with the other parsers: if you change the format, you break our code. At least, it would be nice to have a DTD, an XML scheme or a version tag somewhere in the file so that we can properly refuse too recent or too old URDF versions.
Of course, this makes only sense if there is a spec, first.
Here is my answer to the API proposal:
I think that the scene state should not be encoded in the URDF file, because it will prevent us from keeping one immutable reference file containing the static information regarding a particular robot. By definition if we start putting the scene inside, we will end up with something like "PR2 alone", "PR2 with object A", "PR2 with object B but not A", etc. It will make the number of files explode. Instead, I would suggest to make it either a different format or a subset. It makes much more sense IMHO to have a small file listing the robots and obstacles to be loaded by refering to their url for instance (i.e. "package://pr2_description/pr2.urdf", "package://pr2_env_models/table.dae"...). It is simple and retrocompatible.
For the same reason, I think that the sensor information (field of view for camera or whatever is needed) should not be in the URDF file. We could probably take advantage of the sensor node to avoid multiplying anchor joint in the robot kinematics structure which then pollute the control algorithms. However, of course, we will fail to model all the possible sensors one can put on a robot, so either we make our parser complex with plug-ins or similar mechanisms to handle extensions which model custom sensors or we do the following:
- the sensor node is here to describe the location of the sensor on the robot and its geometry
- it contains an attribute describing the sensor type (a string) and a pointer to a yaml file (the meta-information). The later is sensor dependent, the parameter file for the most common sensors can be normalized easily and progressively so we don't have to think about everything now. As it is not the responsibility of the URDF parser to interpret this data, it keeps it simple and "extension free".
This last change goes toward the SDF compatibility goal but I think that the SDF format is really weak on this particular point so it may require an update IMHO. It will require an update anyway when you will add support for other sensors...
One last point: what about srdf? the model_state element also duplicates efforts here...
- ROS/Gazebo file formats needs to be hardened:
- a spec should be written then a validation scheme should be provided (DTD, XML Scheme...)
- specs should be versioned so that we can be retrocompatible "forever"
- Sensor description is a real need, but is outside URDF scope. Sensor position on the robot is URDF responsibility.
- Scene description is really needed, but is outside the scope of URDF.
I think we need a way to define worlds so that simulators and planning software for instance can use them, without being related to Gazebo itself. URDF should be cleaned of its extensions and sensor description should be handled from the outside of URDF/SDF.
Thanks everyone for your inputs. I think it appears to be the general consensus that we should do two things here:
- Exclude scene, scene_state and model_state extensions from the urdf package (and current review discussion). While these functionalities may be useful, but they are outside of scope of urdf.
- We will focus the current discussion on adding support for sensors to the urdf.
A more detailed response:
<scene>, <scene_state>, <model_state>
Move these discussions somewhere else (and be sure to include SRDF in the new thread).
I see the importance of a home position as suggested by Shaun. To me this will be supported when we agree on a <model_state>, outside of URDF.
<robot> - <model>
The switch from <robot> to <model> is purely cosmetic. I want to make clear that this suggestion was not motivated by SDF, but was considered because the word model is more generic than robot, as it better describes potential simple objects in a scene (such as tables, chairs). I propose supporting both <robot> and <model> at least through H-turtle or possibly indefinitely for the foreseeable future.
- Regarding currently proposed visual sensors (camera/ray):
I agree that urdf_sensor data structure and parser should be completely decoupled from existing URDF data structure and parser, so the sensor headers and parser will be kept under a separate directory inside of urdfdom if we decide to add them.
- Near and far clipping distances do appear to be simulation specific to me, they will be removed unless strong use cases are presented.
- Open questions:
- Please contribute use cases for sensors.
- Proposals on how to deal with the possibility of adding and supporting new sensors (visual and non-visual) in the future. Hopefully the use cases will also help us get a feel of how we might need to expand sensor vocabularies if we do decide to add sensor support to URDF.
- Proposals on how to deal with dynamically changing parameters. Two examples raised by Stu were update rates and image size.
Todos as suggested by Thomas:
- Write a spec (REP).
- Adopt a validation scheme (DTD, XML Scheme...). Please suggest / comment on which validation scheme you prefer.
Thomas: see urdf.xsd, this is a XML Schema validating the current URDF file format (w/o the extensions proposed here).
Add a version attribute under <robot>and give the current version of URDF a version number for backwards compatibility.
- Updates to urdf model:
Renaming of urdf root node from <robot> to <model> in documentation, but support both in cturtle through fuerte.
Renaming urdf_interface to urdf_model inside urdfdom. ros package urdf_interface will be tic-toc'd to urdf_model as well.
Review addition of sensor description.
Review addition of model_state.
- Extending urdf to scenes:
Package status change mark change manifest)
Action items that need to be taken.
Major issues that need to be resolved