tu-darmstadt-ros-pkg / hector_gazebo

hector_gazebo provides packages related to the simulation of robots using gazebo (gazebo plugins, world files etc.).
http://www.ros.org/wiki/hector_gazebo
184 stars 157 forks source link

Support Indigo + gazebo7 #30

Closed nkoenig closed 8 years ago

nkoenig commented 8 years ago

Adds ifdefs around gazebo7 API changes.

This should make it possible to use ros-indigo-gazebo7-ros-pkgs.

meyerj commented 8 years ago

This PR is in conflict with an earlier PR #24 by @nicolaerosia. We definitely want to have the ifdefs to keep the plugins compatible with as many Gazebo versions as possible. But what is the difference between Joint::SetEffortLimit(...) from #24 and Joint::SetParam("fmax", ...) from this PR? The former seems to follow the documented API.

What do you think about the merge resolution in c52145ee61d6660e4b66dec779e5f1ef3657325d for jade-devel?

mikepurvis commented 8 years ago

@nkoenig @wjwwood @tfoote What would you guys think about dropping the direct dependency on gazebo from packages like this one, in favour of a dependency on gazebo_ros?

If gazebo_ros were switched to package format 2, it could <build_export_depend>libgazebo7-dev</build_export_depend>, and thus we'd make it easier for people to move between Gazebo versions and not have hard-coded gazeboN and libgazeboN-dev dependencies everywhere.

tfoote commented 8 years ago

That sounds reasonable to me.

meyerj commented 8 years ago

Hmm, that would probably work. The only drawback that I see is that gazebo_ros already brings in lots of dependencies that are actually not necessarily required:

$ rospack depends1 gazebo_ros
gazebo_msgs
roslib
roscpp
tf
std_srvs
rosgraph_msgs
dynamic_reconfigure
geometry_msgs
message_generation
std_msgs

Most users have all of those installed, but I would prefer a more minimal solution. What if gazebo_ros_pkgs would contain a package gazebo_default which only depends on the default gazebo version of the respective ROS distro, without any functionality like gazebo_ros? All other packages depending on Gazebo (including gazebo_ros) could then depend on gazebo_default.

mikepurvis commented 8 years ago

That sounds good, but I wonder about the possibility of just calling that package gazebo?

@tfoote Given that ROS Indigo's Gazebo is 2, how bad would it be to shadow the gazebo dependency name for this purpose?

(My interest in all this is having a sane story for supporting Gazebo 7 on Clearpath's internal builds, which doesn't require a lot of patching of third-party repositories.)

tfoote commented 8 years ago

@mikepurvis we cannot mask/shadow a dependency. We've run into big problems any time we've tried to switch and have always had to make sure any transition between ros package and rosdep uses different keys.

For your use case if the code is written to be compatible either way you could update the rosdep key on your buildfarm using customized rosdep sources.

mikepurvis commented 8 years ago

We've tried to do it that way, and we may yet pursue that path, but it's awkward when trying to build one distribution against Gazebo 2 and another against Gazebo 7.

Regardless, I think there's merit to the proposal of a gazebo_default dependency package.