ros2_controllers icon indicating copy to clipboard operation
ros2_controllers copied to clipboard

[JTC] Process tolerances sent with action goal (backport #716)

Open mergify[bot] opened this issue 1 year ago • 2 comments

From here:

JointTrajectoryController ignores goal_time_tolerance set via FollowJointTrajectory.Goal.goal_time_tolerance together with path_tolerance and goal_tolerance .

This PR proposes a way how to process the tolerances: The tolerances from one action goal are not saved for the next one, but the default ones will be used if no tolerances are set with the following action goals.

(Temporary) deactivation is also implemented like documented in the msg definition.

From the node parameter we cannot set velocity or acceleration tolerances (except for a single stopped_velocity_tolerance for the goal tolerances of all joints). Should we add them as parameters as well, to have the same structure like the action message?

This new feature could break some existing projects, because the tolerances were just ignored and might now be breaking behaviors. Should we introduce a temporary parameter to opt-in?

Fixes #249


This is an automatic backport of pull request #716 done by Mergify.

mergify[bot] avatar Jul 03 '24 17:07 mergify[bot]

Cherry-pick of 07061f96f21fd4436a1f48b29e74beb21be8709a has failed:

On branch mergify/bp/iron/pr-716
Your branch is up to date with 'origin/iron'.

You are currently cherry-picking commit 07061f9.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   joint_trajectory_controller/CMakeLists.txt
	modified:   joint_trajectory_controller/doc/userdoc.rst
	modified:   joint_trajectory_controller/include/joint_trajectory_controller/joint_trajectory_controller.hpp
	modified:   joint_trajectory_controller/include/joint_trajectory_controller/tolerances.hpp
	modified:   joint_trajectory_controller/src/joint_trajectory_controller.cpp
	new file:   joint_trajectory_controller/test/test_tolerances.cpp
	modified:   joint_trajectory_controller/test/test_trajectory_actions.cpp
	modified:   joint_trajectory_controller/test/test_trajectory_controller_utils.hpp

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   doc/migration/Jazzy.rst
	deleted by us:   doc/release_notes/Jazzy.rst

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

mergify[bot] avatar Jul 03 '24 17:07 mergify[bot]

Codecov Report

Attention: Patch coverage is 88.67925% with 48 lines in your changes missing coverage. Please review.

Project coverage is 86.74%. Comparing base (101b0cb) to head (89a392e).

Additional details and impacted files
@@            Coverage Diff             @@
##             iron    #1190      +/-   ##
==========================================
- Coverage   86.93%   86.74%   -0.19%     
==========================================
  Files          91       92       +1     
  Lines        7937     8347     +410     
  Branches      692      699       +7     
==========================================
+ Hits         6900     7241     +341     
- Misses        789      851      +62     
- Partials      248      255       +7     
Flag Coverage Δ
unittests 86.74% <88.67%> (-0.19%) :arrow_down:

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

Files Coverage Δ
...jectory_controller/joint_trajectory_controller.hpp 100.00% <ø> (ø)
...int_trajectory_controller/test/test_tolerances.cpp 100.00% <100.00%> (ø)
...ectory_controller/test/test_trajectory_actions.cpp 97.75% <100.00%> (+0.47%) :arrow_up:
...ory_controller/src/joint_trajectory_controller.cpp 83.67% <93.75%> (+0.19%) :arrow_up:
...ntroller/test/test_trajectory_controller_utils.hpp 84.12% <23.07%> (-6.22%) :arrow_down:
...include/joint_trajectory_controller/tolerances.hpp 69.90% <63.01%> (-17.20%) :arrow_down:

... and 4 files with indirect coverage changes

codecov[bot] avatar Jul 03 '24 18:07 codecov[bot]

I fixed the merge conflict with the release notes, and removed all entries which are not applicable to this branch.

christophfroehlich avatar Jul 15 '24 08:07 christophfroehlich