Skip to content

Conversation

@pperycz
Copy link
Contributor

@pperycz pperycz commented Nov 13, 2025

Description

Introduce ROS2 Jazzy Jalisco support for Stationary Robot Vision & Control:

Any Newly Introduced Dependencies

...

How Has This Been Tested?

  • ran unit tests locallvia with colcon test for all packaged. Fuzz test/unit tests run successfully for rvc_rotated_object_detection, in the other packages we either have no tests or the target is failing due to linting/code-style
Summary: 25 packages finished [1min 21s]
Starting >>> rvc_rotated_object_detection
============================= test session starts ==============================
platform linux -- Python 3.12.3, pytest-7.4.4, pluggy-1.4.0
cachedir: /rvc/build/rvc_rotated_object_detection/.pytest_cache
rootdir: /rvc/src/rvc_vision/rvc_rotated_object_detection
plugins: ament-flake8-0.17.3, launch-testing-3.4.7, ament-pep257-0.17.3, launch-testing-ros-0.26.9, ament-lint-0.17.3, ament-xmllint-0.17.3, ament-copyright-0.17.3, ament-black-0.2.6, ament-mypy-0.17.3, colcon-core-0.20.1, typeguard-4.1.5
collected 2 items



test/test_detector.py ..                                                 [100%]

---- generated xml file: /rvc/build/rvc_rotated_object_detection/pytest.xml ----
============================== 2 passed in 2.59s ===============================

ran ./docker_build_rvc_img.sh and ./docker_build_rvc_img.sh successfully

  • ran the following make target:
docker run --rm -v $(PWD)/src:/rvc/src $(DEV_IMAGE):$(DEV_TAG) bash -c ' \
		set -e && \
		sudo apt-get update && sudo apt-get install -y --no-install-recommends \
		ros-jazzy-hardware-interface \
		ros-jazzy-ur-robot-driver \
		ros-jazzy-ur-calibration \
		&& sudo rm -rf /var/lib/apt/lists/* \
		&& cd /rvc \
		&& . /opt/ros/$${ROS_DISTRO}/setup.sh \
		&& colcon build --symlink-install --cmake-args -DBUILD_TESTING=OFF -DCMAKE_BUILD_TYPE=Release'

Checklist:

  • I agree to use the APACHE-2.0 license for my code changes.
  • I have not introduced any 3rd party components incompatible with APACHE-2.0.
  • I have not included any company confidential information, trade secret, password or security token.
  • I have performed a self-review of my code.

@jb-balaji jb-balaji requested a review from Copilot November 17, 2025 10:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the Robot Vision & Control (RVC) system from ROS2 Humble to ROS2 Jazzy Jalisco, updating the base OS from Ubuntu 22.04 to Ubuntu 24.04 (Noble) with Python 3.12. The migration includes comprehensive updates to Debian packaging configurations, refactoring C++ code to use Jazzy-compatible MoveIt2 APIs (.h to .hpp header transitions), and adapting to breaking API changes in the servo control and hardware interface libraries.

Key changes:

  • Migration from ROS2 Humble to Jazzy across all packages
  • Updated MoveIt2 Servo API usage with new parameter listener pattern and command type configuration
  • Header file updates from .h to .hpp extensions for modern C++ standards
  • Replaced deprecated APIs with standard C++ alternatives

Reviewed Changes

Copilot reviewed 86 out of 86 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Debian control/rules/changelog files Updated all package metadata from humble to jazzy, jammy to noble distribution
.devcontainer/Dockerfile Updated base image to Ubuntu 24.04, ROS jazzy packages, Python 3.12, and user UID
moveit2_servo_motion_controller Refactored to use new Servo ParamListener API and retrieve frames from IK solver
robotiq_driver Removed deprecated get_name() override and visibility_control.h include
rvc_pose_detector Updated tf2_sensor_msgs header to .hpp extension
rvc_object_detection_engine Added OpenVINO package dependency and updated cv_bridge header
trac_ik plugin headers Updated MoveIt headers from .h to .hpp extensions
rvc_panel_rviz2_plugin Replaced rmw_qos_profile_services_default with rclcpp::ServicesQoS()
rotated_object_detection_parameters.py Auto-generated parameter code with new helper methods and renamed loop variables

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bool Moveit2ServoMotionController::init ( rclcpp::Node::SharedPtr node )
{
auto res = RVCMotionControllerInterface::init(node);
res = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

if super fails why force to true?

Choose a reason for hiding this comment

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

I remember seeing this is various examples and for some reason at the time it seemed like a good thing. But you're absolutely right it completely ignores the previous init. Will revert

auto servo_params = servo_param_listener->get_params();

// Load the planning scene monitor
planning_scene_monitor::PlanningSceneMonitorPtr planning_scene_monitor =
Copy link
Contributor

Choose a reason for hiding this comment

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

does this still works in jazzy? i have a reference of a planning_scene_monitor on the stack in the hpp, and here " planning_scene_monitor = moveit_servo::createPlanningSceneMonitor(node, servo_param_listener->get_params());"

Choose a reason for hiding this comment

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

I tried to look into this a bit more and I'm inclined to say it should work. This is the constructor I expect us to be using: https://github.com/moveit/moveit2/blob/ca2bb1ba4581b7213d68094458f2649de17d237e/moveit_ros/planning/planning_scene_monitor/src/planning_scene_monitor.cpp#L78C1-L82C2.
From what I can see createPlanningSceneMonitor and the code will make use of the same c-tor (https://github.com/moveit/moveit2/blob/ca2bb1ba4581b7213d68094458f2649de17d237e/moveit_ros/moveit_servo/src/utils/common.cpp#L483-L490).

The difference I can see is with createPlanningSceneMonitor we're adding some extra set-up to the planning_scene_monitor that we're not doing in code.

Choose a reason for hiding this comment

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

I'm also now looking through an older change of yours where you're using createPlanningSceneMonitor. Is this what we should be using?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants