Skip to content

Trajectory blending with new trajectory deferral#2401

Open
vedh1234 wants to merge 7 commits into
ros-controls:masterfrom
vedh1234:feat--trajectory-blending-with-deferring-the-new-traj
Open

Trajectory blending with new trajectory deferral#2401
vedh1234 wants to merge 7 commits into
ros-controls:masterfrom
vedh1234:feat--trajectory-blending-with-deferring-the-new-traj

Conversation

@vedh1234

@vedh1234 vedh1234 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Description

Partial port of the ROS 1 "update existing trajectory" behavior to ROS 2 joint_trajectory_controller.

Related to #84

What this PR does:

  • Adds a new parameter allow_trajectory_blending (default: true)
  • When enabled, a trajectory with a future header.stamp is deferred: the currently active trajectory continues executing until that start time arrives, then the new trajectory is installed
  • This eliminates the legacy behavior where a future-stamped trajectory would immediately kill the active trajectory and create a slow spline ramp to the first point
  • Deferred trajectories are correctly dropped on goal cancellation, controller deactivation, and controller reset
  • Action Server feedback/tolerance checking is suppressed during the deferral period to prevent spurious aborts
  • Immediate trajectories (stamp = 0 or stamp <= now) behave exactly as before (no behavioral change)

What this PR does NOT target: ( Compared to ROS1 blending feature )

  • Does not allow omitted joints to continue their old trajectory after the new one starts (ROS 1 full blend). Omitted joints freeze at their current position via fill_partial_goal(), same as legacy ROS 2 behavior
  • Does not handle speed scaling during the deferral period. The trigger uses wall-clock time, so if speed_scaling != 1.0, there will be a positional mismatch at handoff
  • Does not track multiple action goals simultaneously. The new goal immediately preempts the old one at the Action Server level; only the physical trajectory execution is deferred

Is this user-facing behavior change?

Yes. When allow_trajectory_blending is true (default), the active trajectory will continue running until the new trajectory's start time.

Did you use Generative AI?

Yes, Google Gemini and Claude were used to make improvements in manually written code, architectural analysis, edge-case review.

Additional Information

TODOs

To send us a pull request, please:

  • Fork the repository.
  • Modify the source; please focus on the specific change you are contributing. If you also reformat all the code, it will be hard for us to focus on your change.
  • Ensure local tests pass. (colcon test and pre-commit run (requires you to install pre-commit by pip3 install pre-commit)
  • Commit to your fork using clear commit messages.
  • Send a pull request, answering any default questions in the pull request interface.
  • Pay attention to any automated CI failures reported in the pull request, and stay involved in the conversation.

@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.69421% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.93%. Comparing base (3d85551) to head (f136046).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...ectory_controller/test/test_trajectory_actions.cpp 96.89% 0 Missing and 5 partials ⚠️
...ory_controller/src/joint_trajectory_controller.cpp 97.29% 0 Missing and 1 partial ⚠️
...ory_controller/test/test_trajectory_controller.cpp 97.50% 0 Missing and 1 partial ⚠️
...ntroller/test/test_trajectory_controller_utils.hpp 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2401      +/-   ##
==========================================
+ Coverage   86.76%   86.93%   +0.16%     
==========================================
  Files         148      148              
  Lines       15853    16102     +249     
  Branches     1347     1357      +10     
==========================================
+ Hits        13755    13998     +243     
+ Misses       1604     1601       -3     
- Partials      494      503       +9     
Flag Coverage Δ
unittests 86.93% <96.69%> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...jectory_controller/joint_trajectory_controller.hpp 62.50% <100.00%> (+22.50%) ⬆️
...ory_controller/src/joint_trajectory_controller.cpp 85.23% <97.29%> (+0.54%) ⬆️
...ory_controller/test/test_trajectory_controller.cpp 98.81% <97.50%> (-0.04%) ⬇️
...ntroller/test/test_trajectory_controller_utils.hpp 83.72% <0.00%> (-0.60%) ⬇️
...ectory_controller/test/test_trajectory_actions.cpp 97.54% <96.89%> (-0.16%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@christophfroehlich christophfroehlich left a comment

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.

Partial port of the ROS 1 "update existing trajectory" behavior to ROS 2 joint_trajectory_controller.

Fixes #84

For documentation here, what is different from the ROS 1 behavior/what is maybe missing to port?

Please also update the release_notes and update the documentation.

Comment thread doc/release_notes.rst Outdated
const rclcpp::Duration update_rate = rclcpp::Duration::from_seconds(0.01))
{
if (start_time == rclcpp::Time(0, 0, RCL_STEADY_TIME))
if (start_time.nanoseconds() == 0)

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.

Why did you have to change this?

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.

IMO this is a better comparison as it doesn't involve the type of the clock in a zero check

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For the new test I had to pass a future stamp which was required to use ROS_TIME to pass validate_trajectory_msg. This caused a clock type mismatch in updateControllerAsync because the check start_time == rclcpp::Time(0, 0, RCL_STEADY_TIME) throws when start_time is a different clock type. Since the check is purely testing for a zero value, I switched to start_time.nanoseconds() == 0 which is clock-type-agnostic.

@saikishor

Copy link
Copy Markdown
Member

I'll review the logic and tests soon. I need time

@vedh1234 vedh1234 force-pushed the feat--trajectory-blending-with-deferring-the-new-traj branch from 498c4ed to e7db781 Compare June 17, 2026 20:09

@saikishor saikishor left a comment

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.

The main logic looks good.

I still need to verify the tests though

Comment thread doc/release_notes.rst Outdated
Comment thread joint_trajectory_controller/doc/trajectory.rst Outdated
Comment thread joint_trajectory_controller/doc/trajectory.rst
Comment thread joint_trajectory_controller/doc/trajectory.rst Outdated
Comment thread joint_trajectory_controller/doc/trajectory.rst Outdated
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