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

Add velocity to gripper command #99

Merged

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. I propose a new gripper action message that supports a target_velocity field.

This PR is related to the controller developed in ros-controls/ros2_controllers#1002

@pac48 pac48 marked this pull request as ready for review January 30, 2024 17:08
@pac48 pac48 marked this pull request as draft February 1, 2024 19:52
@pac48 pac48 force-pushed the pr-add-gripper-velocity-target-main branch from 8d6f801 to b891826 Compare February 1, 2024 23:19
@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 from b891826 to cbf637d Compare February 1, 2024 23:45
control_msgs/action/AntipodalGripperCommand.action Outdated Show resolved Hide resolved
control_msgs/action/AntipodalGripperCommand.action Outdated Show resolved Hide resolved
control_msgs/action/AntipodalGripperCommand.action Outdated Show resolved Hide resolved
control_msgs/action/AntipodalGripperCommand.action Outdated Show resolved Hide resolved
control_msgs/action/AntipodalGripperCommand.action Outdated Show resolved Hide resolved
Comment on lines 5 to 13
bool stalled # True iff the gripper is exerting max effort and not moving
bool reached_goal # True iff the gripper position has reached the commanded setpoint
Copy link

Choose a reason for hiding this comment

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

Is there value in reporting reached_goal or stalled in the feedback topic? If the feedback was a JointState the caller could inspect the joint velocity and I would suggest changing reached_goal to something like percent complete over a bool.

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 am not sure why those are here. I can actually understand stalled because some users might want to monitor stalled and send a cancel goal after it has been stalled too long. But I don't know why the goal term would be in the feedback.

Choose a reason for hiding this comment

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

I vote we remove both of these options from the feedback topic.

@pac48 pac48 force-pushed the pr-add-gripper-velocity-target-main branch 2 times, most recently from b8d4e2d to 1f4905a Compare February 6, 2024 21:46
@pac48 pac48 changed the title Add target_velocity to gripper command Add velocity to gripper command Feb 6, 2024
Comment on lines 5 to 7
# position refers to the gap size (in meters).
# Optional: velocity refers to the Cartesian velocity of the finger right finger.
# effort refers to the current effort exerted (in Newtons)
Copy link

@MarqRazz MarqRazz Feb 12, 2024

Choose a reason for hiding this comment

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

Suggested change
# position refers to the gap size (in meters).
# Optional: velocity refers to the Cartesian velocity of the finger right finger.
# effort refers to the current effort exerted (in Newtons)
# position refers to the position of each joint (radians or meters).
# Optional: velocity of each joint (radians or meters / Second).
# Optional: effort of each joint (Newtons or Newton-meters)

As we discussed I don't think it makes sense to command distance between the fingers because its not a physical joint on the robot and the result can be interpenetrated in many different ways (especially if the user is able to manufacture custom fingers).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines 5 to 13
bool stalled # True iff the gripper is exerting max effort and not moving
bool reached_goal # True iff the gripper position has reached the commanded setpoint

Choose a reason for hiding this comment

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

I vote we remove both of these options from the feedback topic.

@pac48 pac48 force-pushed the pr-add-gripper-velocity-target-main branch from 1f4905a to 1b36b4f Compare February 13, 2024 17:26
@pac48
Copy link
Contributor Author

pac48 commented Feb 13, 2024

@MarqRazz Okay, I removed the extra fields from the feedback topic and updated the comments

@pac48 pac48 requested a review from MarqRazz February 13, 2024 17:28
@pac48 pac48 requested a review from MarqRazz April 2, 2024 22:57
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.

Will wait for someone else approving

@pac48 pac48 requested a review from bmagyar April 8, 2024 22:40
@bmagyar bmagyar merged commit d4be560 into ros-controls:master Apr 9, 2024
5 of 7 checks passed
@bmagyar
Copy link
Member

bmagyar commented Apr 9, 2024

@Mergifyio backport to humble

Copy link

mergify bot commented Apr 9, 2024

backport to humble

❌ No backport have been created

  • Backport to branch to failed

GitHub error: Branch not found

mergify bot pushed a commit that referenced this pull request Apr 9, 2024
bmagyar pushed a commit that referenced this pull request May 8, 2024
(cherry picked from commit d4be560)

Co-authored-by: Paul Gesel <[email protected]>
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