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

[Polymetis] Completely expose Franka Hand gripper API #1399

Merged
merged 3 commits into from
Dec 2, 2022

Conversation

exhaustin
Copy link
Contributor

Description

Issue #1398 highlights an inflexibility of franka::Gripper::move, which is currently the only API Polymetis is using to command the Franka Hand.
Basically the move command blocks until it reaches the desired goal, which causes problems during grasping.
This PR exposes franka::Gripper::grasp, which is a clunky API that at least allows the user to execute a grasp instead of commanding the gripper to 0 width.

Type of change

Please check the options that are relevant.

  • Bug fix (non-breaking change that fixes an issue)
  • Proposes a change (non-breaking change that isn't necessarily a bug)
  • Refactor
  • New feature (non-breaking change that adds a new functionality)
  • Breaking change (fix or feature that would break some existing functionality downstream)
  • This is a unit test
  • Documentation only change
  • Datasets Release
  • Models Release

Type of requested review

  • I want a thorough review of the implementation.
  • I want a high level review.
  • I want a deep design review.

Before and After

If this PR introduces a change or fixes a bug, please add before and after screenshots (if this is causes a UX change) or before and after examples(this could be plain text, videos etc ) to demonstrate the change.

If this PR introduces a new feature, please also add examples or screenshots demonstrating the feature.

Testing

Tested on hardware

Checklist:

  • I have performed manual end-to-end testing of the feature in my environment.
  • I have added Docstrings and comments to the code.
  • I have made changes to existing documentation where needed.
  • I have added tests that show that the PR is functional.
  • New and existing unit tests pass locally with my changes.
  • I have added relevant collaborators to review the PR before merge.
  • [Polymetis only] I ran on hardware (1) all scripts in tests/scripts, (2) asv benchmarks.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 2, 2022
@exhaustin exhaustin requested a review from 1heart December 2, 2022 00:13
Copy link
Contributor

@1heart 1heart left a comment

Choose a reason for hiding this comment

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

Makes sense.

One question -- it looks like epsilon_inner/outer have different default values in Python/C++. Where are the values coming from?

@exhaustin
Copy link
Contributor Author

Makes sense.

One question -- it looks like epsilon_inner/outer have different default values in Python/C++. Where are the values coming from?

That was from my original misunderstanding of how these parameters work. I thought they were upper and lower limits, and so I set the epsilon_inner to slightly below zero and epsilon_upper to larger than max width.
After learning that they were tolerances from the target grasp width, I just default them to large numbers.

@exhaustin exhaustin merged commit 3f2291e into main Dec 2, 2022
@exhaustin exhaustin deleted the austinw/gripper_grasping branch December 2, 2022 17:35
@1heart
Copy link
Contributor

1heart commented Dec 2, 2022

Seems like mrp tests are broken here? Not sure why

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants