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

iCub3 masses updated #207

Merged
merged 2 commits into from
Aug 6, 2021
Merged

iCub3 masses updated #207

merged 2 commits into from
Aug 6, 2021

Conversation

ale-git
Copy link
Contributor

@ale-git ale-git commented Aug 6, 2021

This PR solves the real iCub3 Vs. iCub3 URDF model mass discrepancy. The real robot weights about 52 kg on the scale, while the mass calculated by the CAD - and thus exported in the model - is about 54.5 kg after the last debugging (hidden stray component removed, other missing component added).

Thus, the ICUB_3_all_options_gazebo.yaml.in has been modified in order to assign the part masses rescaled by a 52.010/54.992 factor.

image

image

@ale-git ale-git marked this pull request as draft August 6, 2021 08:30
@pattacini
Copy link
Member

@traversaro
Copy link
Member

traversaro commented Aug 6, 2021

Hi @ale-git @salvi-mattia @mfussi66 , thanks a lot for the PR!

For better tracking this kind of models, could you report the cad-mechanics checkout from which the SimMechanics model was created? Thanks!

@pattacini
Copy link
Member

pattacini commented Aug 6, 2021

@ale-git, better off referring to an issue in a public repo like robotology/icub-models#94 in the commit message, instead of pointing to our internal workflow.

@traversaro
Copy link
Member

In the past, whenever a new model was regenerated we had tipically had regression due to the direction of the axis that changed, requiring then a change in the reverseRotationAxis parameters to be coherent with the axis direction expected in YARP (see robotology/icub-models#70, #184).

Did you happen to check that?

@traversaro
Copy link
Member

Did you happen to check that?

Actually, this may be checked automatically thanks to #193, let's see the CI response.

@ale-git
Copy link
Contributor Author

ale-git commented Aug 6, 2021

@ale-git, better off referring an issue in a public repo like robotology/icub-models#94, instead of pointing to our internal workflow.

I din't consider that the commit on my repo would have sneaked till here...

@traversaro
Copy link
Member

If you want to inspect the differences in the generated model, you can check the log of CI.

Copy link
Member

@Nicogene Nicogene left a comment

Choose a reason for hiding this comment

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

Probably the changes done in ICUB_3_all_options_gazebo.yaml.in has to be done also in ICUB_3_all_options.yaml, because this fix is not gazebo-specific, what do you think about it @traversaro ?

@traversaro
Copy link
Member

@pattacini pattacini requested review from Nicogene and traversaro 7 minutes ago

We can't review the PR until it is out of Draft state, there is anything missing?

@traversaro
Copy link
Member

@pattacini pattacini requested review from Nicogene and traversaro 7 minutes ago

We can't review the PR until it is out of Draft state, there is anything missing?

Actually I was wrong, we can review even if in Draft. However, what is missing? From my point of view the PR is fine, the only request I have is the one in #207 (comment) .

@pattacini pattacini marked this pull request as ready for review August 6, 2021 08:51
@pattacini
Copy link
Member

@traversaro, the PR is ok for being reviewed 👍🏻

@mfussi66
Copy link
Member

mfussi66 commented Aug 6, 2021

For better tracking this kind of models, could you report the cad-mechanics checkout from which the SimMechanics model was created? Thanks!

@traversaro If I understood the question correctly, cad-mechanics-public master was used, specifically, up to the commit: icub-tech-iit/cad-mechanics-public@b176b2f

@traversaro
Copy link
Member

For better tracking this kind of models, could you report the cad-mechanics checkout from which the SimMechanics model was created? Thanks!

@traversaro If I understood the question correctly, cad-mechanics-public master was used, specifically, up to the commit: icub-tech-iit/cad-mechanics-public@b176b2f

Great, thanks, this is what I needed.

@traversaro
Copy link
Member

Probably the changes done in ICUB_3_all_options_gazebo.yaml.in has to be done also in ICUB_3_all_options.yaml, because this fix is not gazebo-specific, what do you think about it @traversaro ?

Sure, good catch!

@ale-git
Copy link
Contributor Author

ale-git commented Aug 6, 2021

Probably the changes done in ICUB_3_all_options_gazebo.yaml.in has to be done also in ICUB_3_all_options.yaml, because this fix is not gazebo-specific, what do you think about it @traversaro ?

Sure, good catch!

Ok, I'm going to copy-paste the section.

@traversaro
Copy link
Member

traversaro commented Aug 6, 2021

Probably the changes done in ICUB_3_all_options_gazebo.yaml.in has to be done also in ICUB_3_all_options.yaml, because this fix is not gazebo-specific, what do you think about it @traversaro ?

Sure, good catch!

By the way, this is a nice example on why relying on modified models to support quirks of specific physics engines (such as Gazebo/ODE, that has numerical errors with its default simulation parameters with small inertias that are otherwise perfectly valid) is not robusts, and the reason for which relying instead on simulation softare-specific plugins that modified the model as required during loading, like the one discussed in robotology/icub-models#33 would be useful.

@traversaro traversaro merged commit e0f80a0 into robotology:master Aug 6, 2021
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