Skip to content

Add motion_primitives_from_trajectory_controller with trajectory approximation using PTP/LIN primitives#1858

Draft
mathias31415 wants to merge 144 commits intoros-controls:masterfrom
b-robotized-forks:moprim_from_traj_controller_with_base_controller
Draft

Add motion_primitives_from_trajectory_controller with trajectory approximation using PTP/LIN primitives#1858
mathias31415 wants to merge 144 commits intoros-controls:masterfrom
b-robotized-forks:moprim_from_traj_controller_with_base_controller

Conversation

@mathias31415
Copy link
Copy Markdown
Contributor

This PR adds the motion_primitives_from_trajectory_controller. To make reviewing easier, another PR #1857 was created first to move shared functionality into a base controller.

This PR replaces PR #1837.

This PR is marked as a draft because PR #1636 and PR #1857 must be merged first.

This PR complements motion_primitives_forward_controller PR #1636 .
Instead of sending hardcoded motion primitives to the motion_primitives_forward_controller, this controller (motion_primitives_from_trajectory_controller) approximates a JointTrajectory using either PTP or LIN motion primitives.

This allows, for example, collision-free paths planned with MoveIt to be executed as motion primitives. The approximation is performed using the Ramer-Douglas-Peucker (RDP) algorithm:

  • For PTP primitives in joint space
  • For LIN primitives in Cartesian space

mathias31415 and others added 30 commits April 8, 2025 14:35
…time --> added gitignore, to not push it to github --> remove it after everything is done
… the hardware interface can receive a new motion primitive — replaces the previous, more complex handling via execution_status
…ls according to other controllers in the repo
… rtw template to fit the controller implementation
…tation from rtw template to fit the controller implementation
@github-actions github-actions bot removed the stale label Feb 2, 2026
Copy link
Copy Markdown
Member

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

Thanks. But there are some build failures now:

From the docs

 /home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/userdoc.rst:11: WARNING: Duplicate explicit target name: "definition file located in the src folder".
/home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/userdoc.rst:11: WARNING: Duplicate explicit target name: "the test directory".
/home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/userdoc.rst:37: WARNING: Include file '/home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/test/motion_primitives_from_trajectory_controller_params.yaml' not found or reading it failed
/home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/README.md:96: WARNING: duplicate label doc/ros2_controllers/motion_primitives_controllers/userdoc:architecture overview, other instance in /home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/userdoc.rst
/home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/README.md:105: WARNING: duplicate label doc/ros2_controllers/motion_primitives_controllers/userdoc:demo-video with ur10e, other instance in /home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/userdoc.rst
/home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/README.md:109: WARNING: duplicate label doc/ros2_controllers/motion_primitives_controllers/userdoc:demo-video with kr3, other instance in /home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/userdoc.rst
/home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/userdoc.rst:16: WARNING: duplicate label doc/ros2_controllers/motion_primitives_controllers/userdoc:motion_primitives_forward_controller, other instance in /home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/userdoc.rst
/home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/userdoc.rst:29: WARNING: duplicate label doc/ros2_controllers/motion_primitives_controllers/userdoc:motion_primitives_from_trajectory_controller, other instance in /home/runner/work/ros2_controllers/ros2_controllers/doc/ros2_controllers/motion_primitives_controllers/userdoc.rst

from clang

2026-02-08T11:51:18.5191489Z --- stderr: motion_primitives_controllers
2026-02-08T11:51:18.5195110Z �[1m/home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/motion_primitives_controllers/src/rdp.cpp:105:63: �[0m�[0;1;31merror: �[0m�[1mimplicit conversion changes signedness: 'std::size_t' (aka 'unsigned long') to 'difference_type' (aka 'long') [-Werror,-Wsign-conversion]�[0m
2026-02-08T11:51:18.5197861Z   105 |       rdpRecursive(PointList(points.begin(), points.begin() + index + 1), epsilon, offset);�[0m
2026-02-08T11:51:18.5198861Z       | �[0;1;32m                                                            ~ ^~~~~
2026-02-08T11:51:18.5201308Z �[0m�[1m/home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/motion_primitives_controllers/src/rdp.cpp:107:47: �[0m�[0;1;31merror: �[0m�[1mimplicit conversion changes signedness: 'std::size_t' (aka 'unsigned long') to 'difference_type' (aka 'long') [-Werror,-Wsign-conversion]�[0m
2026-02-08T11:51:18.5203821Z   107 |       rdpRecursive(PointList(points.begin() + index, points.end()), epsilon, offset + index);�[0m
2026-02-08T11:51:18.5204985Z       | �[0;1;32m                                            ~ ^~~~~
2026-02-08T11:51:18.5207487Z �[0m�[1m/home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/motion_primitives_controllers/src/rdp.cpp:191:52: �[0m�[0;1;31merror: �[0m�[1mimplicit conversion changes signedness: 'size_t' (aka 'unsigned long') to 'difference_type' (aka 'long') [-Werror,-Wsign-conversion]�[0m
2026-02-08T11:51:18.5209890Z   191 |         quaternions.begin(), quaternions.begin() + max_index + 1),�[0m
2026-02-08T11:51:18.5210709Z       | �[0;1;32m                                                 ~ ^~~~~~~~~
2026-02-08T11:51:18.5212956Z �[0m�[1m/home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/motion_primitives_controllers/src/rdp.cpp:196:31: �[0m�[0;1;31merror: �[0m�[1mimplicit conversion changes signedness: 'size_t' (aka 'unsigned long') to 'difference_type' (aka 'long') [-Werror,-Wsign-conversion]�[0m
2026-02-08T11:51:18.5215067Z   196 |         quaternions.begin() + max_index, quaternions.end()),�[0m
2026-02-08T11:51:18.5215694Z       | �[0;1;32m      

christophfroehlich pushed a commit that referenced this pull request Feb 22, 2026
…(backport #1857) (#2176)

Co-authored-by: Mathias Fuhrer <148789048+mathias31415@users.noreply.github.com>
@mathias31415
Copy link
Copy Markdown
Contributor Author

@christophfroehlich sorry for the late response. Unfortunately, I’m a bit short on time for ROS at the moment. I hope this works for now. From what I can see, the only issue left in the docs is the link to the parameter file, as this file isn’t in the final repository yet.

Copy link
Copy Markdown
Member

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

Debian build fails because we'd need to add moveit_msgs there, compatibility builds are broken currently and under investigation.

Copy link
Copy Markdown
Member

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

I see that we don't have a single test for the new controller, have you forgotten to push something? You are also referring to a non-existent parameter file.

Btw: I fixed the TOC structure with the last commit:

Image

Copy link
Copy Markdown
Member

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

I fixed the docs and converted everything to rst files. (thanks to copilot)

Before I start to integrate moveit_msgs into our debian and windows CI jobs:
I have objections against the use of /compute_fk service, maybe @mathias31415 or @destogl can comment on that.

pt.joint_positions = point.positions;
try
{
pt.pose = fk_client_->computeFK(joint_names, point.positions, "base", "tool0");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we really need this dependency on moveit's move group service?

Why not using the kinematics_interface plugins we have? the robot_description is available in every controller, wouldn't this be more appropriate?

And why are the frames here hardcoded?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just confirmed in todays PMC meeting: we should use kinematics_interface FK instead.

@christophfroehlich christophfroehlich moved this to Needs discussion in Review triage Feb 24, 2026
@github-project-automation github-project-automation bot moved this from Needs discussion to WIP in Review triage Feb 24, 2026
@christophfroehlich christophfroehlich added backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted. labels Feb 24, 2026
@christophfroehlich christophfroehlich moved this from WIP to Needs discussion in Review triage Feb 25, 2026
@christophfroehlich christophfroehlich moved this from Needs discussion to WIP in Review triage Feb 25, 2026
@christophfroehlich christophfroehlich marked this pull request as draft February 25, 2026 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted.

Projects

Status: WIP

Development

Successfully merging this pull request may close these issues.

3 participants