From aa46bed408d9d0216fe1061cf8bc0b915f12bf63 Mon Sep 17 00:00:00 2001 From: Tony Peng Date: Tue, 12 Oct 2021 23:25:26 -0700 Subject: [PATCH 1/2] Change the way how visibility file for each pb header is added --- rosidl_adapter_proto/bin/rosidl_adapter_proto | 18 +++++++++++++ .../cmake/rosidl_adapt_proto_interfaces.cmake | 27 ++++++++++++------- .../rosidl_adapter_proto-extras.cmake.in | 21 +-------------- 3 files changed, 36 insertions(+), 30 deletions(-) diff --git a/rosidl_adapter_proto/bin/rosidl_adapter_proto b/rosidl_adapter_proto/bin/rosidl_adapter_proto index bf49b23..57e048d 100644 --- a/rosidl_adapter_proto/bin/rosidl_adapter_proto +++ b/rosidl_adapter_proto/bin/rosidl_adapter_proto @@ -79,6 +79,24 @@ def main(argv=sys.argv[1:]): proto_files = proto_files, package_name = package_name ) + + # Modify the generated .pb.h to include visibility file + for proto_file in proto_files: + # Just in case if the path contains another .proto, e.g., + # /home/.proto/ + proto_file = proto_file.split("/") + msg_type = proto_file[-2] + msg = os.path.splitext(proto_file[-1])[0] + + with open(f"{cpp_out_dir}/{package_name}/{msg_type}/{msg}.pb.h", "r") as f: + contents = f.readlines() + + # Insert the visibility file on line 6 + contents.insert( + 6, + f"#include <{package_name}/rosidl_adapter_proto__visibility_control.h>\n") + with open(f"{cpp_out_dir}/{package_name}/{msg_type}/{msg}.pb.h", "w") as f: + f.write("".join(contents)) return rc diff --git a/rosidl_adapter_proto/cmake/rosidl_adapt_proto_interfaces.cmake b/rosidl_adapter_proto/cmake/rosidl_adapt_proto_interfaces.cmake index 230e933..aa50d91 100644 --- a/rosidl_adapter_proto/cmake/rosidl_adapt_proto_interfaces.cmake +++ b/rosidl_adapter_proto/cmake/rosidl_adapt_proto_interfaces.cmake @@ -83,14 +83,21 @@ configure_file( @ONLY ) -install( - DIRECTORY ${rosidl_adapter_proto_OUTPUT_DIR} - DESTINATION "include/" - PATTERN "*.h" -) +# Headers inside `build` folder is meant to be used by the current msg +# package. Anything downstream packages should use the headers in `install` +# folder. +include_directories("${CMAKE_CURRENT_BINARY_DIR}/rosidl_adapter_proto") + +if(NOT rosidl_generate_interfaces_SKIP_INSTALL) + install( + DIRECTORY ${rosidl_adapter_proto_OUTPUT_DIR} + DESTINATION "include/" + PATTERN "*.h" + ) -install( - DIRECTORY ${rosidl_adapter_proto_OUTPUT_DIR} - DESTINATION "share/" - PATTERN "*.proto" -) \ No newline at end of file + install( + DIRECTORY ${rosidl_adapter_proto_OUTPUT_DIR} + DESTINATION "share/" + PATTERN "*.proto" + ) +endif() diff --git a/rosidl_adapter_proto/cmake/rosidl_adapter_proto-extras.cmake.in b/rosidl_adapter_proto/cmake/rosidl_adapter_proto-extras.cmake.in index 66d4339..16410ea 100644 --- a/rosidl_adapter_proto/cmake/rosidl_adapter_proto-extras.cmake.in +++ b/rosidl_adapter_proto/cmake/rosidl_adapter_proto-extras.cmake.in @@ -11,7 +11,7 @@ else() ament_register_extension( "rosidl_generate_idl_interfaces" "rosidl_adapter_proto" - "${rosidl_adapter_proto_DIR}/rosidl_adapt_proto_interfaces.cmake") + "rosidl_adapt_proto_interfaces.cmake") set(rosidl_adapter_proto_BIN "${rosidl_adapter_proto_DIR}/../../../lib/rosidl_adapter_proto/rosidl_adapter_proto") @@ -28,25 +28,6 @@ else() normalize_path(rosidl_adapter_proto_TEMPLATE_DIR "${rosidl_adapter_proto_TEMPLATE_DIR}") - include("${rosidl_adapter_proto_DIR}/rosidl_adapt_proto_interfaces.cmake") - add_compile_definitions("ROSIDL_ADAPTER_PROTO_BUILDING_DLL__${PROJECT_NAME}") - # There is no clean way to include additional files into protobuf headers - if(NOT WIN32) - add_compile_options("-include${rosidl_adapter_proto_VISIBILITY_CONTROL_HEADER}") - else() - add_compile_options( "/FI\"${rosidl_adapter_proto_VISIBILITY_CONTROL_HEADER}\"") - endif() - - foreach(_pkg_name ${rosidl_generate_interfaces_DEPENDENCY_PACKAGE_NAMES}) - set(_proto_dir "${${_pkg_name}_DIR}/../../../include/${_pkg_name}/rosidl_adapter_proto__visibility_control.h") - normalize_path(_proto_dir "${_proto_dir}") - if(NOT WIN32) - add_compile_options("-include${_proto_dir}") - else() - add_compile_options( "/FI\"${_proto_dir}\"") - endif() - endforeach() - endif() From 41c0b20c7087e642a35b2f0d6ec6902e9c89c75f Mon Sep 17 00:00:00 2001 From: Tony Peng Date: Tue, 12 Oct 2021 23:27:07 -0700 Subject: [PATCH 2/2] Reformat the way how dependencies are exported in protobuf packages --- rosidl_typesupport_protobuf/CMakeLists.txt | 9 ++++++++- rosidl_typesupport_protobuf/package.xml | 1 + rosidl_typesupport_protobuf_c/CMakeLists.txt | 10 ++-------- .../rosidl_typesupport_protobuf_c-extras.cmake.in | 1 + ...typesupport_protobuf_c_generate_interfaces.cmake | 4 +++- rosidl_typesupport_protobuf_cpp/CMakeLists.txt | 11 ++++++++--- .../rosidl_typesupport_protobuf_cpp-extras.cmake.in | 1 + ...pesupport_protobuf_cpp_generate_interfaces.cmake | 13 ++++++++++--- 8 files changed, 34 insertions(+), 16 deletions(-) diff --git a/rosidl_typesupport_protobuf/CMakeLists.txt b/rosidl_typesupport_protobuf/CMakeLists.txt index f881b69..980cc9f 100644 --- a/rosidl_typesupport_protobuf/CMakeLists.txt +++ b/rosidl_typesupport_protobuf/CMakeLists.txt @@ -22,8 +22,15 @@ project(rosidl_typesupport_protobuf) find_package(ament_cmake REQUIRED) find_package(ament_cmake_python REQUIRED) -find_package(rosidl_generator_c REQUIRED) +include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/discover-ros-distro-extras.cmake) +if(is_foxy_or_greater) + find_package(rosidl_runtime_c REQUIRED) + ament_export_dependencies(rosidl_runtime_c) +else() + find_package(rosidl_generator_c REQUIRED) + ament_export_dependencies(rosidl_generator_c) +endif() ament_export_include_directories(include) ament_python_install_package(${PROJECT_NAME}) diff --git a/rosidl_typesupport_protobuf/package.xml b/rosidl_typesupport_protobuf/package.xml index 9ea19be..024b0c4 100644 --- a/rosidl_typesupport_protobuf/package.xml +++ b/rosidl_typesupport_protobuf/package.xml @@ -9,6 +9,7 @@ ament_cmake rosidl_generator_c + rosidl_runtime_c rosidl_generator_c diff --git a/rosidl_typesupport_protobuf_c/CMakeLists.txt b/rosidl_typesupport_protobuf_c/CMakeLists.txt index a7e25cd..ca2c2d1 100644 --- a/rosidl_typesupport_protobuf_c/CMakeLists.txt +++ b/rosidl_typesupport_protobuf_c/CMakeLists.txt @@ -57,19 +57,11 @@ endif() find_package(rmw REQUIRED) find_package(ament_cmake_python REQUIRED) find_package(rosidl_typesupport_protobuf REQUIRED) -find_package(rosidl_generator_c REQUIRED) -if(${is_foxy_or_greater}) - find_package(rosidl_runtime_c REQUIRED) -endif() find_package(rosidl_typesupport_introspection_c REQUIRED) find_package(rosidl_typesupport_introspection_cpp REQUIRED) ament_export_dependencies(rmw) ament_export_dependencies(rosidl_cmake) -ament_export_dependencies(rosidl_generator_c) -if(${is_foxy_or_greater}) - ament_export_dependencies(rosidl_runtime_c) -endif() ament_export_dependencies(rosidl_typesupport_introspection_c) ament_export_dependencies(rosidl_typesupport_introspection_cpp) ament_export_dependencies(rosidl_typesupport_interface) @@ -77,6 +69,8 @@ ament_export_dependencies(rosidl_typesupport_protobuf) ament_export_include_directories(include) +ament_export_dependencies(rosidl_adapter_proto) + ament_python_install_package(${PROJECT_NAME}) add_library(${PROJECT_NAME} SHARED diff --git a/rosidl_typesupport_protobuf_c/cmake/rosidl_typesupport_protobuf_c-extras.cmake.in b/rosidl_typesupport_protobuf_c/cmake/rosidl_typesupport_protobuf_c-extras.cmake.in index 9a297aa..a5afc27 100644 --- a/rosidl_typesupport_protobuf_c/cmake/rosidl_typesupport_protobuf_c-extras.cmake.in +++ b/rosidl_typesupport_protobuf_c/cmake/rosidl_typesupport_protobuf_c-extras.cmake.in @@ -2,6 +2,7 @@ # rosidl_typesupport_protobuf_c/rosidl_typesupport_protobuf_c-extras.cmake.in find_package(Protobuf REQUIRED) +find_package(rosidl_adapter_proto REQUIRED) if(NOT Protobuf_FOUND) message(STATUS "Could not find Protobuf: skipping rosidl_typesupport_protobuf_c") diff --git a/rosidl_typesupport_protobuf_c/cmake/rosidl_typesupport_protobuf_c_generate_interfaces.cmake b/rosidl_typesupport_protobuf_c/cmake/rosidl_typesupport_protobuf_c_generate_interfaces.cmake index 7077c9c..bd24682 100644 --- a/rosidl_typesupport_protobuf_c/cmake/rosidl_typesupport_protobuf_c_generate_interfaces.cmake +++ b/rosidl_typesupport_protobuf_c/cmake/rosidl_typesupport_protobuf_c_generate_interfaces.cmake @@ -142,6 +142,8 @@ configure_file( @ONLY ) +ament_export_dependencies(rosidl_typesupport_protobuf rosidl_typesupport_protobuf_c) + # Depend on dependencies foreach(_pkg_name ${rosidl_generate_interfaces_DEPENDENCY_PACKAGE_NAMES}) ament_target_dependencies(${rosidl_generate_interfaces_TARGET}${_target_suffix} @@ -181,5 +183,5 @@ if(NOT rosidl_generate_interfaces_SKIP_INSTALL) rosidl_export_typesupport_libraries(${_target_suffix} ${rosidl_generate_interfaces_TARGET}${_target_suffix}) - ament_export_libraries(${rosidl_generate_interfaces_TARGET}${_target_suffix}) + ament_export_libraries(${rosidl_generate_interfaces_TARGET}${_target_suffix} ${Protobuf_LIBRARY}) endif() diff --git a/rosidl_typesupport_protobuf_cpp/CMakeLists.txt b/rosidl_typesupport_protobuf_cpp/CMakeLists.txt index d7697aa..2ae9dc8 100644 --- a/rosidl_typesupport_protobuf_cpp/CMakeLists.txt +++ b/rosidl_typesupport_protobuf_cpp/CMakeLists.txt @@ -40,7 +40,6 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "MSVC") add_compile_definitions(_CRT_SECURE_NO_WARNINGS) endif() -find_package(ament_cmake REQUIRED) if(PROTOBUF_STATIC_DISABLE) ament_package() message(STATUS "Protobuf static typesupport implementation explicitly disabled - skipping '${PROJECT_NAME}'") @@ -54,13 +53,19 @@ find_package(rosidl_generator_c REQUIRED) ament_export_dependencies(rmw) ament_export_dependencies(rcutils) ament_export_dependencies(rosidl_cmake) -ament_export_dependencies(rosidl_generator_c) -ament_export_dependencies(rosidl_generator_cpp) ament_export_dependencies(rosidl_typesupport_interface) ament_export_dependencies(rosidl_typesupport_protobuf) +ament_export_dependencies(rosidl_adapter_proto) ament_export_include_directories(include) +if(${is_foxy_or_greater}) + ament_export_dependencies(rosidl_runtime_cpp) +else() + ament_export_dependencies(rosidl_generator_c) + ament_export_dependencies(rosidl_generator_cpp) +endif() + ament_python_install_package(${PROJECT_NAME}) add_library(${PROJECT_NAME} SHARED diff --git a/rosidl_typesupport_protobuf_cpp/cmake/rosidl_typesupport_protobuf_cpp-extras.cmake.in b/rosidl_typesupport_protobuf_cpp/cmake/rosidl_typesupport_protobuf_cpp-extras.cmake.in index fc06bc4..92578d2 100644 --- a/rosidl_typesupport_protobuf_cpp/cmake/rosidl_typesupport_protobuf_cpp-extras.cmake.in +++ b/rosidl_typesupport_protobuf_cpp/cmake/rosidl_typesupport_protobuf_cpp-extras.cmake.in @@ -3,6 +3,7 @@ # rosidl_typesupport_protobuf_cpp-extras.cmake.in find_package(Protobuf REQUIRED) +find_package(rosidl_adapter_proto REQUIRED) if(NOT Protobuf_FOUND) message(STATUS diff --git a/rosidl_typesupport_protobuf_cpp/cmake/rosidl_typesupport_protobuf_cpp_generate_interfaces.cmake b/rosidl_typesupport_protobuf_cpp/cmake/rosidl_typesupport_protobuf_cpp_generate_interfaces.cmake index 62c4068..01e4acd 100644 --- a/rosidl_typesupport_protobuf_cpp/cmake/rosidl_typesupport_protobuf_cpp_generate_interfaces.cmake +++ b/rosidl_typesupport_protobuf_cpp/cmake/rosidl_typesupport_protobuf_cpp_generate_interfaces.cmake @@ -140,8 +140,13 @@ set_target_properties(${rosidl_generate_interfaces_TARGET}${_target_suffix} ) # Include headers from other generators -target_include_directories(${rosidl_generate_interfaces_TARGET}${_target_suffix} - PUBLIC +# This is supposed to be +# `target_include_directories(${rosidl_generate_interfaces_TARGET}${_target_suffix}` +# but there's no way to pass the include folder to the top level target +# ${rosidl_generate_interfaces_TARGET}. +# This include_directories will only be used if message is used in the package +# where it's created +include_directories( ${_output_path} ${Protobuf_INCLUDE_DIR} ${rosidl_adapter_proto_INCLUDE_DIR} @@ -156,6 +161,8 @@ ament_target_dependencies(${rosidl_generate_interfaces_TARGET}${_target_suffix} rosidl_typesupport_interface ) +ament_export_dependencies(rosidl_typesupport_protobuf rosidl_typesupport_protobuf_cpp) + # Depend on dependencies foreach(_pkg_name ${rosidl_generate_interfaces_DEPENDENCY_PACKAGE_NAMES}) ament_target_dependencies(${rosidl_generate_interfaces_TARGET}${_target_suffix} @@ -199,5 +206,5 @@ if(NOT rosidl_generate_interfaces_SKIP_INSTALL) rosidl_export_typesupport_libraries(${_target_suffix} ${rosidl_generate_interfaces_TARGET}${_target_suffix}) - ament_export_libraries(${rosidl_generate_interfaces_TARGET}${_target_suffix}) + ament_export_libraries(${rosidl_generate_interfaces_TARGET}${_target_suffix} ${Protobuf_LIBRARY}) endif() \ No newline at end of file