-
Notifications
You must be signed in to change notification settings - Fork 28
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
Misc fastrtps typesupport generator cleanup #87
Misc fastrtps typesupport generator cleanup #87
Conversation
Signed-off-by: Shane Loretz <[email protected]>
Signed-off-by: Shane Loretz <[email protected]>
Signed-off-by: Shane Loretz <[email protected]>
Signed-off-by: Shane Loretz <[email protected]>
Marked as draft while I open other PRs. So far in my local testing it seems fixing these issues requires fixing up cmake issues in rmw_dds_common which requires fixing up cmake issues in rosidl_typesupport_c which requires the cmake fixes in ros2/rosidl#662 . |
Depends on ros2/rmw_dds_common#57 |
@hidmic I've added you as a reviewer, but this one depends on ros2/rmw_dds_common#57 which depends on ros2/rosidl_typesupport#123 which depends on ros2/rosidl#662, so I've left it marked as a draft until those are merged. |
rosidl_typesupport_fastrtps_c/cmake/rosidl_typesupport_fastrtps_c_generate_interfaces.cmake
Outdated
Show resolved
Hide resolved
rosidl_typesupport_fastrtps_c/cmake/rosidl_typesupport_fastrtps_c_generate_interfaces.cmake
Show resolved
Hide resolved
rosidl_typesupport_fastrtps_c/cmake/rosidl_typesupport_fastrtps_c_generate_interfaces.cmake
Outdated
Show resolved
Hide resolved
rosidl_typesupport_fastrtps_cpp/cmake/rosidl_typesupport_fastrtps_cpp_generate_interfaces.cmake
Show resolved
Hide resolved
Signed-off-by: Shane Loretz <[email protected]> Co-authored-by: Michel Hidalgo <[email protected]>
Signed-off-by: Shane Loretz <[email protected]>
Signed-off-by: Shane Loretz <[email protected]>
…ithub.com:ros2/rosidl_typesupport_fastrtps into sloretz__rosidl_typesupport_fastrtps_cmake_issues
Signed-off-by: Shane Loretz <[email protected]>
Signed-off-by: Shane Loretz <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once this comes out of draft mode
Leaving in draft until ros2/rosidl_typesupport#123 and then ros2/rmw_dds_common#57 are merged. |
This cleans up a bunch of stuff to do with the generators for
rosidl_typesupport_fastrps_c
androsidl_typesupport_fastrtps_cpp
find_package()
dependencies needed by generated coderosidl_typesupport_fastrtps_cpp
find_packagerosidl_generator_cpp
so it generates files firstament_export_dependencies(...)
whenSKIP_INSTALL
is used.CMakeLists.txt
and into the generator's CMake codetarget_compile_definitions(...BUILDING_DLL...)
line in anif(WIN32)
block because it's unnecessary. The visibility header seems to do stuff on other platforms too.target_link_libraries()
instead ofament_target_dependencies()
Is ament_target_dependencies() still necessary? ament/ament_cmake#292 in a couple placestarget_link_libraries()
to the generated target of the other rather than hard-coding the directory it's likely to create in the build foldertarget_link_libraries()
on targets from this generator in dependency packages, use the..._TARGETS${_target_suffix}
variable instead of the..._LIBRARIES${_target_suffix}
rosidl_export_typesupport_targets()
where it wasn't alreadyif(NOT _generated_headers...
checks - because they don't seem to do anythingpackage.xml
s with commentsDepends on ros2/rmw_dds_common#57