From adf80ba17762a9bad31f4c251bde1481396d329a Mon Sep 17 00:00:00 2001 From: Ryan Friedman Date: Fri, 19 Jan 2024 17:26:13 -0700 Subject: [PATCH 1/2] Use ament_export_targets * Remove legacy ament functions * Add ament_export_targets * Match namespace for conan and ROS * Depends on https://github.com/ament/ament_cmake/pull/498 merging and backport to humble * Add CI for export tests and colcon building Signed-off-by: Ryan Friedman --- .github/workflows/colcon_ament.yml | 64 +++++++++++++++ CMakeLists.txt | 3 + README.md | 2 +- cmake/ament_build.cmake | 10 ++- cmake/conan_build.cmake | 2 +- tests/export/README.md | 5 ++ tests/export/ament_target_deps/CMakeLists.txt | 15 ++++ tests/export/ament_target_deps/main.cpp | 78 +++++++++++++++++++ tests/export/target_link_libs/CMakeLists.txt | 16 ++++ tests/export/target_link_libs/main.cpp | 78 +++++++++++++++++++ 10 files changed, 267 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/colcon_ament.yml create mode 100644 tests/export/README.md create mode 100644 tests/export/ament_target_deps/CMakeLists.txt create mode 100644 tests/export/ament_target_deps/main.cpp create mode 100644 tests/export/target_link_libs/CMakeLists.txt create mode 100644 tests/export/target_link_libs/main.cpp diff --git a/.github/workflows/colcon_ament.yml b/.github/workflows/colcon_ament.yml new file mode 100644 index 000000000..c4d082439 --- /dev/null +++ b/.github/workflows/colcon_ament.yml @@ -0,0 +1,64 @@ +name: colcon ament Ubuntu + +on: [push, pull_request] + +env: + # Customize the CMake build type here (Release, Debug, RelWithDebInfo, etc.) + BUILD_TYPE: Release + +jobs: + build: + container: ${{ matrix.container }} + runs-on: ${{ matrix.os }} + strategy: + matrix: + container: ['osrf/ros:humble-desktop', 'osrf/ros:rolling-desktop'] + os: [ubuntu-22.04] + ros_distro: [humble, rolling] + include: + - container: osrf/ros:humble-desktop + ros_distro: humble + - container: osrf/ros:rolling-desktop + ros_distro: rolling + steps: + - uses: actions/checkout@v2 + + - name: Install binary dependencies with rosdep + id: rosdep + run: | + apt update + rosdep update + source /opt/ros/${{matrix.rosdistro}}/setup.bash + rosdep install --from-paths src --ignore-src -y --rosdistro ${{matrix.rosdistro}} + shell: bash + + - name: Build with colcon + id: colcon-build + run: | + source /opt/ros/${{matrix.rosdistro}}/setup.bash + colcon build + shell: bash + + - name: Test with colcon + id: colcon-test + run: | + source /opt/ros/${{matrix.rosdistro}}/setup.bash + colcon test + colcon test-result --all --verbose + shell: bash + + - name: Test consuming with ament_target_dependencies + run: | + source /opt/ros/${{matrix.rosdistro}}/setup.bash + # This sets the variable AMENT_PREFIX_PATH + source install/setup.bash + cd tests/export/ament_target_deps + colcon build + + - name: Test consuming with target_link_libraries + run: | + source /opt/ros/${{matrix.rosdistro}}/setup.bash + # This sets the variable AMENT_PREFIX_PATH + source install/setup.bash + cd tests/export/target_link_libs + colcon build diff --git a/CMakeLists.txt b/CMakeLists.txt index 5a2e972b2..5fc439a8f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -50,6 +50,9 @@ endif() find_package(ament_cmake QUIET) +# This is a private variable to control the exported namespace name for installation +# macros that support namespace targets. +set(_BTCPP_EXPORTED_NAMESPACE BT::) if ( ament_cmake_FOUND ) add_definitions( -DUSING_ROS2 ) diff --git a/README.md b/README.md index add4b0f60..3acf5fd61 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ If you are looking for a more fancy graphical user interface (and I know you do) **BT.CPP** requires a compile that supports c++17. -Three build systems are supported: +Four build systems are supported: - **catkin**, if you use ROS - **colcon (ament)**, if you use ROS2 diff --git a/cmake/ament_build.cmake b/cmake/ament_build.cmake index 9de2fd099..4bc025e39 100644 --- a/cmake/ament_build.cmake +++ b/cmake/ament_build.cmake @@ -18,8 +18,6 @@ set( BTCPP_EXTRA_LIBRARIES $ ) -ament_export_dependencies(ament_index_cpp) - set( BTCPP_LIB_DESTINATION lib ) set( BTCPP_INCLUDE_DESTINATION include ) set( BTCPP_BIN_DESTINATION bin ) @@ -31,7 +29,11 @@ mark_as_advanced( BTCPP_BIN_DESTINATION ) macro(export_btcpp_package) - ament_export_include_directories(include) - ament_export_libraries(${BTCPP_LIBRARY}) + ament_export_targets( + ${PROJECT_NAME}Targets + NAMESPACE "{_BTCPP_EXPORTED_NAMESPACE}" + HAS_LIBRARY_TARGET + ) + ament_export_dependencies(ament_index_cpp) ament_package() endmacro() diff --git a/cmake/conan_build.cmake b/cmake/conan_build.cmake index 3bf3a7225..5a0026d47 100644 --- a/cmake/conan_build.cmake +++ b/cmake/conan_build.cmake @@ -29,7 +29,7 @@ macro(export_btcpp_package) install(EXPORT ${PROJECT_NAME}Targets FILE "${PROJECT_NAME}Targets.cmake" DESTINATION "${BTCPP_LIB_DESTINATION}/cmake/${PROJECT_NAME}" - NAMESPACE BT:: + NAMESPACE "{_BTCPP_EXPORTED_NAMESPACE}" ) export(PACKAGE ${PROJECT_NAME}) diff --git a/tests/export/README.md b/tests/export/README.md new file mode 100644 index 000000000..e533ea975 --- /dev/null +++ b/tests/export/README.md @@ -0,0 +1,5 @@ +# Export tests + +The purpose of the tests below is to ensure that BTCPP exports itself correctly for use with find_package. + +Without testing this, it is easy to accidentally break consumer applications when working on CMake installation code. diff --git a/tests/export/ament_target_deps/CMakeLists.txt b/tests/export/ament_target_deps/CMakeLists.txt new file mode 100644 index 000000000..d0e312dff --- /dev/null +++ b/tests/export/ament_target_deps/CMakeLists.txt @@ -0,0 +1,15 @@ +cmake_minimum_required(VERSION 3.22.1) # version on Ubuntu Jammy +project(btcpp_ament_tgt_dep LANGUAGES CXX) + +set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD_REQUIRED ON) + +find_package(ament_cmake 1 CONFIG REQUIRED) +find_package(behaviortree_cpp 4 REQUIRED) + +add_executable(btcpp_sample main.cpp) + +# This function doesn't yet support PRIVATE as an argument. +# See https://github.com/ament/ament_cmake/issues/158#issuecomment-475384147 +# Instead, test preserving legacy usage with no linkage specifier. +ament_target_dependencies(btcpp_sample behaviortree_cpp) diff --git a/tests/export/ament_target_deps/main.cpp b/tests/export/ament_target_deps/main.cpp new file mode 100644 index 000000000..aa7bb9b25 --- /dev/null +++ b/tests/export/ament_target_deps/main.cpp @@ -0,0 +1,78 @@ +// Copy of https://github.com/BehaviorTree/btcpp_sample/blob/main/main.cpp + +#include "behaviortree_cpp/bt_factory.h" + +using namespace BT; + +// clang-format off +static const char* xml_text = R"( + + + + + + + + + + + + + + )"; +// clang-format on + +class SaySomething : public BT::SyncActionNode +{ + public: + SaySomething(const std::string& name, const BT::NodeConfig& config) : + BT::SyncActionNode(name, config) + {} + + BT::NodeStatus tick() override + { + std::string msg; + getInput("message", msg); + std::cout << msg << std::endl; + return BT::NodeStatus::SUCCESS; + } + + static BT::PortsList providedPorts() + { + return {BT::InputPort("message")}; + } +}; + +class ThinkWhatToSay : public BT::SyncActionNode +{ + public: + ThinkWhatToSay(const std::string& name, const BT::NodeConfig& config) : + BT::SyncActionNode(name, config) + {} + + BT::NodeStatus tick() override + { + setOutput("text", "The answer is 42"); + return BT::NodeStatus::SUCCESS; + } + + static BT::PortsList providedPorts() + { + return {BT::OutputPort("text")}; + } +}; + +int main() +{ + + BehaviorTreeFactory factory; + + factory.registerNodeType("SaySomething"); + factory.registerNodeType("ThinkWhatToSay"); + + auto tree = factory.createTreeFromText(xml_text); + + tree.tickWhileRunning(); + + return 0; +} diff --git a/tests/export/target_link_libs/CMakeLists.txt b/tests/export/target_link_libs/CMakeLists.txt new file mode 100644 index 000000000..c81b9fae5 --- /dev/null +++ b/tests/export/target_link_libs/CMakeLists.txt @@ -0,0 +1,16 @@ +cmake_minimum_required(VERSION 3.22.1) # version on Ubuntu Jammy +project(btcpp_tgt_link_libs LANGUAGES CXX) + + +set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD_REQUIRED ON) + +find_package(ament_cmake 1 CONFIG REQUIRED) +set(CMAKE_FIND_DEBUG_MODE TRUE) +find_package(behaviortree_cpp 4 CONFIG REQUIRED) +set(CMAKE_FIND_DEBUG_MODE FALSE) + +add_executable(btcpp_sample main.cpp) + +# Check it works with the same target that conan users use +target_link_libraries(btcpp_sample PRIVATE BT::behaviortree_cpp) diff --git a/tests/export/target_link_libs/main.cpp b/tests/export/target_link_libs/main.cpp new file mode 100644 index 000000000..aa7bb9b25 --- /dev/null +++ b/tests/export/target_link_libs/main.cpp @@ -0,0 +1,78 @@ +// Copy of https://github.com/BehaviorTree/btcpp_sample/blob/main/main.cpp + +#include "behaviortree_cpp/bt_factory.h" + +using namespace BT; + +// clang-format off +static const char* xml_text = R"( + + + + + + + + + + + + + + )"; +// clang-format on + +class SaySomething : public BT::SyncActionNode +{ + public: + SaySomething(const std::string& name, const BT::NodeConfig& config) : + BT::SyncActionNode(name, config) + {} + + BT::NodeStatus tick() override + { + std::string msg; + getInput("message", msg); + std::cout << msg << std::endl; + return BT::NodeStatus::SUCCESS; + } + + static BT::PortsList providedPorts() + { + return {BT::InputPort("message")}; + } +}; + +class ThinkWhatToSay : public BT::SyncActionNode +{ + public: + ThinkWhatToSay(const std::string& name, const BT::NodeConfig& config) : + BT::SyncActionNode(name, config) + {} + + BT::NodeStatus tick() override + { + setOutput("text", "The answer is 42"); + return BT::NodeStatus::SUCCESS; + } + + static BT::PortsList providedPorts() + { + return {BT::OutputPort("text")}; + } +}; + +int main() +{ + + BehaviorTreeFactory factory; + + factory.registerNodeType("SaySomething"); + factory.registerNodeType("ThinkWhatToSay"); + + auto tree = factory.createTreeFromText(xml_text); + + tree.tickWhileRunning(); + + return 0; +} From 4de9b00841a48973aaca1f9bc44c02545c8bbbfa Mon Sep 17 00:00:00 2001 From: Ryan Friedman Date: Fri, 19 Jan 2024 20:44:52 -0700 Subject: [PATCH 2/2] Prevent cross-talk between distros in matrix Signed-off-by: Ryan Friedman --- .github/workflows/colcon_ament.yml | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/.github/workflows/colcon_ament.yml b/.github/workflows/colcon_ament.yml index c4d082439..9fd2e2984 100644 --- a/.github/workflows/colcon_ament.yml +++ b/.github/workflows/colcon_ament.yml @@ -8,18 +8,12 @@ env: jobs: build: - container: ${{ matrix.container }} + container: osrf/ros:${{matrix.ros_distro}}-desktop runs-on: ${{ matrix.os }} strategy: matrix: - container: ['osrf/ros:humble-desktop', 'osrf/ros:rolling-desktop'] os: [ubuntu-22.04] ros_distro: [humble, rolling] - include: - - container: osrf/ros:humble-desktop - ros_distro: humble - - container: osrf/ros:rolling-desktop - ros_distro: rolling steps: - uses: actions/checkout@v2 @@ -28,28 +22,28 @@ jobs: run: | apt update rosdep update - source /opt/ros/${{matrix.rosdistro}}/setup.bash - rosdep install --from-paths src --ignore-src -y --rosdistro ${{matrix.rosdistro}} + source /opt/ros/${{matrix.ros_distro}}/setup.bash + rosdep install --from-paths src --ignore-src -y shell: bash - name: Build with colcon id: colcon-build run: | - source /opt/ros/${{matrix.rosdistro}}/setup.bash + source /opt/ros/${{matrix.ros_distro}}/setup.bash colcon build shell: bash - name: Test with colcon id: colcon-test run: | - source /opt/ros/${{matrix.rosdistro}}/setup.bash + source /opt/ros/${{matrix.ros_distro}}/setup.bash colcon test colcon test-result --all --verbose shell: bash - name: Test consuming with ament_target_dependencies run: | - source /opt/ros/${{matrix.rosdistro}}/setup.bash + source /opt/ros/${{matrix.ros_distro}}/setup.bash # This sets the variable AMENT_PREFIX_PATH source install/setup.bash cd tests/export/ament_target_deps @@ -57,7 +51,7 @@ jobs: - name: Test consuming with target_link_libraries run: | - source /opt/ros/${{matrix.rosdistro}}/setup.bash + source /opt/ros/${{matrix.ros_distro}}/setup.bash # This sets the variable AMENT_PREFIX_PATH source install/setup.bash cd tests/export/target_link_libs