tesseract-robotics / tesseract

Motion Planning Environment
http://tesseract-docs.rtfd.io
Other
271 stars 89 forks source link

Erroneous SRDF Version warning #378

Closed mpowelson closed 4 years ago

mpowelson commented 4 years ago

Currently we have a warning in SRDF_model.cpp "No version number was provided so latest parser will be used.". First of all, it doesn't look like we are setting the version if it isn't given. Second, it looks like the version is never used? None of the functions it is passed into actually use it.

This has the same sort of problem as urdf versions. It is only useful if we have documentation about what the versions mean. I was trying to figure out how to get the warning to go away without changing how things are currently working. It looks like I can just fill it out as anything (but what? 1.0? 0.1?), but I think the better approach might be to get rid of the warning until we have something that actually uses that field.

Levi-Armstrong commented 4 years ago

Currently the Tesseract Setup Wizard should set the version if used. It is currently integrated but not used because the srdf format has not changed, but it should be checking if the version is provided if it is supported. I will take a look and see if it is currently being checked. The current format is #.#.# or #.#. I will also update the warning message to include more information.

mpowelson commented 4 years ago

If we should have a version that's fine, but what version would I use? What version is a moveit setup assistant generated one? I don't think any of the examples have the tag.

Levi-Armstrong commented 4 years ago

The current version is 1.0.0.

mpowelson commented 4 years ago

So while it doesn't currently hurt anything, to eliminate the warning add version to your robot tag like below if using an srdf from the moveit setup assistant

<robot name="my_robot" version="1.0.0">
Levi-Armstrong commented 4 years ago

I have also updated the warning message to print what is needed to suppress the warning.

mpowelson commented 4 years ago

It looks like this has been addressed, so I am going to close it.