feat: use exported targets #69
Conversation
wep21
commented
Jun 25, 2022
wep21
commented
- revert the changes for ament cmake auto and use exported targets instead.
…)" This reverts commit bea4f9b.
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
|
@clalancette @hidmic Could anyone review this PR? |
| INSTALL_TO_SHARE | ||
| launch | ||
| ) | ||
| ament_package() |
There was a problem hiding this comment.
We are missing the export of the dependencies. This should be something like:
ament_export_dependencies(laser_geometry message_filters rclcpp sensor_msgs tf2_ros)
There was a problem hiding this comment.
@clalancette I added missing ament_export_dependencies in be9226b.
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
CMakeLists.txt
Outdated
| # use exported target after https://github.com/ros2/geometry2/pull/536 is merged | ||
| # tf2_sensor_msgs::tf2_sensor_msgs |
There was a problem hiding this comment.
So when I tried to build this, it failed with:
pc_to_laserscan_ws/src/pointcloud_to_laserscan/src/pointcloud_to_laserscan_node.cpp:52:10: fatal error: tf2_sensor_msgs/tf2_sensor_msgs.h: No such file or directory
52 | #include "tf2_sensor_msgs/tf2_sensor_msgs.h"
A workaround would be to add ${tf2_sensor_msgs_INCLUDE_DIRS} to the target_include_directories. We can either do that, or we can wait for ros2/geometry2#536 to be merged. I leave the decision to you, either is fine with me.
There was a problem hiding this comment.
@clalancette I decided to add workaround until ros2/geometry2#536 is merged. f4e8781
Thank you for letting me know.
There was a problem hiding this comment.
@clalancette friendly ping
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
|
@clalancette friendly ping |
1 similar comment
|
@clalancette friendly ping |
Now that we merged in the geometry2 PR, we can use the target as appropriate. Also fix a warning when building against rolling. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
There was a problem hiding this comment.
@wep21 I ended up going back to the tf2_sensor_msg::tf2_sensor_msgs target, as we've merged in the geometry2 PR in the meantime. I also fixes a warning when building. This now looks good to me, thank you!
|
@ros-pull-request-builder retest this please |
