Skip to content

moveit_visual_tools appears to use geometric_shapes directly without declaring it in package.xml #155

Description

@Plumezz

Summary

While reviewing ROS 2 package dependency metadata, I noticed that moveit_visual_tools appears to use geometric_shapes directly, but geometric_shapes does not seem to be declared in moveit_visual_tools/package.xml.

This may make the package rely on moveit_core or moveit_ros_planning to provide geometric_shapes transitively.

Evidence

Direct usage

moveit_visual_tools directly includes geometric_shapes in C++ source or public headers:

  • src/moveit_visual_tools.cpp:67: #include <geometric_shapes/solid_primitive_dims.h>
  • src/moveit_visual_tools.cpp:68: #include <geometric_shapes/shape_operations.h>

The package uses dependency-owned types or APIs from those headers:

  • src/moveit_visual_tools.cpp:861: shapes::Shape* mesh = shapes::createMeshFromResource(mesh_path); // make sure its prepended by file://
  • src/moveit_visual_tools.cpp:862: shapes::ShapeMsg shape_msg; // this is a boost::variant type from shape_messages.h
  • src/moveit_visual_tools.cpp:863: if (!mesh || !shapes::constructMsgFromShape(mesh, shape_msg))

Current package.xml

I could not find a direct declaration of geometric_shapes in moveit_visual_tools/package.xml.

The package currently declares the following intermediate dependency/dependencies, which make geometric_shapes reachable transitively:

<depend>moveit_core</depend>
<depend>moveit_ros_planning</depend>

Observed during build/test

Build/test file-access tracing observed some events associated with geometric_shapes during build. Representative accessed paths include:

  • geometric_shapes/share/ament_index/resource_index/package_run_dependencies/geometric_shapes
  • geometric_shapes/share/geometric_shapes/cmake/geometric_shapesConfig-version.cmake
  • geometric_shapes/share/geometric_shapes/cmake/geometric_shapesConfig.cmake
  • geometric_shapes/share/geometric_shapes/cmake/ament_cmake_export_targets-extras.cmake

Transitive path

moveit_visual_tools -> moveit_core -> geometric_shapes
moveit_visual_tools -> moveit_ros_planning -> moveit_core -> geometric_shapes
moveit_visual_tools -> moveit_ros_planning -> moveit_ros_occupancy_map_monitor -> geometric_shapes
moveit_visual_tools -> moveit_ros_planning -> moveit_ros_occupancy_map_monitor -> moveit_core -> geometric_shapes

Suggested fix

If this direct usage is intentional, would it make sense to add:

<depend>geometric_shapes</depend>

to moveit_visual_tools/package.xml? The corresponding find_package(geometric_shapes REQUIRED) and CMake target dependency should also be added for the target(s) that compile these files, where required by the package's CMake structure.

Notes

This issue does not claim that the package currently fails to build. The concern is that the package directly uses geometric_shapes while relying on a transitive dependency path to make it available. The observation is based on package metadata, concrete source-level use, recursive dependency closure analysis, and build/test file-access tracing.

Could you please confirm whether this dependency is intentionally left implicit through the transitive dependency path shown above, or whether adding an explicit dependency would be appropriate?

I would be happy to open a small PR adding the dependency if that matches the intended package metadata.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions