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

Configure gripper speed and effort with hardware interface #1002

Merged
merged 39 commits into from
Jul 11, 2024

Conversation

pac48
Copy link
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
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
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.

antipodal_gripper_controller/CMakeLists.txt Outdated Show resolved Hide resolved
antipodal_gripper_controller/doc/userdoc.rst Outdated Show resolved Hide resolved
antipodal_gripper_controller/doc/userdoc.rst Outdated Show resolved Hide resolved
Comment on lines 32 to 33
using GripperCommandAction = control_msgs::action::AntipodalGripperCommand;
using GoalHandle = rclcpp_action::ServerGoalHandle<GripperCommandAction>;
Copy link
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
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

Copy link

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 pac48 requested review from bmagyar and saikishor April 2, 2024 22:57
@nbbrooks
Copy link
Contributor

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

Copy link
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!

@bmagyar bmagyar merged commit 91ebb73 into ros-controls:master Jul 11, 2024
3 of 20 checks passed
henrygerardmoore pushed a commit to henrygerardmoore/ros2_controllers that referenced this pull request Jul 19, 2024
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