ros-controls / ros2_controllers

Generic robotic controllers to accompany ros2_control
https://control.ros.org
Apache License 2.0
321 stars 290 forks source link

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

Open mergify[bot] opened 4 days ago

mergify[bot] commented 4 days ago

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] commented 4 days ago

Cherry-pick of 07061f96f21fd4436a1f48b29e74beb21be8709a has failed:

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

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

codecov[bot] commented 4 days ago

Codecov Report

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

Project coverage is 86.71%. Comparing base (5d8a1f5) to head (d128129).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## humble #1189 +/- ## ========================================== + Coverage 86.64% 86.71% +0.07% ========================================== Files 86 87 +1 Lines 7489 7906 +417 Branches 617 630 +13 ========================================== + Hits 6489 6856 +367 - Misses 767 805 +38 - Partials 233 245 +12 ``` | [Flag](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1189/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1189/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls) | `86.71% <88.67%> (+0.07%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1189?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls) | Coverage Δ | | |---|---|---| | [...jectory\_controller/joint\_trajectory\_controller.hpp](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1189?src=pr&el=tree&filepath=joint_trajectory_controller%2Finclude%2Fjoint_trajectory_controller%2Fjoint_trajectory_controller.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#diff-am9pbnRfdHJhamVjdG9yeV9jb250cm9sbGVyL2luY2x1ZGUvam9pbnRfdHJhamVjdG9yeV9jb250cm9sbGVyL2pvaW50X3RyYWplY3RvcnlfY29udHJvbGxlci5ocHA=) | `100.00% <ø> (ø)` | | | [...int\_trajectory\_controller/test/test\_tolerances.cpp](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1189?src=pr&el=tree&filepath=joint_trajectory_controller%2Ftest%2Ftest_tolerances.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#diff-am9pbnRfdHJhamVjdG9yeV9jb250cm9sbGVyL3Rlc3QvdGVzdF90b2xlcmFuY2VzLmNwcA==) | `100.00% <100.00%> (ø)` | | | [...ectory\_controller/test/test\_trajectory\_actions.cpp](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1189?src=pr&el=tree&filepath=joint_trajectory_controller%2Ftest%2Ftest_trajectory_actions.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#diff-am9pbnRfdHJhamVjdG9yeV9jb250cm9sbGVyL3Rlc3QvdGVzdF90cmFqZWN0b3J5X2FjdGlvbnMuY3Bw) | `97.75% <100.00%> (+0.47%)` | :arrow_up: | | [...ory\_controller/src/joint\_trajectory\_controller.cpp](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1189?src=pr&el=tree&filepath=joint_trajectory_controller%2Fsrc%2Fjoint_trajectory_controller.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#diff-am9pbnRfdHJhamVjdG9yeV9jb250cm9sbGVyL3NyYy9qb2ludF90cmFqZWN0b3J5X2NvbnRyb2xsZXIuY3Bw) | `84.14% <93.75%> (+0.31%)` | :arrow_up: | | [...ntroller/test/test\_trajectory\_controller\_utils.hpp](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1189?src=pr&el=tree&filepath=joint_trajectory_controller%2Ftest%2Ftest_trajectory_controller_utils.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#diff-am9pbnRfdHJhamVjdG9yeV9jb250cm9sbGVyL3Rlc3QvdGVzdF90cmFqZWN0b3J5X2NvbnRyb2xsZXJfdXRpbHMuaHBw) | `84.59% <23.07%> (-6.06%)` | :arrow_down: | | [...include/joint\_trajectory\_controller/tolerances.hpp](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1189?src=pr&el=tree&filepath=joint_trajectory_controller%2Finclude%2Fjoint_trajectory_controller%2Ftolerances.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls#diff-am9pbnRfdHJhamVjdG9yeV9jb250cm9sbGVyL2luY2x1ZGUvam9pbnRfdHJhamVjdG9yeV9jb250cm9sbGVyL3RvbGVyYW5jZXMuaHBw) | `69.90% <63.01%> (-17.20%)` | :arrow_down: | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/ros-controls/ros2_controllers/pull/1189/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-controls)