Remove visibility macros

[christophfroehlich]

As a consequence of ros-controls/ros2_control#1627 there is an include for HARWARE_INTERFACE_PUBLIC missing, causing the build of example_7 to fail.

As we've discussed and decided in ros-controls/ros2_controllers#1053, we wanted to remove all the visibility macros and use set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON) instead. I took the chance to do so here.

Fixes #538 and also resolves #73

• Docker and binary(main) builds fails because of the latest release of ros-rolling-gz-ros2-control not being synced.
• Semi-binary build needs Add now missing includes ros2_controllers#1226

[christophfroehlich]

👀 @traversaro

[henrygerardmoore]

Fixes build for me locally, but shouldn't lines such as

# Causes the visibility macros to use dllexport rather than dllimport,
# which is appropriate when building the dll but not consuming it.
target_compile_definitions(${PROJECT_NAME} PRIVATE "ROS2_CONTROL_DEMO_EXAMPLE_1_BUILDING_DLL")

be removed? This is lines 39-41 of example 1's CMakeLists but every example has similar lines

[christophfroehlich]

Ah, thanks for the review. I missed that

[saikishor]

Seems fine as per my knowledge.

Thank you!

[christophfroehlich]

@traversaro do you know if we need target_compile_definitions or similar for windows builds?

[traversaro]

@traversaro do you know if we need target_compile_definitions or similar for windows builds?

If you do not have any visibility macro, it is ok not to add any target_compile_definitions. As a general comment (in case someone reads this) also if you have the visibility macro it is not ideal to have code like:

target_compile_definitions(${PROJECT_NAME} PRIVATE "ROS2_CONTROL_DEMO_EXAMPLE_1_BUILDING_DLL")

as this will define the macro even if the target ${PROJECT_NAME} is a static library. The correct property to set is the DEFINE_SYMBOL (https://cmake.org/cmake/help/latest/prop_tgt/DEFINE_SYMBOL.html) of the target, or even better just use the default DEFINE_SYMBOL value that is <targetName>_EXPORTS .

[traversaro]

Changes seems ok to me, thanks! As raised in the PR review, you can also remove the target_compile_definitions(${PROJECT_NAME} PRIVATE "ROS2_CONTROL_DEMO_EXAMPLE_7_BUILDING_DLL") code, but if there are no visibility headers that code is basically ignored.

[christophfroehlich]

Changes seems ok to me, thanks! As raised in the PR review, you can also remove the target_compile_definitions(${PROJECT_NAME} PRIVATE "ROS2_CONTROL_DEMO_EXAMPLE_7_BUILDING_DLL") code, but if there are no visibility headers that code is basically ignored.

Thanks a lot for the review, I'll remove that dead code then ;)