Skip to content
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

urdfdom compatibility #1064

Merged

Conversation

rhaschke
Copy link
Contributor

Reapplied #1044, using urdf::SharedPtr instead of explicit boost::shared_ptr.
Fixed compatibility issues with older urdfdom 0.3, by relying on ros/robot_model#160.

jspricke and others added 2 commits October 27, 2016 18:27
)

urdfdom_headers uses C++ std::shared_ptr. As it exports it as custom
*SharedPtr type, we can use the to sty compatible.

This also adds a proper dependency for urdfdom-headers
... relying on the compatibility layer of urdf package
@wjwwood
Copy link
Member

wjwwood commented Oct 27, 2016

Wow thanks @rhaschke, I was expecting to do this today. I appreciate you opening the pr!

The code changes lgtm, I need to do a build on Wily real quick to test it.

@rhaschke
Copy link
Contributor Author

You are welcome. I was working on that anyway. Please also have a look at ros/urdfdom_headers#33. The most annoying incompatibility was the new urdf_world/types.h, which doesn't make sense to me at all.

@wjwwood
Copy link
Member

wjwwood commented Oct 27, 2016

Is ros/urdfdom_headers#33 necessary for these pr's to get merged? I would think not (since we probably cannot get those changes released into Debian/Ubuntu at this point).

@rhaschke
Copy link
Contributor Author

No, it's not. There is a workaround in the compatiblity header to also define the ModelInterface types correctly without urdf_world/types.h.

@@ -39,8 +39,7 @@
#include <OgreQuaternion.h>
#include <OgreAny.h>

#include <urdf_model/types.h>
#include <urdf_world/types.h>
#include <urdf/model.h> // can be replaced later by urdf_model/types.h
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be changed to include urdf_model/types.h as soon as ros/urdfdom_headers#33 is merged.

@wjwwood
Copy link
Member

wjwwood commented Oct 27, 2016

I tested this manually on Wily with ros/robot_model#160. Thanks again @rhaschke!

@wjwwood wjwwood merged commit 378b7dd into ros-visualization:kinetic-devel Oct 27, 2016
@rhaschke rhaschke deleted the urdfdom-compatibility branch October 27, 2016 21:52
@languitar
Copy link

Is it possible to also merge this into indigo-devel?

@wjwwood
Copy link
Member

wjwwood commented Apr 7, 2017

@languitar maybe, why do you need it though?

@languitar
Copy link

At least on archlinux I am unable to compile indigo desktop without this fix.

@wjwwood
Copy link
Member

wjwwood commented Apr 8, 2017

That's probably because archlinux has a newer version of urdfdom/urdfdom_headers that we use on Ubuntu Trusty for ROS Indigo.

I'll consider backporting it next time I do a release, but honestly you should update to Kinetic if you're using the latest dependency versions on arch.

@languitar
Copy link

Sure, I'd like to if everything I wanted to try worked well with that version.

@wjwwood
Copy link
Member

wjwwood commented Apr 10, 2017

Sure, I'd like to if everything I wanted to try worked well with that version.

I assume you mean, something that you're using doesn't support Kinetic?

For me there's three things you could do:

  • Use Indigo on Ubuntu Trusty
    • Nobody wants to ask you to do that, but it's the least effort
  • Push on the thing you're using to provide support for Kinetic
    • This is more work, but the best solution
  • Ask me to backport patches to a stable release (Indigo) in rviz
    • I might be willing to help you out, but in the context of the other options, it seems the least appropriate to me

I'll see what I can do, but please put pressure on the upstream thing to support Kinetic too.

wjwwood pushed a commit that referenced this pull request Jun 5, 2017
* Use urdf::*ShredPtr instead of boost::shared_ptr (#1044)

urdfdom_headers uses C++ std::shared_ptr. As it exports it as custom
*SharedPtr type, we can use the to sty compatible.

This also adds a proper dependency for urdfdom-headers

* adaptions to build against both urdfdom 0.3 and 0.4

... relying on the compatibility layer of urdf package
@wjwwood
Copy link
Member

wjwwood commented Jun 5, 2017

Backported in #1110

wjwwood added a commit that referenced this pull request Jun 5, 2017
* Add fullscreen option. (#1017)

* urdfdom compatibility (#1064)

* Use urdf::*ShredPtr instead of boost::shared_ptr (#1044)

urdfdom_headers uses C++ std::shared_ptr. As it exports it as custom
*SharedPtr type, we can use the to sty compatible.

This also adds a proper dependency for urdfdom-headers

* adaptions to build against both urdfdom 0.3 and 0.4

... relying on the compatibility layer of urdf package

* Update display if empty pointcloud2 is published (#1073)

Do not show last point cloud any more, if published point cloud message does not contain any points

* Correctly scale the render panel on high resolution displays (#1078)

* support multiple material for one link (#1079)

* Fixed duplicate property name for Path colors (#1089)

See issue #1087.

* fix type error in newer versions of urdf (#1098)

* Use unique material names for robot links. (#1102)

* avoid C++11 feature for back port to indigo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants