Skip to content

Remove ament_target_dependencies to match ROS 2 Rolling #32

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

Merged
sgillen merged 3 commits into from
Feb 26, 2026
Merged

Remove ament_target_dependencies to match ROS 2 Rolling #32

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0c10299
Add ament_cmake as package in CMakelist
bmchalenv Jan 28, 2026
f9046ae
Add more depends
bmchalenv Feb 26, 2026
834eb64
Test new target link libraries
bmchalenv Feb 26, 2026
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
59 changes: 34 additions & 25 deletions greenwave_monitor/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ target_include_directories(greenwave_diagnostics INTERFACE
$<INSTALL_INTERFACE:include>)

add_executable(greenwave_monitor src/greenwave_monitor.cpp src/greenwave_monitor_main.cpp)
ament_target_dependencies(greenwave_monitor
rclcpp
std_msgs
diagnostic_msgs
greenwave_monitor_interfaces
target_link_libraries(greenwave_monitor
greenwave_diagnostics
${rclcpp_TARGETS}
${std_msgs_TARGETS}
${diagnostic_msgs_TARGETS}
${greenwave_monitor_interfaces_TARGETS}
Comment thread
sgillen marked this conversation as resolved.
)
Comment on lines +33 to 39
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Feb 26, 2026

Choose a reason for hiding this comment

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

Changed from ament_target_dependencies() to target_link_libraries() with ${package_TARGETS} variables. This deviates from standard ROS 2 practice. ament_target_dependencies() is the recommended approach as it properly handles include directories, link libraries, and compile definitions across different ROS 2 versions.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Collaborator

@sgillen sgillen Feb 26, 2026

Choose a reason for hiding this comment

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

ament_target_dependencies is deprecated, so we need to switch.

target_link_libraries(greenwave_monitor greenwave_diagnostics)

target_include_directories(greenwave_monitor PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
Expand All @@ -50,8 +50,13 @@ install(TARGETS greenwave_monitor
add_executable(minimal_publisher_node
src/minimal_publisher_node.cpp
src/minimal_publisher_main.cpp)
ament_target_dependencies(minimal_publisher_node rclcpp std_msgs sensor_msgs diagnostic_msgs)
target_link_libraries(minimal_publisher_node greenwave_diagnostics)
target_link_libraries(minimal_publisher_node
greenwave_diagnostics
${rclcpp_TARGETS}
${std_msgs_TARGETS}
${sensor_msgs_TARGETS}
${diagnostic_msgs_TARGETS}
)
target_include_directories(minimal_publisher_node PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
$<INSTALL_INTERFACE:include>)
Expand All @@ -62,8 +67,12 @@ install(TARGETS minimal_publisher_node
add_executable(example_greenwave_publisher_node
src/example_greenwave_publisher_node.cpp
src/example_greenwave_publisher_main.cpp)
ament_target_dependencies(example_greenwave_publisher_node rclcpp sensor_msgs diagnostic_msgs)
target_link_libraries(example_greenwave_publisher_node greenwave_diagnostics)
target_link_libraries(example_greenwave_publisher_node
greenwave_diagnostics
${rclcpp_TARGETS}
${sensor_msgs_TARGETS}
${diagnostic_msgs_TARGETS}
)
target_include_directories(example_greenwave_publisher_node PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
$<INSTALL_INTERFACE:include>)
Expand Down Expand Up @@ -133,12 +142,12 @@ if(BUILD_TESTING)
TIMEOUT 60
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
)
ament_target_dependencies(test_greenwave_diagnostics
rclcpp
std_msgs
diagnostic_msgs
target_link_libraries(test_greenwave_diagnostics
greenwave_diagnostics
${rclcpp_TARGETS}
${std_msgs_TARGETS}
${diagnostic_msgs_TARGETS}
)
target_link_libraries(test_greenwave_diagnostics greenwave_diagnostics)
target_include_directories(test_greenwave_diagnostics PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
$<INSTALL_INTERFACE:include>
Expand All @@ -151,12 +160,12 @@ if(BUILD_TESTING)
TIMEOUT 60
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
)
ament_target_dependencies(test_minimal_publisher
rclcpp
sensor_msgs
diagnostic_msgs
target_link_libraries(test_minimal_publisher
greenwave_diagnostics
${rclcpp_TARGETS}
${sensor_msgs_TARGETS}
${diagnostic_msgs_TARGETS}
)
target_link_libraries(test_minimal_publisher greenwave_diagnostics)
target_include_directories(test_minimal_publisher PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
$<INSTALL_INTERFACE:include>
Expand All @@ -169,12 +178,12 @@ if(BUILD_TESTING)
TIMEOUT 60
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
)
ament_target_dependencies(test_example_greenwave_publisher
rclcpp
sensor_msgs
diagnostic_msgs
target_link_libraries(test_example_greenwave_publisher
greenwave_diagnostics
${rclcpp_TARGETS}
${sensor_msgs_TARGETS}
${diagnostic_msgs_TARGETS}
)
target_link_libraries(test_example_greenwave_publisher greenwave_diagnostics)
target_include_directories(test_example_greenwave_publisher PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
$<INSTALL_INTERFACE:include>
Expand Down