Skip to content

fix: migrate active goal and tolerances to thread safe box in JTC - #2484

Open
SouriRishik wants to merge 5 commits into
ros-controls:masterfrom
SouriRishik:jtc-realtime-api
Open

fix: migrate active goal and tolerances to thread safe box in JTC#2484
SouriRishik wants to merge 5 commits into
ros-controls:masterfrom
SouriRishik:jtc-realtime-api

Conversation

@SouriRishik

Copy link
Copy Markdown
Contributor

Description

This PR migrates the JointTrajectoryController (JTC) to the modern RealtimeThreadSafeBox API to replace the deprecated RealtimeBuffer for the active goal handle and tolerances.

The migration from stale PR #1480 has been completed and brought up to date with the following fixes:

  1. Fixed a bug in all five try_set() calls where the lambda parameter was passed by value ([](auto goal)). This caused the compiler to copy the smart pointer, resulting in the active goal handle never being cleared from the box and causing subsequent actions/tests to fail or timeout.
  2. Resolved the merge conflict in preempt_active_goal() by merging the new box .set() API with the master branch's preemption abort-result delivery logic (active_goal->runNonRealtime()).
  3. Added a clean target fallback check in CMakeLists.txt to define the ros2_control_test_assets::ros2_control_test_assets target if it is not exported, ensuring compatibility with older ROS environments (such as Humble).

This finishes the stale PR #1480 and resolves the test failures.

Is this user-facing behavior change?

No

Did you use Generative AI?

No

Additional Information

Verified locally that the pre-commit checks run and pass successfully:

pre-commit run --files joint_trajectory_controller/src/joint_trajectory_controller.cpp joint_trajectory_controller/include/joint_trajectory_controller/joint_trajectory_controller.hpp joint_trajectory_controller/test/test_trajectory_controller_utils.hpp

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.85%. Comparing base (c7b97ca) to head (96eaa97).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2484      +/-   ##
==========================================
+ Coverage   86.83%   86.85%   +0.02%     
==========================================
  Files         148      148              
  Lines       16273    16278       +5     
  Branches     1368     1369       +1     
==========================================
+ Hits        14131    14139       +8     
+ Misses       1635     1631       -4     
- Partials      507      508       +1     
Flag Coverage Δ
unittests 86.85% <100.00%> (+0.02%) ⬆️

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/src/joint_trajectory_controller.cpp 85.43% <100.00%> (+0.19%) ⬆️
...ntroller/test/test_trajectory_controller_utils.hpp 83.72% <100.00%> (ø)

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

@SouriRishik

Copy link
Copy Markdown
Contributor Author

Hi @christophfroehlich, could you please review?

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