Enhance: Added joint limit loader plugin by sachinkum0009 · Pull Request #2442 · ros-controls/ros2_controllers

sachinkum0009 · 2026-06-26T13:42:57Z

Description

This PR integrates the Joint Limiter using Plugin Loader to use Joint Saturation Limiter.

Summary

Added Pluginlib to load the JointSaturationLimit plugin
Added Tests to validate the plugin for Pos, Vel, Vel + Acc and Pos + Vel + Acc

Is this user-facing behavior change?

Yes, with this PR, the JTC uses Joint Saturation Limiter to limit the pos, vel, acc, or jerk based on the configuration.

Did you use Generative AI?

Used OpenCode to understand the code and write tests.

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.

…g of available classes Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

…ation limits Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

codecov · 2026-06-26T14:16:59Z

Codecov Report

❌ Patch coverage is 93.85965% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.97%. Comparing base (c7b97ca) to head (6172714).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
...ory_controller/src/joint_trajectory_controller.cpp	39.28%	12 Missing and 5 partials ⚠️
...jectory_controller/test/test_joint_limiter_jtc.cpp	98.51%	0 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2442      +/-   ##
==========================================
+ Coverage   86.83%   86.97%   +0.13%     
==========================================
  Files         148      149       +1     
  Lines       16273    16615     +342     
  Branches     1368     1394      +26     
==========================================
+ Hits        14131    14451     +320     
- Misses       1635     1644       +9     
- Partials      507      520      +13

Flag	Coverage Δ
unittests	`86.97% <93.85%> (+0.13%)`	⬆️

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% <ø> (ø)`
...ory_controller/test/test_trajectory_controller.cpp	`98.83% <100.00%> (+0.01%)`	⬆️
...ntroller/test/test_trajectory_controller_utils.hpp	`83.97% <100.00%> (+0.25%)`	⬆️
...jectory_controller/test/test_joint_limiter_jtc.cpp	`98.51% <98.51%> (ø)`
...ory_controller/src/joint_trajectory_controller.cpp	`83.85% <39.28%> (-1.39%)`	⬇️

... 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.

type Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

sachinkum0009 · 2026-06-29T13:00:27Z

Test Integrated

Test Name	Implemented
Joint limiter failed to configure	✅
Joint limiter config successfully	✅
joint limiter failed to activate	✅
Joint limiter activate successfully	✅
	✅
Limits Position	✅
Limits Velocity	✅
Limits Acceleration	✅
Limits Pos, Vel, Acc	✅
Short Sample Traj within limits	✅
Short Sample Traj on limits	✅
Short Sample Traj over limits	✅
Long Sample Traj within limits	✅
Long Sample Traj on limits	✅
Long Sample Traj over limits	✅

Moved joint limiter tests from `test_trajectory_controller.cpp` to a new `test_joint_limiter_jtc.cpp` file and added a helper function to provide default limiter parameters to reduce duplication. Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

Expanded the test suite to include long and short trajectories that exceed defined limits. Also replaced `std::abs` with `std::fabs` for floating-point comparisons in tests and marked helper function as inline. Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

mergify · 2026-06-30T17:57:47Z

This pull request is in conflict. Could you fix it @sachinkum0009?

destogl

Please comment. It seems a bit trivial implemenation. Explain why you think it is correct.

destogl · 2026-07-08T20:11:50Z

+      // Apply joint limiter to the next commanded state
+      if (joint_limiter_)
+      {
+        joint_limiter_->enforce(state_current_, command_next_, update_period_);


why is here the right place to enforce limits? Shouldn't we do this in before and after interpolation?

I think the best place to enforce limits is after the interpolation. If appied limits before interpolation, it can produce inconsistent interpolation results.

What happens than with the interpolation? Will we still have interpolated values?

Thanks. Yes, we still have the interpolated values, as the joint limiter is applied before publishing the next cmd to hardware interface.

destogl · 2026-07-08T20:12:25Z

+    // Apply joint limiter to the requested state
+    if (joint_limiter_)
+    {
+      joint_limiter_->enforce(state_current_, state_requested, update_period_);


Why this second call in the query state service?

As the ~/query_state service (control_msgs::srv::QueryTrajectoryState) allows querying the expected state at a future time. I think if we have joint limiter plugin enabled then we should return the response with limits applied.

destogl · 2026-07-08T20:14:37Z

+  EXPECT_NEAR(joint_vel_[0], 0.5, COMMON_THRESHOLD);
+  EXPECT_NEAR(joint_vel_[1], 0.5, COMMON_THRESHOLD);
+  EXPECT_NEAR(joint_vel_[2], 0.5, COMMON_THRESHOLD);
+  EXPECT_NEAR(joint_pos_[0], INITIAL_POS_JOINT1 + 0.5 * 0.01, COMMON_THRESHOLD);


I am not sure whether this is correct, as you are testing the limiter. What happens with interpolation in this case? Are we breaking it?

Thanks, I have updated the tests to validate the interpolated values and make sure the position, velocity and acceleration remains within the limits.

updates Replace static initial positions with a trajectory containing three distinct waypoints. Adjust the update loop to step through each segment using constant 10ms intervals, ensuring verification covers the full duration of the published points up to t=0.6s. Use `limit_enforced_points` for assertions to account for clamped values at specific times rather than checking immediately after publishing a single point. Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

Replaced single-step snapshot assertions with a loop that advances the controller at 10ms intervals. This change verifies both velocity and acceleration limits across multiple interpolated points for each segment of the trajectory. Tests now iterate through all waypoints, ensuring constraints hold throughout motion rather than just after initial application. Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

Increase `max_velocity` for joint1, joint2, and joint3 from 0.5 to 2.0 radians/s in the unit tests configuration function. This adjustment reflects realistic actuator capabilities used during testing scenarios involving dynamic motion commands. Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

clamping - Remove unused `limit_enforced_points` vectors from velocity and acceleration tests. - Replace manual assertions with a loop stepping through the trajectory at 10ms intervals. - Verify that commanded velocities, accelerations, and positions respect all limits throughout execution. Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

sachinkum0009 · 2026-07-20T20:57:33Z

I have plotted few test cases for JTC with joint saturation limiter plugin.

Trajector on the limits

Trajectory over the limits

Pls LMK if there are some more suggestions or feedback. I would like to merge this ASAP. 🙏

sachinkum0009 added 4 commits June 24, 2026 16:20

joint limit loader plugin added

48fb1b5

pre commit fix

9932bf2

Refactor joint limiter management to use UniquePtr and enhance loggin…

91eea52

…g of available classes Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

Merge branch 'ros-controls:master' into feat/jtc-jerk-limits

197f368

github-actions Bot requested review from MarqRazz, Maverobot, bijoua29, destogl and fmauch June 26, 2026 13:43

Add tests for joint limiter enforcing position, velocity, and acceler…

0da6f6b

…ation limits Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

Tests updated for configure and activate controller with joint limiter

e8dd630

type Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

sachinkum0009 added 3 commits June 29, 2026 15:03

license updated

bc08228

Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

sachinkum0009 added 3 commits June 30, 2026 23:07

Merge branch 'master' into feat/jtc-jerk-limits

f8cbdb3

Merge branch 'master' into feat/jtc-jerk-limits

d63a61e

Merge branch 'master' into feat/jtc-jerk-limits

41f4524

destogl reviewed Jul 8, 2026

View reviewed changes

sachinkum0009 added 9 commits July 17, 2026 13:34

Merge branch 'master' into feat/jtc-jerk-limits

5e201d1

Refactor velocity limiter test to use iterative simulation loop

70e781b

Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

Enforce joint limits on current state before interpolation

e9091d3

Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

test updated for pos, vel, and acc

7c711b7

Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

test updated to validate the trajectory limits within limits

bed52bf

Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

removed unwanted joint limiter

ea6f549

Signed-off-by: Sachin Kumar <sachinkum123567@gmail.com>

Merge branch 'master' into feat/jtc-jerk-limits

6172714

Uh oh!

Conversation

sachinkum0009 commented Jun 26, 2026

Description

Is this user-facing behavior change?

Did you use Generative AI?

Additional Information

TODOs

Uh oh!

codecov Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sachinkum0009 commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Integrated

Uh oh!

mergify Bot commented Jun 30, 2026

Uh oh!

destogl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sachinkum0009 commented Jul 20, 2026

Trajector on the limits

Trajectory over the limits

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Jun 26, 2026 •

edited

Loading

sachinkum0009 commented Jun 29, 2026 •

edited

Loading