Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JTC] Make goal_time_tolerance overwrite default value only if explicitly set (backport #1192 + #1209) #1208

Merged
merged 2 commits into from
Jul 16, 2024

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Jul 15, 2024

Since #716 we process the tolerances set from the action goal. This was done as described in https://github.com/ros-controls/control_msgs/blob/65814d985aa29bed0adfaed5f2ebd8cf26266056/control_msgs/action/FollowJointTrajectory.action#L6-L10 and https://github.com/ros-controls/control_msgs/blob/65814d985aa29bed0adfaed5f2ebd8cf26266056/control_msgs/msg/JointTolerance.msg#L7-L10.

However, the goal_time_tolerance was always overwritten with the value from the action goal. This actually changed the JTC's default behavior if

  • a goal_time_tolerance != 0 was set in the controller config and
  • no goal_time_tolerance or explicitly a 0.0 was given in the action goal.

In this, case, the controller would wait indefinitely until the robot reached the final target independent of the goal_time_tolerance configured inside the controller's default values.

This PR makes it such as

  • If a goal_time_tolerance of 0.0 is given in the action (which is the default if left empty), the value from the controller config will be used
  • If a goal_time_tolerance of -1.0 is given the controller will wait at the end of the trajectory until the error gets lower than the goal_tolerance for each joint.
  • If a goal_time_tolerance > 0.0 is given, the goal_time_tolerance from the action goal will be used.
  • Any other negative value for goal_time_tolerance will make the controller fall back to its configured tolerances completely.
    This is an automatic backport of pull request [JTC] Make goal_time_tolerance overwrite default value only if explicitly set #1192 done by Mergify.

@christophfroehlich christophfroehlich changed the title [JTC] Make goal_time_tolerance overwrite default value only if explicitly set (backport #1192) [JTC] Make goal_time_tolerance overwrite default value only if explicitly set (backport #1192 + #1209) Jul 16, 2024
Copy link

codecov bot commented Jul 16, 2024

Codecov Report

Attention: Patch coverage is 95.31250% with 3 lines in your changes missing coverage. Please review.

Project coverage is 87.08%. Comparing base (143bfc9) to head (9fbd7fe).

Additional details and impacted files
@@            Coverage Diff             @@
##           humble    #1208      +/-   ##
==========================================
+ Coverage   86.93%   87.08%   +0.14%     
==========================================
  Files          87       87              
  Lines        7966     7981      +15     
  Branches      630      625       -5     
==========================================
+ Hits         6925     6950      +25     
+ Misses        799      794       -5     
+ Partials      242      237       -5     
Flag Coverage Δ
unittests 87.08% <95.31%> (+0.14%) ⬆️

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

Files Coverage Δ
...int_trajectory_controller/test/test_tolerances.cpp 100.00% <100.00%> (ø)
...ectory_controller/test/test_trajectory_actions.cpp 97.75% <100.00%> (ø)
...ntroller/test/test_trajectory_controller_utils.hpp 84.31% <0.00%> (-0.28%) ⬇️
...include/joint_trajectory_controller/tolerances.hpp 79.79% <94.87%> (+9.89%) ⬆️

... and 3 files with indirect coverage changes

@destogl destogl merged commit 8d753a8 into humble Jul 16, 2024
10 of 11 checks passed
@destogl destogl deleted the mergify/bp/humble/pr-1192 branch July 16, 2024 08:16
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.

3 participants