Skip to content

Fix: Silence tl_expected and rmw_qos_profile_t deprecation warnings - #862

Merged
fdavulcu merged 1 commit into
v10.0from
fix/tl-expected-and-qos-warnings
Aug 26, 2026
Merged

Fix: Silence tl_expected and rmw_qos_profile_t deprecation warnings#862
fdavulcu merged 1 commit into
v10.0from
fix/tl-expected-and-qos-warnings

Conversation

@fdavulcu

@fdavulcu fdavulcu commented Aug 19, 2026

Copy link
Copy Markdown

[written by AI]

A fresh moveit_pro build prints deprecation warnings from several sources in this workspace:

  • example_behaviors includes the deprecated tl_expected/expected.hpp wrapper, which ROS Jazzy's tl_expected 1.3.1 flags with a #pragma message on every include. Switched to <tl/expected.hpp> from libexpected-dev, the replacement the deprecation notice recommends. The header ships in the MoveIt Pro runtime image, so this builds against current releases. The package declares the dependency it now consumes directly: <depend>libexpected-dev</depend>, find_package(tl-expected REQUIRED), an explicit tl::expected link, and the ament_export_dependencies entry (the system config exports only the imported target, so ament_target_dependencies cannot carry it).
  • moveit_studio_kinova_pstop_manager uses the rmw_qos_profile_t overloads of create_service/create_client, deprecated in favor of rclcpp::QoS. Switched to rclcpp::ServicesQoS(), which wraps the same profile.
  • The fanuc and ros2_kortex submodules were pinned before their own warning fixes merged. Bumped both to the merge commits of Bump cmake_minimum_required to 3.16 to silence CMake deprecation warnings fanuc#1 (cmake_minimum_required 3.3 to 3.16 across 21 packages) and Fix: Set CMP0135 to NEW to silence FetchContent timestamp warnings ros2_kortex#6 (a CMP0135 policy guard so FetchContent stops warning about archive extraction timestamps). Both are fast-forwards carrying nothing but those fixes.
  • moveit_studio_ur_pstop_manager had the same rmw_qos_profile_t problem. It was fixed upstream in Use rclcpp::QoS overloads for service and client creation moveit_studio_ur_pstop_manager#4, but this workspace could not reach it, because it pins phoebe_ws and phoebe_ws pins that repo. fix(ur_pstop_manager): point submodule at the rclcpp::QoS deprecation fix (no-dups) phoebe_ws#41 moved the inner pin, and the phoebe_ws submodule here moves to that merge. It also picks up three commits this workspace was already behind on, namely a licensing sweep, removal of a dead CLIPSeg subtree from phoebe_sim, and deletion of an unused ewellix_description launch file.

Release notes

None

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1ce90e38-8ee1-4d35-9363-a168e199c818

📥 Commits

Reviewing files that changed from the base of the PR and between e1bdcd5 and d123213.

📒 Files selected for processing (1)
  • src/external_dependencies/phoebe_ws

Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 8 reviews per hour.


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved compatibility and build reliability for example behaviors using updated expected-value support.
    • Improved service communication reliability for controller switching and fault-reset operations through standardized quality-of-service settings.
    • Added required support for consistent configuration and builds across supported environments.
  • Maintenance

    • Updated supported hardware integration components to improve compatibility and ongoing maintenance.

Walkthrough

The pull request updates expected-library includes and package dependencies, changes protective-stop service QoS construction, and updates three external dependency references.

Changes

Expected header updates

Layer / File(s) Summary
Update expected-library includes
src/example_behaviors/include/..., src/example_behaviors/src/..., src/example_behaviors/CMakeLists.txt, src/example_behaviors/package.xml
The example behavior files use tl/expected.hpp. The package discovers, links, exports, and declares the expected-library dependency.

Protective-stop service QoS

Layer / File(s) Summary
Update protective-stop service QoS
src/moveit_pro_kinova_configs/moveit_studio_kinova_pstop_manager/src/protective_stop_manager_node.cpp
The recovery service, controller-switch client, callback-group wiring, and fault-reset client use rclcpp::ServicesQoS().

External dependency references

Layer / File(s) Summary
Update external dependency references
src/external_dependencies/fanuc, src/external_dependencies/ros2_kortex, src/external_dependencies/phoebe_ws
The fanuc, ros2_kortex, and phoebe_ws submodule pointers reference new commits.

Suggested reviewers: bkanator


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore (reviewers only)

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Human Review Check ❌ Error The PR updates phoebe_ws from f5014310... to c73ef077.... The referenced upstream range removes src/ewellix_description/launch/description.launch.py (132 lines). This is a launch-script change… This PR requires review by a requested human reviewer. After review, a non-author requested reviewer should override this pre-merge check.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description clearly explains the expected-header updates, QoS API changes, dependency declarations, and submodule updates shown in the changeset.
Full details: Human Review Check

Explanation

The PR updates phoebe_ws from f5014310... to c73ef077.... The referenced upstream range removes src/ewellix_description/launch/description.launch.py (132 lines). This is a launch-script change caused by the submodule update, so the PR matches an explicit failure condition. The other direct edits are small header, dependency, and QoS updates.

  • Fix all pre-merge checks with AI

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 19, 2026
@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@fdavulcu
fdavulcu marked this pull request as ready for review August 19, 2026 15:40
@fdavulcu
fdavulcu requested a review from a team August 19, 2026 15:40
coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 19, 2026
<depend>moveit_studio_vision_msgs</depend>
<depend>moveit_studio_vision</depend>
<depend>moveit_pro_behavior</depend>
<depend>libexpected-dev</depend>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In other places we use the tl_expected dependency, e.g: https://github.com/PickNikRobotics/moveit_pro/blob/v10.0/src/kinematics/path_ik/package.xml#L20

And include as tl_expected/expected.hpp, e.g: https://github.com/PickNikRobotics/moveit_pro/blob/e0dbb7aecf00ea93e48f629d4ae744f92955aaee/src/kinematics/path_ik/include/path_ik/cartesian_timing.hpp#L14

Assuming those don't throw a warning, I think we should do the same here for consistency

@fdavulcu fdavulcu Aug 24, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I am also changing them here: https://github.com/PickNikRobotics/moveit_pro/pull/21708 -- they do emit warnings while building the main repo too.

marioprats
marioprats previously approved these changes Aug 26, 2026
@fdavulcu
fdavulcu dismissed stale reviews from marioprats and coderabbitai[bot] via e1bdcd5 August 26, 2026 12:39
@fdavulcu
fdavulcu force-pushed the fix/tl-expected-and-qos-warnings branch from e3d9ad7 to e1bdcd5 Compare August 26, 2026 12:40
coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 26, 2026
Declare libexpected-dev in example_behaviors so the tl/expected.hpp include resolves through find_package(tl-expected) rather than the compiler's default include path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@fdavulcu
fdavulcu enabled auto-merge August 26, 2026 13:03
@fdavulcu
fdavulcu merged commit fcb2f0f into v10.0 Aug 26, 2026
14 checks passed
@fdavulcu
fdavulcu deleted the fix/tl-expected-and-qos-warnings branch August 26, 2026 13:08
@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

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.

2 participants