Skip to content

Conversation

@mergify
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
@codecov
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.18%. Comparing base (f533cfd) to head (7a62ba3).

Additional details and impacted files
@@            Coverage Diff             @@
##             iron    #1207      +/-   ##
==========================================
+ Coverage   86.92%   87.18%   +0.25%     
==========================================
  Files          92       92              
  Lines        8347     8363      +16     
  Branches      699      694       -5     
==========================================
+ Hits         7256     7291      +35     
+ Misses        836      822      -14     
+ Partials      255      250       -5     
Flag Coverage Δ
unittests 87.18% <95.31%> (+0.25%) ⬆️

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 83.83% <0.00%> (-0.29%) ⬇️
...include/joint_trajectory_controller/tolerances.hpp 79.79% <94.87%> (+9.89%) ⬆️

... and 2 files with indirect coverage changes

@destogl destogl enabled auto-merge (squash) July 16, 2024 08:17
@destogl destogl disabled auto-merge July 16, 2024 08:17
@destogl destogl merged commit c728db8 into iron Jul 16, 2024
1 check passed
@destogl destogl deleted the mergify/bp/iron/pr-1192 branch July 16, 2024 08:17
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.

4 participants