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

Create sdformat_urdf_plugin as SHARED instead of MODULE for macOS compatibility #22

Merged
merged 2 commits into from Nov 11, 2023

Conversation

traversaro
Copy link
Contributor

Technically speaking, when creating a library is only meant to be opened via dlopen (and not linked at link time) MODULE is instead the correct library type. However, this does not work in pluginlib (see ros/pluginlib#200), so as a workaround to get this plugin to work on macOS, we change sdformat_urdf_plugin to be a SHARED library, not a MODULE library. This also is aligned with what is done with the urdf_parser (see https://github.com/ros2/urdf/blob/1d257eb6a3fa34593c0da67f5d16cd0155d7d91b/urdf/CMakeLists.txt#L58). On Windows and Linux, changing from MODULE to SHARED does not change anything, as far as I know.

A similar issue on Gazebo side is gazebosim/gazebo-classic#800 .

@traversaro
Copy link
Contributor Author

traversaro commented Nov 7, 2023

This will avoid that downstream users suggest workarounds as manually creating links, as done in https://github.com/ArduPilot/ardupilot_gz/blame/49706c77b962615e8e305bd893bc3fde0dd71974/README.md#L156 and RoboStack/ros-humble#104, fyi @srmainwaring @Ryanf55

Copy link
Contributor

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

LGTM with one nit

sdformat_urdf/CMakeLists.txt Outdated Show resolved Hide resolved
@sloretz
Copy link
Contributor

sloretz commented Nov 11, 2023

Thanks for the PR!

@sloretz sloretz merged commit 9f88985 into ros:ros2 Nov 11, 2023
6 checks passed
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

2 participants