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

gazebo_plugins: export plugin path in package.xml #923

Merged
merged 1 commit into from
Dec 26, 2019
Merged

gazebo_plugins: export plugin path in package.xml #923

merged 1 commit into from
Dec 26, 2019

Conversation

lopsided98
Copy link
Contributor

gazebo_plugins does not have a gazebo_ros export in its package.xml, so it is not picked up by gazebo_ros_paths_plugin. This PR adds the necessary export tag and adds gazebo_ros as a dependency so that pluginlib processes this tag correctly.

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. I'm wondering how this hasn't come up before. What plugin were you trying to use when you ran into this?

gazebo_plugins/package.xml Show resolved Hide resolved
@chapulina chapulina merged commit 6f48667 into ros-simulation:melodic-devel Dec 26, 2019
@lopsided98 lopsided98 deleted the export-plugin-path branch December 26, 2019 21:01
nim65s pushed a commit to nim65s/robotpkg that referenced this pull request Mar 9, 2020
patch-ab: adapt
patch-ad: python 3 compatibility

Changelog for package gazebo_ros
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

2.8.6 (2019-12-26)
------------------
* ROS API: remove unhelpful error in GetWorldProperties call (`%747 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/747>`_)
* Create reconfigure thread only if network enabled (`%919 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/919>`_)
  This thread was blocked in client.waitForExistance because the services
  it depends on are only created if `enable_ros_network` is true. This in
  turn blocked gazebo from being shut down.
  Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* spawn_model: Fix urlparse imports for Python 3
* spawn_model: Ensure that "model_xml" is a string, required for Python 3
* catkin_find gazebo plugin from bin folder. (`%993 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/993>`_)
* [Windows][melodic-devel] more Windows build break fix (`%975 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/975>`_)
  * Fix CMake install error for Windows build.
  * conditionally include <sys/time.h>
* provide Windows implemenation for setenv. (`%879 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/879>`_)
* implement basic gazebo scripts to support launch file on Windows build. (`%880 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/880>`_)
* Contributors: Kartik Mohta, Kevin Allen, Sean Yen, Shane Loretz

2.8.5 (2019-06-04)
------------------
* Add output arg to launch files, plus some small fixes (melodic) (`%907 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/907>`_)
  * Add output arg to empty_world
  * add output arg to elevator_world
  * add output arg to range_world
  * don't set use_sim_time in range_world
  Instead parse it to empty world, where it will be set.
  * add xml prolog to all launch files
  * Remove unnecessary arg in range_world.launch
* use C++11 std sleep instead of usleep. (`%877 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/877>`_)
* fix issue `%198 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/198>`_ (`%825 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/825>`_)
* Lower minimum cmake version (`%817 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/817>`_)
* Contributors: Matthijs van der Burgh, Paul Bovbel, Sean Yen [MSFT], Steven Peters

Changelog for package gazebo_ros_control
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

2.8.6 (2019-12-26)
------------------
* restrict Windows header namespace. (`%1023 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/1023>`_)
* [Windows][melodic-devel] more Windows build break fix (`%975 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/975>`_)
  * Fix CMake install error for Windows build.
  * conditionally include <sys/time.h>
* Contributors: Sean Yen

2.8.5 (2019-06-04)
------------------
* use C++11 std sleep instead of usleep. (`%877 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/877>`_)
* Lower minimum cmake version (`%817 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/817>`_)
* Contributors: Paul Bovbel, Sean Yen [MSFT]

Changelog for package gazebo_plugins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

2.8.6 (2019-12-26)
------------------
* gazebo_plugins: export plugin path in package.xml (`%923 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/923>`_)
* Fix destructor of gazebo_ros_diff_drive.cpp (`%1021 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/1021>`_)
  Fix issue referenced in `%123 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/123>`_ where the destructor of ROS DiffDrive plugin causes gzserver to crash on model deletion.
* [Windows][melodic-devel] more Windows build break fix (`%975 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/975>`_)
  * Fix CMake install error for Windows build.
  * conditionally include <sys/time.h>
* Use ignition::math::Rand utility for portability. (`%878 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/878>`_)
* Contributors: Ben Wolsieffer, RemiRigal, Sean Yen

2.8.5 (2019-06-04)
------------------
* use C++11 std sleep instead of usleep. (`%877 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/877>`_)
* Lower minimum cmake version (`%817 <https://github.com/ros-simulation/gazebo_ros_pkgs/issues/817>`_)
* Contributors: Paul Bovbel, Sean Yen [MSFT]
@jonbinney
Copy link
Contributor

Is this change actually needed? That exported path will only be correct in install space anyway, right? In devel space ${prefix} will (i think) resolve to the package's location in source space, and the path won't lead to the devel/lib folder.

I almost added something similar to a package because it suddenly stopped finding plugins, but the cause turned out to be that cmake 3.0.2 didn't prepend lib to library filenames (cmake 2.8.6 did).

@chapulina
Copy link
Contributor

@jonbinney , you may be interested in this conversation: https://bitbucket.org/osrf/gazebo_tutorials/pull-requests/552/

I recently started recommending that people use env-hooks instead of exporting paths through package.xml.

@jonbinney
Copy link
Contributor

@chapulina env hooks do sound better than exporting from package.xml. But in my experience you don't need either for the plugin path (at least in devel space). I think this is because the setup.sh that catkin generates adds devel/lib to your LD_LIBRARY_PATH. I think this is true for install space as well, since /opt/ros/melodic/setup.sh adds /opt/ros/melodic/lib to LD_LIBRARY_PATH.

$ . /opt/ros/melodic/setup.sh 

$ env|grep LD_LIBRARY_PATH     
LD_LIBRARY_PATH=/opt/ros/melodic/lib

$ ls /opt/ros/melodic/lib|grep "libgazebo.*\.so"
libgazebo_ros_api_plugin.so
libgazebo_ros_block_laser.so
libgazebo_ros_bumper.so
libgazebo_ros_camera.so
libgazebo_ros_camera_utils.so
libgazebo_ros_control.so
libgazebo_ros_depth_camera.so
libgazebo_ros_diff_drive.so
libgazebo_ros_elevator.so

And in gazebo it falls back to just calling dlOpen on the library name if it doesn't find anything by that name in the plugin path: https://bitbucket.org/osrf/gazebo/src/84ae5943bf02843b91a10f82ba084db0d753aee7/gazebo/common/Plugin.hh#lines-171

So i don't think this PR needed to add plugin_path="${prefix}/../../lib" in the package.xml (or do anything to env hooks). gazebo_media_path is a different story I assume.

@chapulina
Copy link
Contributor

Yeah that makes sense, Gazebo will look into LD_LIBRARY_PATH too.

the setup.sh that catkin generates adds devel/lib to your LD_LIBRARY_PATH

Do you know if that's also true for catkin_tools and colcon? I'm pretty sure we need either the env-hooks or the exported paths for ROS 2. I haven't used ROS 1 much lately though.

@jonbinney
Copy link
Contributor

I just did a test with catkin tools, and it does add devel/lib to LD_LIBRARY_PATH. I tried out colcon both with and without --symlink-install, and it adds install/<package_name>/lib to LD_LIBRARY_PATH for every package in the workspace.

I mean, they have to, right? Otherwise ROS nodes wouldn't be able to find the libraries they want to link against. Unless they used rpath or something to get around LD_LIBRARY_PATH completely.

@jonbinney
Copy link
Contributor

Oh! I just realized who @chapulina is :-P

@chapulina
Copy link
Contributor

I just realized who @chapulina is :-P

Hi there 🙋

Thanks for the quick tests. I'd have to investigate a bit more to find out why the env hooks / export paths were necessary in the first place. I'm 90% sure Gazebo won't find plugins without them.

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.

None yet

3 participants