search

ros2 / rclcpp

rclcpp (ROS Client Library for C++)
Apache License 2.0
534 stars 413 forks source link

rclcpp_components_register_nodes() fails if not in the project's root CMakeLists.txt #1698

Open stonier opened 3 years ago

stonier commented 3 years ago

Bug report

Required Info:

Steps to reproduce issue

Expected behavior

The talker component will be present in share/ament_index/resource_index/rclcpp_components/composition, or at least, an error at CMake configure time.

Actual behavior

The talker component is missing.

iuhilnehc-ynos commented 3 years ago
  • Create a nested CMakeLists.txt in the demos/composition package
  • Move the talker_component logic into the nested CMakeLists.txt (be sure to include the registering macro)

Hi, @stonier

I suppose you want to use the subdirectory to organize the source code. I think you could refer to this link firstly. (FYI: I have tried the workaround (the method might be a little different from the above link) for the demo and it seems working.)

wjwwood commented 3 years ago

I think it should be technically possible to support what @stonier is doing, and probably a good idea to do so as well. @sloretz's workaround will work, but is somewhat fragile because it depends on a cmake variable name that we otherwise don't expose and could be changed in the future. I don't have time to work on this at the moment, but we could consider changing how we set/get that variable to work around the scoping, or maybe we could add a macro that must be called in the child scope to make this work (a less-fragile form of the PARENT_SCOPE workaround).

iuhilnehc-ynos commented 3 years ago

we could add a macro that must be called in the child scope to make this work

Yeah, this could be a method. But I think users want to use rclcpp_components_register_nodes in subdirectories in the same way as the root directory.

Patches that I am not sure if it's the right way.

rclcpp

```shell diff --git a/rclcpp_components/cmake/rclcpp_components_register_nodes.cmake b/rclcpp_components/cmake/rclcpp_components_register_nodes.cmake index e80550a3..74045f99 100644 --- a/rclcpp_components/cmake/rclcpp_components_register_nodes.cmake +++ b/rclcpp_components/cmake/rclcpp_components_register_nodes.cmake @@ -67,6 +67,15 @@ macro(rclcpp_components_register_nodes target) "${_RCLCPP_COMPONENTS_${resource_index}__NODES}${_arg};${_path}/$\n") list(APPEND _RCLCPP_COMPONENTS_PACKAGE_RESOURCE_INDICES ${resource_index}) endforeach() - endif() -endmacro() + # Check if current scope is subdirectory or not + if(NOT CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) + set(_RCLCPP_COMPONENTS_${resource_index}__NODES + "${_RCLCPP_COMPONENTS_${resource_index}__NODES}" + PARENT_SCOPE) + set(_RCLCPP_COMPONENTS_PACKAGE_RESOURCE_INDICES + ${_RCLCPP_COMPONENTS_PACKAGE_RESOURCE_INDICES} + PARENT_SCOPE) + endif() + endif() +endmacro() \ No newline at end of file diff --git a/rclcpp_components/rclcpp_components-extras.cmake.in b/rclcpp_components/rclcpp_components-extras.cmake.in index 45a4e5ac..5dc33d9b 100644 --- a/rclcpp_components/rclcpp_components-extras.cmake.in +++ b/rclcpp_components/rclcpp_components-extras.cmake.in @@ -17,7 +17,11 @@ # register ament_package() hook for node plugins once. macro(_rclcpp_components_register_package_hook) if(NOT DEFINED _RCLCPP_COMPONENTS_PACKAGE_HOOK_REGISTERED) - set(_RCLCPP_COMPONENTS_PACKAGE_HOOK_REGISTERED TRUE) + if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) + set(_RCLCPP_COMPONENTS_PACKAGE_HOOK_REGISTERED TRUE) + else() + set(_RCLCPP_COMPONENTS_PACKAGE_HOOK_REGISTERED TRUE PARENT_SCOPE) + endif() find_package(ament_cmake_core QUIET REQUIRED) ament_register_extension("ament_package" "rclcpp_components" ```

ament_cmake_core

```shell diff --git a/ament_cmake_core/cmake/core/ament_register_extension.cmake b/ament_cmake_core/cmake/core/ament_register_extension.cmake index 45effb2..b4118a4 100644 --- a/ament_cmake_core/cmake/core/ament_register_extension.cmake +++ b/ament_cmake_core/cmake/core/ament_register_extension.cmake @@ -30,4 +30,10 @@ macro(ament_register_extension extension_point package_name cmake_filename) list(APPEND AMENT_EXTENSIONS_${extension_point} "${package_name}:${cmake_filename}") + + if(NOT CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) + set(AMENT_EXTENSIONS_${extension_point} + ${AMENT_EXTENSIONS_${extension_point}} + PARENT_SCOPE) + endif() endmacro() ```

HighPriest commented 2 years ago

I think I am having the same issue, where my component is closely based on the DEMO, but the only thing which is returned by the ros2 pkg executables command and is linked to the project is a .so file with the name of the library created in CMakeLists.txt.

In my case I have two levels of add_subdirectory. One for the project level, which adds the src level, which adds all the components.