-
Notifications
You must be signed in to change notification settings - Fork 47
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
Make sure to add the version when creating a new URDF. #62
Conversation
It turns out that when I added the version stuff, I completely forgot about creating a new URDF. This PR fixes it so that we can successfully round-trip through the URDF parser, and adds tests to ensure the same. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@sloretz Friendly ping on this one; mind reviewing? |
Thanks, appreciate the review. Merging. |
Hi @clalancette, let us know if there is anything we can do to help to get the fix shipped also to the |
It turns out that when I added the version stuff, I completely forgot about creating a new URDF. This PR fixes it so that we can successfully round-trip through the URDF parser, and adds tests to ensure the same. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
It totally fell off of my radar, thanks for the ping. See #66 for the ROS 1 fix. |
It turns out that when I added the version stuff, I completely forgot about creating a new URDF. This PR fixes it so that we can successfully round-trip through the URDF parser, and adds tests to ensure the same. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
It turns out that when I added the version stuff, I completely
forgot about creating a new URDF. This PR fixes it so that
we can successfully round-trip through the URDF parser, and
adds tests to ensure the same.
Signed-off-by: Chris Lalancette clalancette@openrobotics.org
Note that once this is reviewed, approved, and merged, I'll do a similar fix for the ROS 1 code. This should fix #59