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

Added finger coupling management for iCub new Mk3 hand. #469

Merged
merged 6 commits into from
Jul 17, 2020

Conversation

ale-git
Copy link
Contributor

@ale-git ale-git commented Mar 5, 2020

I've added the coupling class of the new iCub hand under developmente at present.
Since the coupling between the second and first phalanges is not linear, and thus not easily invertible, it makes use of an interpolated look up table that will be also useful in firmware implementation in control boards.

@CLAassistant
Copy link

CLAassistant commented Apr 20, 2020

CLA assistant check
All committers have signed the CLA.

@ale-git
Copy link
Contributor Author

ale-git commented May 28, 2020

Ciao @traversaro, can this be merged now?

@traversaro
Copy link
Member

Ciao @traversaro, can this be merged now?

Sorry, I had a leftover review comment that was not sent, yes I think we can merge it now.

@traversaro
Copy link
Member

Ah, given the recent work on coupling controlboard perhaps @xEnVrE may be also interested in reviewing this.

cc @randaz81

@randaz81
Copy link
Member

LGTM 👍
Just for additional clarity, can you add to the description of this PR the following infos (if available):

  1. a link to the relevant hand documentation (e.g. its geometry...)
  2. a link to the sdf model of the robot which uses the new hand mk3.

@xEnVrE
Copy link
Contributor

xEnVrE commented May 28, 2020

I agree with @randaz81. Also, some of the implemented equations are complex and if, in the future, someone wants to make a comparison between the implemented code and the documentation, it would be great if the equations are available somewhere.

@xEnVrE
Copy link
Contributor

xEnVrE commented May 28, 2020

I also want to stress the fact that with the actual code of the controlboard, while the decoupling between coupled and controlled joints will be fine, the joints limits used to instantiate the trajectory generator in the coupled-joints space will be wrong as longs as #472 this is not handled. As a result, the overall range of motion of the fingers will be different from the expected.

@traversaro
Copy link
Member

I also want to stress the fact that with the actual code of the controlboard, while the decoupling between coupled and controlled joints will be fine, the joints limits used to instantiate the trajectory generator in the coupled-joints space will be wrong as longs as #472 this is not handled. As a result, the overall range of motion of the fingers will be different from the expected.

I guess a possible workaround for now is to pass really loose limits, to ensure that the trajectory generation is not affected?

@ale-git
Copy link
Contributor Author

ale-git commented May 28, 2020

Although scary, the relation is pretty almost linear:

prox2dist

@traversaro
Copy link
Member

Link to geometry documentation:

https://github.com/icub-tech-iit/dev/blob/master/projects/icub-hand-Mk3/calculations/FingerkinV2.docx

Link to the URDF model:

https://github.com/icub-tech-iit/dev/tree/master/projects/icub-hand-Mk3/simulation_model/gazebo/iCubHandMK3_model

I am not sure if everyone has access to this private repos. In any case, adding these models to public repos is tracked in robotology/icub-models-generator#130, so eventually we could add the links suggested by @randaz81 .

@traversaro
Copy link
Member

Just to avoid confusion, the only comment left is #469 (comment) .

@traversaro
Copy link
Member

Thanks @ale-git ! @xEnVrE it is ok if I now merge this PR and then you rebase #499 on the top of the merged devel ?

@xEnVrE
Copy link
Contributor

xEnVrE commented Jul 17, 2020

@xEnVrE it is ok if I now merge this PR and then you rebase #499 on the top of the merged devel ?

Yep!

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.

5 participants