Add support for Humble #685
Conversation
| @@ -16,7 +16,28 @@ find_package(std_srvs REQUIRED) | |||
|
|
|||
| # TODO pjr What to do about swri_roscpp-extras.cmake? | |||
|
|
|||
| ### Build Test Node ### | |||
| add_library(${PROJECT_NAME} INTERFACE) | |||
There was a problem hiding this comment.
This will cause a cmake error if tests are not disabled; a target named ${PROJECT_NAME} is already defined below at line 105 with rosidl_generate_interfaces(${PROJECT_NAME}. If this needs to be built and installed as a library, I would recommend renaming the test target.
| @@ -88,7 +109,8 @@ if (BUILD_TESTING) | |||
| ) | |||
|
|
|||
| ament_add_gtest(topic_service_test_server test/topic_service_test.cpp) | |||
| rosidl_target_interfaces(topic_service_test_server ${PROJECT_NAME} "rosidl_typesupport_cpp") | |||
| rosidl_get_typesupport_target(cpp_typesupport_target "${PROJECT_NAME}" "rosidl_typesupport_cpp") | |||
There was a problem hiding this comment.
This will fail to compile on ROS Galactic or earlier because rosidl_get_typesupport_target does not exist.
You could replace this with something like:
if ("$ENV{ROS_DISTRO}" STRGREATER "galactic")
rosidl_get_typesupport_target(cpp_typesupport_target "${PROJECT_NAME}_test" "rosidl_typesupport_cpp")
target_link_libraries(topic_service_test_server "${cpp_typesupport_target}")
else()
rosidl_target_interfaces(topic_service_test_server "${PROJECT_NAME}_test" "rosidl_typesupport_cpp")
endif()
target_link_libraries(topic_service_test_server
Boost::system
swri_roscpp_test__rosidl_typesupport_cpp
)Note that rosidl_generate_interfaces(${PROJECT_NAME} above would also need to be changed to rosidl_generate_interfaces(${PROJECT_NAME}_test to reflect the new target name.
There was a problem hiding this comment.
Can also confirm it's broken on prior ROS2 releases.
| @@ -36,7 +36,7 @@ | |||
| #include <rcl_interfaces/msg/floating_point_range.hpp> | |||
| #include <rcl_interfaces/msg/parameter_descriptor.hpp> | |||
| #include <tf2_ros/transform_broadcaster.h> | |||
| #include <tf2_geometry_msgs/tf2_geometry_msgs.h> | |||
| #include <tf2_geometry_msgs/tf2_geometry_msgs.hpp> | |||
There was a problem hiding this comment.
This, and all other files that include tf2_geometry_msgs.hpp, will fail to compile on ROS Galactic and earlier because there the file is named tf2_geometry_msgs.h. Unfortunately, while that file exists in Humble, including it will print a warning that cannot be suppressed, which is incredibly annoying.
Probably the best way to make this work across different ROS2 releases is to modify the CMakeLists.txt to pull the ROS_DISTRO environment variable and set that in a compiler definition, then use preprocessor macros here to conditionally include either tf2_geometry_msgs.h or tf2_geometry_msgs.hpp.
There was a problem hiding this comment.
Can confirm it's broken on prior ROS2 releases.
|
@shrijitsingh99 did you want to make these changes? Otherwise I can fork your changes and make them. |
|
@pjreed I've started working on the C API switch here: https://github.com/danthony06/marti_common/tree/c_api_switch. |
|
@pjreed My C API branch is now building and passing tests, but I'm not entirely happy with the implementation and how it's handling GEOS contexts. |
|
I've started working on bringing these changes into a branch here: https://github.com/danthony06/marti_common/tree/humble_support |
|
@danthony06 Thanks for taking over this. I have not have had the cycles to take this further, sorry about that. |
|
@shrijitsingh99 Thank you for all your help on this. I've based a new PR on this over here: #691. I'm closing this PR in favor of the other one. |
* Adding Humble support based on @shrijitsingh99 PR in #685
Fixes dependencies and build issues while building for Humble
Updates geos library with and uses their new API
Fixes CMake issues