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

Fix gazebo_ros:plugin_path in package.xml #2

Open
wants to merge 2 commits into
base: melodic-devel
Choose a base branch
from

Conversation

peci1
Copy link

@peci1 peci1 commented Dec 9, 2019

Using ${prefix}/lib is wrong.

According to the rospack implementation, ${prefix} is not context-dependent and get always expanded to the package path. If you're using devel space, this results in path to the package's source, not to the devel directory.

And really, this is from a DEBUG log of gazebo_ros.paths_plugin:

[DEBUG] [1575894134.676531222]: plugin path /opt/ros/cras_subt/src/realsense_gazebo_plugin/lib

The subexpression used in the PR works so that it asks catkin for the absolute path to the built library and takes the directory it resides in. This expression is evaluated at launch time, so we can be sure the library is already built at that time.

v-lopez and others added 2 commits December 3, 2019 17:06
Using `${prefix}/lib` is wrong. 

According to the [rospack implementation](https://github.com/ros/rospack/blob/8c5a699461f32f620204ccfaf93c5e5181f20f14/src/rospack.cpp#L2182), `${prefix}` is not context-dependent and get always expanded to the package path. If you're using devel space, this results in path to the package's source, not to the devel directory.

And really, this is from a DEBUG log of gazebo_ros.paths_plugin:

    [DEBUG] [1575894134.676531222]: plugin path /opt/ros/cras_subt/src/realsense_gazebo_plugin/lib

The subexpression used in the PR works so that it asks catkin for the absolute path to the built library and takes the directory it resides in. This expression is evaluated at launch time, so we can be sure the library is already built at that time.
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.

2 participants