Skip to content

Configure gripper speed and effort with hardware interface#1002

Merged
bmagyar merged 39 commits intoros-controls:masterfrom
pac48:pr-add-gripper-velocity-target-main
Jul 11, 2024
Merged

Configure gripper speed and effort with hardware interface#1002
bmagyar merged 39 commits intoros-controls:masterfrom
pac48:pr-add-gripper-velocity-target-main

Conversation

@pac48
Copy link
Copy Markdown
Contributor

@pac48 pac48 commented Jan 30, 2024

Many robot grippers support effort, position, and velocity controls. The current technique to set the velocity involves reading a hardcoded value from the URDF. This is problematic when the user wants to change the gripper velocity at runtime. After discussions in the ros2_control WG meeting, the best strategy to achieve this goal while allowing backporting to Humble is to add a new controller that specifically supports this and deprecate the old controller.

This PR adds a controller called antipodal_gripper_controllers which has parameters to optionally claim the hardware interfaces JOINT_NAME/gripper_effort and JOINT_NAME/gripper_speed, which will be used to set the gripper speed and effort if enabled.

This PR requires:

@pac48 pac48 force-pushed the pr-add-gripper-velocity-target-main branch 2 times, most recently from 000891b to 2f0d850 Compare January 30, 2024 18:12
@pac48 pac48 marked this pull request as draft February 1, 2024 19:53
@pac48 pac48 force-pushed the pr-add-gripper-velocity-target-main branch 2 times, most recently from 66a9b7f to d19c800 Compare February 1, 2024 23:33
@pac48 pac48 marked this pull request as ready for review February 1, 2024 23:34
@pac48 pac48 force-pushed the pr-add-gripper-velocity-target-main branch 5 times, most recently from 63d50fc to e52efc0 Compare February 2, 2024 00:41
@moriarty
Copy link
Copy Markdown
Contributor

moriarty commented Feb 5, 2024

  --- stderr: antipodal_gripper_controller
  In file included from /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/antipodal_gripper_controller/src/antipodal_gripper_action_controller.cpp:18:
  /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/antipodal_gripper_controller/include/antipodal_gripper_controller/antipodal_gripper_action_controller.hpp:30:10: fatal error: control_msgs/action/antipodal_gripper_command.hpp: No such file or directory
     30 | #include "control_msgs/action/antipodal_gripper_command.hpp"
        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@pac48 can you include a temporary change on this pr to update the .repos file and test against your other PR.

https://github.com/ros-controls/ros2_controllers/blob/master/ros2_controllers-not-released.rolling.repos

I believe that should turn Rolling Semi-Binary Build - main / semi_binary / rolling main (pull_request) green.

Once your ros-controls/control_msgs#99 is approved an merged we'll need to update the .repos to pull in main from the messages until the next ros2 sync.

Copy link
Copy Markdown
Contributor

@MarqRazz MarqRazz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still need to pull this down and test but here are some high level suggestions.

Comment on lines +32 to +33
using GripperCommandAction = control_msgs::action::AntipodalGripperCommand;
using GoalHandle = rclcpp_action::ServerGoalHandle<GripperCommandAction>;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to define these again, they are already in the header antipodal_gripper_action_controller.hpp.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is needed here because it is defined inside a class in antipodal_gripper_action_controller.hpp

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 6, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 71.31%. Comparing base (0b43291) to head (114012b).
Report is 14 commits behind head on master.

❗ Current head 114012b differs from pull request most recent head ebb347f. Consider uploading reports for the commit ebb347f to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1002      +/-   ##
==========================================
- Coverage   71.86%   71.31%   -0.56%     
==========================================
  Files          41       41              
  Lines        3650     3374     -276     
  Branches     1794     1627     -167     
==========================================
- Hits         2623     2406     -217     
+ Misses        707      667      -40     
+ Partials      320      301      -19     
Flag Coverage Δ
unittests 71.31% <0.00%> (-0.56%) ⬇️

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

Files Coverage Δ
...per_controllers/gripper_action_controller_impl.hpp 34.26% <0.00%> (-0.25%) ⬇️

... and 11 files with indirect coverage changes

pac48 and others added 4 commits February 6, 2024 15:36
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
pac48 added 4 commits April 2, 2024 15:15
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
@pac48 pac48 requested review from bmagyar and saikishor April 2, 2024 22:57
pac48 added 2 commits April 8, 2024 16:56
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
@nbbrooks
Copy link
Copy Markdown
Contributor

@bmagyar It looks like the renaming to parallel_gripper_controller has been addressed - can you resolve your request for changes?

Copy link
Copy Markdown
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spotted one last piece of commented code, otherwise looks good!

pac48 and others added 2 commits July 11, 2024 06:51
…r/parallel_gripper_action_controller_impl.hpp

Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
@bmagyar bmagyar merged commit 91ebb73 into ros-controls:master Jul 11, 2024
henrygerardmoore pushed a commit to henrygerardmoore/ros2_controllers that referenced this pull request Jul 19, 2024
medvedevigorek pushed a commit to AivotRobotics/ros2_controllers that referenced this pull request Apr 30, 2025
medvedevigorek added a commit to AivotRobotics/ros2_controllers that referenced this pull request May 1, 2025
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.

7 participants