Add dependency for moveit_ros_occupancy_map_monitor - #153
Merged
Conversation
Add dependency for moveit_ros_occupancy_map_monitor
nbbrooks
approved these changes
Aug 3, 2026
nbbrooks
left a comment
nbbrooks
left a comment
Contributor
There was a problem hiding this comment.
Needed to fulfill transitive dependency via moveit_ros_planning
nbbrooks
added a commit
to PickNikRobotics/moveit_visual_tools
that referenced
this pull request
Aug 3, 2026
moveit_visual_tools #includes these packages and uses their API directly,
but declares none of them in package.xml or CMakeLists.txt. All three
resolve today only because moveit_core / moveit_ros_planning happen to
pull them in transitively.
geometric_shapes src/moveit_visual_tools.cpp:67-68 includes
solid_primitive_dims.h + shape_operations.h and calls
shapes::createMeshFromResource /
shapes::constructMsgFromShape at L861-863.
Reported as moveit#155.
moveit_msgs include/moveit_visual_tools/moveit_visual_tools.h:48-52
includes five moveit_msgs headers, and the types appear
in public method signatures (publishGrasps,
publishTrajectoryPath, processCollisionObjectMsg, ...).
ament_index_cpp src/moveit_visual_tools_demo.cpp:36-41.
Each is scoped to what it is actually used for rather than added uniformly:
- moveit_msgs is PUBLIC and exported via THIS_PACKAGE_INCLUDE_DEPENDS: it
appears in installed public headers, so downstream consumers of
find_package(moveit_visual_tools) need it resolved.
- geometric_shapes is PRIVATE and deliberately NOT exported. It appears in
no public header -- include/ contains zero references to it or to
shapes::. This is a SHARED library, so a PRIVATE link is not recorded in
the exported target's interface at all (no $<LINK_ONLY:> entry, which
would appear for a STATIC library), and downstream consumers correctly
do not need geometric_shapes. Linking the library target therefore uses
the keyword signature; the demo target keeps the plain form, which is
legal because the plain/keyword rule is per-target.
- ament_index_cpp is linked to the demo executable only and kept out of
THIS_PACKAGE_INCLUDE_DEPENDS; it is not part of the library's interface.
All three still get <depend> in package.xml. A find_package(... REQUIRED)
without a matching <depend> is a build-time dependency rosdep will never
install, so the two halves are not substitutes for each other.
This matters more on this branch than on ros2. The preceding commit replaces
ament_target_dependencies with target_link_libraries, and
ament_target_dependencies was flattening the recursive dependency tree onto
every target -- which is precisely what was papering over these three gaps.
Declaring them is a prerequisite for that migration, not a metadata cleanup.
moveit_ros_occupancy_map_monitor is deliberately not among the three. moveit#153
added find_package(moveit_ros_occupancy_map_monitor REQUIRED) on the base
branch, but nothing here includes an occupancy_map_monitor header directly;
it is reachable only via
moveit/planning_scene_monitor/planning_scene_monitor.hpp:53. Verified against
the built moveit_ros_planning export set: moveit_planning_scene_monitor
carries moveit_ros_occupancy_map_monitor::moveit_ros_occupancy_map_monitor in
its INTERFACE_LINK_LIBRARIES and moveit_ros_planning::moveit_ros_planning
re-exports it, so planning_scene_monitor.hpp compiles under
target_link_libraries with no explicit find_package. That line is left as-is
here; note it has no matching <depend> in package.xml, which is the
find_package/<depend> mismatch this commit avoids for the other three.
nbbrooks
added a commit
that referenced
this pull request
Aug 3, 2026
…t_index_cpp header rename (#156) * CMake: migrate ament_target_dependencies → target_link_libraries + add interactive_markers Rolling removed ament_target_dependencies. Migrate to target_link_libraries with imported targets across the library and demo call sites. Add explicit find_package(interactive_markers) + <depend> + link — no longer transitive through rviz_visual_tools after rviz_visual_tools#277. * demo: guard ament_index_cpp header rename with AMENT_INDEX_CPP_VERSION_GTE ament_index_cpp 1.14+ removed <get_package_share_directory.hpp>. Use the new get_package_share_path (returns std::filesystem::path) when available, fall back to the old API otherwise. Same 3-tier guard pattern moveit2#3705 adopted. * Declare the three dependencies used directly but never declared moveit_visual_tools #includes these packages and uses their API directly, but declares none of them in package.xml or CMakeLists.txt. All three resolve today only because moveit_core / moveit_ros_planning happen to pull them in transitively. geometric_shapes src/moveit_visual_tools.cpp:67-68 includes solid_primitive_dims.h + shape_operations.h and calls shapes::createMeshFromResource / shapes::constructMsgFromShape at L861-863. Reported as #155. moveit_msgs include/moveit_visual_tools/moveit_visual_tools.h:48-52 includes five moveit_msgs headers, and the types appear in public method signatures (publishGrasps, publishTrajectoryPath, processCollisionObjectMsg, ...). ament_index_cpp src/moveit_visual_tools_demo.cpp:36-41. Each is scoped to what it is actually used for rather than added uniformly: - moveit_msgs is PUBLIC and exported via THIS_PACKAGE_INCLUDE_DEPENDS: it appears in installed public headers, so downstream consumers of find_package(moveit_visual_tools) need it resolved. - geometric_shapes is PRIVATE and deliberately NOT exported. It appears in no public header -- include/ contains zero references to it or to shapes::. This is a SHARED library, so a PRIVATE link is not recorded in the exported target's interface at all (no $<LINK_ONLY:> entry, which would appear for a STATIC library), and downstream consumers correctly do not need geometric_shapes. Linking the library target therefore uses the keyword signature; the demo target keeps the plain form, which is legal because the plain/keyword rule is per-target. - ament_index_cpp is linked to the demo executable only and kept out of THIS_PACKAGE_INCLUDE_DEPENDS; it is not part of the library's interface. All three still get <depend> in package.xml. A find_package(... REQUIRED) without a matching <depend> is a build-time dependency rosdep will never install, so the two halves are not substitutes for each other. This matters more on this branch than on ros2. The preceding commit replaces ament_target_dependencies with target_link_libraries, and ament_target_dependencies was flattening the recursive dependency tree onto every target -- which is precisely what was papering over these three gaps. Declaring them is a prerequisite for that migration, not a metadata cleanup. moveit_ros_occupancy_map_monitor is deliberately not among the three. #153 added find_package(moveit_ros_occupancy_map_monitor REQUIRED) on the base branch, but nothing here includes an occupancy_map_monitor header directly; it is reachable only via moveit/planning_scene_monitor/planning_scene_monitor.hpp:53. Verified against the built moveit_ros_planning export set: moveit_planning_scene_monitor carries moveit_ros_occupancy_map_monitor::moveit_ros_occupancy_map_monitor in its INTERFACE_LINK_LIBRARIES and moveit_ros_planning::moveit_ros_planning re-exports it, so planning_scene_monitor.hpp compiles under target_link_libraries with no explicit find_package. That line is left as-is here; note it has no matching <depend> in package.xml, which is the find_package/<depend> mismatch this commit avoids for the other three. * CMake: link the specific planning_scene_monitor target, not the umbrella moveit_ros_planning::moveit_ros_planning is an INTERFACE target added by moveit2#3726 (merged 2026-06-16). It exists only on moveit2 main -- every released MoveIt predates it, so linking it fails at configure time on all three distros this package ships to: CMake Error: Target "moveit_visual_tools" links to moveit_ros_planning::moveit_ros_planning but the target was not found. Verified against the released debs; the specific target is present everywhere, the umbrella nowhere: distro moveit_ros_planning ::moveit_planning_scene_monitor ::moveit_ros_planning humble 2.5.9 present absent jazzy 2.12.4 present absent kilted 2.14.3 present absent moveit2 main - present present It is also the accurate dependency rather than merely the portable one: planning_scene_monitor.hpp is the only moveit_ros_planning header this package includes, so the umbrella would over-link all 14 of that package's libraries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add dependency for moveit_ros_occupancy_map_monitor