Improve CMakeLists.txt #4
Conversation
| @@ -1,8 +1,6 @@ | |||
| cmake_minimum_required(VERSION 3.7.2) | |||
| cmake_minimum_required(VERSION 3.10.2) | |||
There was a problem hiding this comment.
The oldest CMake supported by ROS 1 is 3.10.2
| project(point_cloud2_filters) | ||
|
|
||
| add_compile_options(-std=c++17) |
| CATKIN_DEPENDS roscpp pluginlib filters tf2_ros pcl_ros pcl_conversions sensor_msgs | ||
| #DEPENDS PCL | ||
| ) | ||
| catkin_package() |
There was a problem hiding this comment.
It is not expected that any other package will link to the filter provided by this package. In such case, catkin_package() should be empty because all the information within it are just for downstream packages trying to link to this one.
| ) | ||
| add_dependencies(pass_through_filter_point_cloud2 ${catkin_EXPORTED_TARGETS} ${${PROJECT_NAME}_EXPORTED_TARGETS}) | ||
| class_loader_hide_library_symbols(pass_through_filter_point_cloud2) |
There was a problem hiding this comment.
This is a bit harder to explain, see ros-perception/perception_pcl#9 . Basically, hiding the dynamic symbols helps interoperability of the plugin with other plugins. And it makes linking to the dynamic library impossible, which is however wanted.
There was a problem hiding this comment.
This one is great, I have never seen it :D
| @@ -69,10 +58,6 @@ install(TARGETS pass_through_filter_point_cloud2 | |||
| RUNTIME DESTINATION ${CATKIN_PACKAGE_BIN_DESTINATION} | |||
| ) | |||
|
|
|||
| # Install headers | |||
There was a problem hiding this comment.
As it is not expected any other package would link to this one, installing the includes does not make sense. It would only tempt users to write the includes, but then they'd find out they have no way to link to this plugin.
|
Thanks for the explanations, usually cmake has obscure commands for me. I just adapted this one copying from the robot body filter one. |
Here are a few suggestions to improve CMakeLists.txt of the project.