Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Switch nav2_map_server to use modern CMake idioms. #4557

Merged
merged 2 commits into from
Jul 24, 2024
Merged

Switch nav2_map_server to use modern CMake idioms. #4557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9b2362f
Switch nav2_map_server to use modern CMake idioms.
clalancette Jul 24, 2024
66e300b
Make library_name depend on map_io_library_name.
clalancette Jul 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
201 changes: 106 additions & 95 deletions nav2_map_server/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,135 +4,143 @@ project(nav2_map_server)
list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_LIST_DIR}/cmake_modules)

find_package(ament_cmake REQUIRED)
find_package(GRAPHICSMAGICKCPP REQUIRED)
find_package(lifecycle_msgs REQUIRED)
find_package(nav_msgs REQUIRED)
find_package(nav2_common REQUIRED)
find_package(nav2_msgs REQUIRED)
find_package(nav2_util REQUIRED)
find_package(rclcpp REQUIRED)
find_package(rclcpp_lifecycle REQUIRED)
find_package(rclcpp_components REQUIRED)
find_package(nav_msgs REQUIRED)
find_package(nav2_msgs REQUIRED)
find_package(yaml_cpp_vendor REQUIRED)
find_package(rclcpp_lifecycle REQUIRED)
find_package(std_msgs REQUIRED)
find_package(tf2 REQUIRED)
find_package(nav2_util REQUIRED)
find_package(GRAPHICSMAGICKCPP REQUIRED)
find_package(yaml_cpp_vendor REQUIRED)
find_package(yaml-cpp REQUIRED)

nav2_package()

include_directories(include)

set(map_server_executable map_server)

set(library_name ${map_server_executable}_core)

set(map_io_library_name map_io)

set(map_saver_cli_executable map_saver_cli)

set(map_saver_server_executable map_saver_server)

set(costmap_filter_info_server_executable costmap_filter_info_server)

add_executable(${map_server_executable}
src/map_server/main.cpp)

add_executable(${map_saver_cli_executable}
src/map_saver/main_cli.cpp)

add_executable(${map_saver_server_executable}
src/map_saver/main_server.cpp)

add_executable(${costmap_filter_info_server_executable}
src/costmap_filter_info/main.cpp)

set(map_io_library_name map_io)

set(library_name ${map_server_executable}_core)

add_library(${map_io_library_name} SHARED
src/map_mode.cpp
src/map_io.cpp)
target_include_directories(${map_io_library_name}
PUBLIC
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<INSTALL_INTERFACE:include/${PROJECT_NAME}>")
target_include_directories(${map_io_library_name}
PRIVATE
${GRAPHICSMAGICKCPP_INCLUDE_DIRS})
target_link_libraries(${map_io_library_name} PUBLIC
nav2_util::nav2_util_core
${nav_msgs_TARGETS}
)
target_link_libraries(${map_io_library_name} PRIVATE
${GRAPHICSMAGICKCPP_LIBRARIES}
tf2::tf2
yaml-cpp::yaml-cpp
)

add_library(${library_name} SHARED
src/map_server/map_server.cpp
src/map_saver/map_saver.cpp
src/costmap_filter_info/costmap_filter_info_server.cpp)
target_include_directories(${library_name}
PUBLIC
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<INSTALL_INTERFACE:include/${PROJECT_NAME}>")
target_link_libraries(${library_name} PUBLIC
${map_io_library_name}
${nav_msgs_TARGETS}
${nav2_msgs_TARGETS}
nav2_util::nav2_util_core
rclcpp::rclcpp
rclcpp_lifecycle::rclcpp_lifecycle
)
target_link_libraries(${library_name} PRIVATE
${lifecycle_msgs_TARGETS}
rclcpp_components::component
yaml-cpp::yaml-cpp
)

set(map_io_dependencies
yaml_cpp_vendor
nav_msgs
nav2_util
tf2)

set(map_server_dependencies
rclcpp
rclcpp_lifecycle
rclcpp_components
nav_msgs
nav2_msgs
yaml_cpp_vendor
std_msgs
nav2_util)

set(map_saver_dependencies
rclcpp
rclcpp_lifecycle
nav_msgs
nav2_msgs
nav2_util)

ament_target_dependencies(${map_server_executable}
${map_server_dependencies})

ament_target_dependencies(${map_saver_cli_executable}
${map_saver_dependencies})

ament_target_dependencies(${map_saver_server_executable}
${map_saver_dependencies})

ament_target_dependencies(${costmap_filter_info_server_executable}
${map_saver_dependencies})

ament_target_dependencies(${library_name}
${map_server_dependencies})

ament_target_dependencies(${map_io_library_name}
${map_io_dependencies})

target_link_libraries(${library_name}
${map_io_library_name})

target_link_libraries(${map_server_executable}
${library_name})

if(WIN32)
target_compile_definitions(${map_server_executable} PRIVATE
YAML_CPP_DLL)
endif()

target_link_libraries(${map_saver_cli_executable}
${library_name})

target_link_libraries(${map_saver_server_executable}
${library_name})

target_link_libraries(${costmap_filter_info_server_executable}
${library_name})
add_executable(${map_server_executable}
src/map_server/main.cpp)
target_include_directories(${map_server_executable}
PRIVATE
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<INSTALL_INTERFACE:include/${PROJECT_NAME}>")
target_link_libraries(${map_server_executable} PRIVATE
${library_name}
${map_io_library_name}
rclcpp::rclcpp
)

target_include_directories(${map_io_library_name} SYSTEM PRIVATE
${GRAPHICSMAGICKCPP_INCLUDE_DIRS})
add_executable(${map_saver_cli_executable}
src/map_saver/main_cli.cpp)
target_include_directories(${map_saver_cli_executable}
PRIVATE
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<INSTALL_INTERFACE:include/${PROJECT_NAME}>")
target_link_libraries(${map_saver_cli_executable} PRIVATE
${library_name}
${map_io_library_name}
${nav_msgs_TARGETS}
${nav2_msgs_TARGETS}
nav2_util::nav2_util_core
rclcpp::rclcpp
rclcpp_lifecycle::rclcpp_lifecycle
)

target_link_libraries(${map_io_library_name}
${GRAPHICSMAGICKCPP_LIBRARIES}
yaml-cpp::yaml-cpp)
add_executable(${map_saver_server_executable}
src/map_saver/main_server.cpp)
target_include_directories(${map_saver_server_executable}
PRIVATE
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<INSTALL_INTERFACE:include/${PROJECT_NAME}>")
target_link_libraries(${map_saver_server_executable} PRIVATE
${library_name}
${map_io_library_name}
${nav_msgs_TARGETS}
${nav2_msgs_TARGETS}
nav2_util::nav2_util_core
rclcpp::rclcpp
rclcpp_lifecycle::rclcpp_lifecycle
)

if(WIN32)
target_compile_definitions(${map_io_library_name} PRIVATE
YAML_CPP_DLL)
endif()
add_executable(${costmap_filter_info_server_executable}
src/costmap_filter_info/main.cpp)
target_include_directories(${costmap_filter_info_server_executable}
PRIVATE
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<INSTALL_INTERFACE:include/${PROJECT_NAME}>")
target_link_libraries(${costmap_filter_info_server_executable} PRIVATE
${library_name}
${map_io_library_name}
${nav_msgs_TARGETS}
${nav2_msgs_TARGETS}
nav2_util::nav2_util_core
rclcpp::rclcpp
rclcpp_lifecycle::rclcpp_lifecycle
)

rclcpp_components_register_nodes(${library_name} "nav2_map_server::CostmapFilterInfoServer")
rclcpp_components_register_nodes(${library_name} "nav2_map_server::MapSaver")
rclcpp_components_register_nodes(${library_name} "nav2_map_server::MapServer")

install(TARGETS
${library_name} ${map_io_library_name}
EXPORT ${library_name}
ARCHIVE DESTINATION lib
LIBRARY DESTINATION lib
RUNTIME DESTINATION bin)
Expand All @@ -143,7 +151,7 @@ install(TARGETS
RUNTIME DESTINATION lib/${PROJECT_NAME})

install(DIRECTORY include/
DESTINATION include/)
DESTINATION include/${PROJECT_NAME})

install(DIRECTORY launch DESTINATION share/${PROJECT_NAME})

Expand All @@ -154,6 +162,7 @@ if(BUILD_TESTING)
ament_lint_auto_find_test_dependencies()

find_package(ament_cmake_gtest REQUIRED)
ament_find_gtest()
add_subdirectory(test)
endif()

Expand All @@ -162,5 +171,7 @@ ament_export_libraries(
${library_name}
${map_io_library_name}
)
ament_export_dependencies(${map_io_dependencies} ${map_server_dependencies} yaml-cpp)
ament_export_dependencies(nav_msgs nav2_msgs nav2_util rclcpp rclcpp_lifecycle)
SteveMacenski marked this conversation as resolved.
Show resolved Hide resolved
ament_export_targets(${library_name})

ament_package()
1 change: 1 addition & 0 deletions nav2_map_server/include/nav2_map_server/costmap_filter_info_server.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <memory>

#include "rclcpp/rclcpp.hpp"
#include "rclcpp_lifecycle/state.hpp"
#include "nav2_util/lifecycle_node.hpp"
#include "nav2_msgs/msg/costmap_filter_info.hpp"

Expand Down
3 changes: 2 additions & 1 deletion nav2_map_server/include/nav2_map_server/map_saver.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@
#ifndef NAV2_MAP_SERVER__MAP_SAVER_HPP_
#define NAV2_MAP_SERVER__MAP_SAVER_HPP_

#include <string>
#include <memory>
#include <string>

#include "rclcpp/rclcpp.hpp"
#include "rclcpp_lifecycle/state.hpp"
#include "nav2_util/lifecycle_node.hpp"
#include "nav2_msgs/srv/save_map.hpp"

Expand Down
6 changes: 3 additions & 3 deletions nav2_map_server/include/nav2_map_server/map_server.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@
#ifndef NAV2_MAP_SERVER__MAP_SERVER_HPP_
#define NAV2_MAP_SERVER__MAP_SERVER_HPP_

#include <string>
#include <memory>
#include <functional>
#include <string>

#include "rclcpp/rclcpp.hpp"
#include "nav2_util/lifecycle_node.hpp"
#include "nav_msgs/msg/occupancy_grid.hpp"
#include "nav_msgs/srv/get_map.hpp"
#include "nav2_msgs/srv/load_map.hpp"
#include "rclcpp/rclcpp.hpp"
#include "rclcpp_lifecycle/state.hpp"

namespace nav2_map_server
{
Expand Down
1 change: 1 addition & 0 deletions nav2_map_server/package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
<depend>nav2_msgs</depend>
<depend>nav2_util</depend>
<depend>graphicsmagick</depend>
<depend>lifecycle_msgs</depend>

<test_depend>ament_lint_common</test_depend>
<test_depend>ament_lint_auto</test_depend>
Expand Down
1 change: 1 addition & 0 deletions nav2_map_server/src/map_io.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include "nav2_util/geometry_utils.hpp"

#include "yaml-cpp/yaml.h"

#include "tf2/LinearMath/Matrix3x3.h"
#include "tf2/LinearMath/Quaternion.h"
#include "nav2_util/occ_grid_values.hpp"
Expand Down
20 changes: 14 additions & 6 deletions nav2_map_server/test/component/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
include_directories(${PROJECT_SOURCE_DIR}/test)

# map_server component test
ament_add_gtest_executable(test_map_server_node
test_map_server_node.cpp
${PROJECT_SOURCE_DIR}/test/test_constants.cpp
)
ament_target_dependencies(test_map_server_node rclcpp nav_msgs)
target_include_directories(test_map_server_node
PRIVATE
"$<BUILD_INTERFACE:${CMAKE_SOURCE_DIR}/test>")
target_link_libraries(test_map_server_node
rclcpp::rclcpp
${nav_msgs_TARGETS}
${library_name}
)

Expand All @@ -25,17 +27,23 @@ ament_add_gtest_executable(test_map_saver_node
test_map_saver_node.cpp
${PROJECT_SOURCE_DIR}/test/test_constants.cpp
)

ament_target_dependencies(test_map_saver_node rclcpp nav_msgs)
target_include_directories(test_map_saver_node
PRIVATE
"$<BUILD_INTERFACE:${CMAKE_SOURCE_DIR}/test>")
target_link_libraries(test_map_saver_node
rclcpp::rclcpp
${nav_msgs_TARGETS}
${library_name}
${map_io_library_name}
)

add_executable(test_map_saver_publisher
test_map_saver_publisher.cpp
${PROJECT_SOURCE_DIR}/test/test_constants.cpp
)

target_include_directories(test_map_saver_publisher
PRIVATE
"$<BUILD_INTERFACE:${CMAKE_SOURCE_DIR}/test>")
target_link_libraries(test_map_saver_publisher
${map_io_library_name}
)
Expand Down
7 changes: 2 additions & 5 deletions nav2_map_server/test/map_saver_cli/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
include_directories(${PROJECT_SOURCE_DIR}/test)

# map_saver CLI
ament_add_gtest(test_map_saver_cli
test_map_saver_cli.cpp
${PROJECT_SOURCE_DIR}/test/test_constants.cpp
)

ament_target_dependencies(test_map_saver_cli rclcpp nav_msgs)
target_link_libraries(test_map_saver_cli
${dependencies}
rclcpp::rclcpp
${nav_msgs_TARGETS}
)
6 changes: 3 additions & 3 deletions nav2_map_server/test/map_saver_cli/test_map_saver_cli.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ TEST(MapSaverCLI, CLITest)
std::string(
"ros2 run nav2_map_server map_saver_cli "
"-t map_failure --occ 100 --free 2 --mode trinary --fmt png -f ") + file_path +
std::string("--ros-args __node:=map_saver_test_node");
std::string(" --ros-args --remap __node:=map_saver_test_node");
return_code = system(command.c_str());
EXPECT_EQ(return_code, 65280);
EXPECT_EQ(return_code, 256);
Copy link
Member

@SteveMacenski SteveMacenski Jul 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No note assuming CI test is happy, but noting this for myself to check. Map server package seems to have failed its CI tests, but still running so I can't see where yet

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it looks like there are bunch of failing external symbols in the tests. That means that something is wrong with this PR. I'll take a closer look and see if I can reproduce locally.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I can't reproduce, and looking at the code again there is nothing obviously wrong jumping out at me.

I'm not that familiar with CircleCI, but in the job below it never seems to run the "Setup Workspace" or "Build Workspace" steps that are listed in https://github.com/ros-navigation/navigation2/blob/main/.circleci/config.yml . Looking at those logs would be helpful for me to try and reproduce.

@SteveMacenski Do you know why those steps aren't present in that run, and/or how I might get access to those logs?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The release_build job below does the building of the main software stack (system_build does the system tests separately). The job that's failing (release_test) is only doing the colcon test bits on those built and cached artifacts from the builds.

You can find the build artifacts here: https://app.circleci.com/pipelines/github/ros-navigation/navigation2/12291/workflows/ec12a613-f542-4479-9c2b-640744e3fbaf/jobs/37306/artifacts
You can find the test artifacts here: https://app.circleci.com/pipelines/github/ros-navigation/navigation2/12291/workflows/ec12a613-f542-4479-9c2b-640744e3fbaf/jobs/37308/artifacts

I'm also retriggering the build from the start to validate its not a fluke, but it does seem like its a problem

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something else worth trying is to update v26 to v27 in the 3x places in the circle config file to break the cache which may contain the previous works and rebuild from scratch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks for pointing out where it was built. Following that, I was able to see that the CircleCI job uses a number of mixins, and compiling locally with those mixins I can reproduce. Now to see what is going on here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All right, I found the issue. 66e300b should fix it.


rclcpp::Rate(0.25).sleep();

Expand Down Expand Up @@ -140,7 +140,7 @@ TEST(MapSaverCLI, CLITest)

command =
std::string(
"ros2 run nav2_map_server map_saver_cli --ros-args -r __node:=map_saver_test_node");
"ros2 run nav2_map_server map_saver_cli --ros-args --remap __node:=map_saver_test_node");
return_code = system(command.c_str());
EXPECT_EQ(return_code, 0);
}
Loading
Loading