search

robotology / icub-models

Official URDF and SDF models of the iCub humanoid robot.
Creative Commons Attribution Share Alike 4.0 International
33 stars 34 forks source link

[iCubGazeboV3] Some collisions are detected in the arms #100

Closed GiulioRomualdi closed 3 years ago

GiulioRomualdi commented 3 years ago

I'm currently using the latest master of icub-models https://github.com/robotology/icub-models/commit/c6e295a4b935b5c773fe7da72ee3868066446a0b

After spawning ICubGazeboV3 I noticed a really low real-time factor.

image

Showing the collisions I noticed that some collisions are detected on the arms.

image

Is this related to https://github.com/robotology/icub-models-generator/pull/202? Is it wanted?

gazebo --version
Gazebo multi-robot simulator, version 11.5.1
Copyright (C) 2012 Open Source Robotics Foundation.
Released under the Apache 2 License.
http://gazebosim.org

cc @traversaro

traversaro commented 3 years ago

I guess this is not wanted. The strange thing, especially of the one at the elbow, is that collision between neighbor links should be disabled by default in Gazebo, so I wonder why this is happening. @AlexAntn @Nicogene do you have any insight on this?

traversaro commented 3 years ago

(By the way, a regression test on RTF for the simulated models could be a good idea to avoid regression such as this one).

Nicogene commented 3 years ago

Very strange indeed, @GiulioRomualdi do you know if this kind of slow down was already present when using the older models of icubgazebov3 ?

GiulioRomualdi commented 3 years ago

The following commit is fine 

commit 955de8fdfebb7ab1f60d8fd1dcadfe05bc587222 (HEAD, tag: v1.20.0)
Merge: 1c9dd81 85debb1
Author: Silvio Traversaro <silvio.traversaro@iit.it>
Date:   Sun May 30 12:23:39 2021 +0200

    Merge pull request #89 from robotology/devel

    Merge devel in master and do a release

image

https://github.com/robotology/icub-models/compare/v1.20.0...v1.21.0

Nicogene commented 3 years ago

From what I understood, https://github.com/robotology/icub-models-generator/pull/202 should reduce the volume/visual of the upper arm meshes, am I right @AlexAntn ?

AlexAntn commented 3 years ago

From what I understood, robotology/icub-models-generator#202 should reduce the volume/visual of the upper arm meshes, am I right @AlexAntn ?

it actually increased the volume, since the previous versions of the arms didn't account for the skin.

I believe this is indeed related to this issue since we activated the collisions for the upper arms with this PR. As @traversaro commented there shouldn't be any collisions between neighbor links, I can investigate it to try to find the reason (we didn't have such collisions during the experiments on collisions with the torso), but I think for now we can remove the collisions

pattacini commented 3 years ago

Hi @AlexAntn

As @traversaro commented there shouldn't be any collisions between neighbor links, I can investigate it to try to find the reason (we didn't have such collisions during the experiments on collisions with the torso),

If you could do a quick test with the latest model to see if you can reproduce it or not, that would be great. Maybe this will bring up something easy to fix.

but I think for now we can remove the collisions

We can proceed like that indeed. Could you deal with it?

AlexAntn commented 3 years ago

Ok, I have double-checked the work we did for the study, and in fact, in the repo study-collisions-icub we have the following warning (see https://github.com/icub-tech-iit/study-collisions-icub#icub3-gazebo-model-issues):

The iCub3 URDF model used for this study was tweaked to allow the study of the collisions for the upper arms only. As a consequence, collisions in the shoulder and elbow joints and on the forearms have been disabled. If you are using your own model to run the collision test and are detecting too many collisions, double-check the URDF file in the sections corresponding to these joints/links, and remove the <collision> segment.

When we committed the changes in the model, in order not to remove anything from the previous model, we kept the collisions on the shoulders/elbows, so that is why we are getting them now.

So the question now is, should we remove them from these joints, keeping the model as it was? or should we remove them from the arms (so revert to the previous status, except with the new meshes)? or should we remove all collisions there entirely? @pattacini @traversaro

pattacini commented 3 years ago

So the question now is, should we remove them from these joints, keeping the model as it was? or should we remove them from the arms (so revert to the previous status, except with the new meshes)? or should we remove all collisions there entirely?

I would personally stick to the status the URDF model was in ahead of this contribution, not making any difference among types of meshes.

So, if I got it right: no collisions among neighbor links and no collisions among meshes.

pattacini commented 3 years ago

So, if I got it right: no collisions among neighbor links and no collisions among meshes.

To clarify this, as you mentioned earlier, the user can always enable collisions on purpose for carrying out specific studies.

traversaro commented 3 years ago

@GiulioRomualdi the PR https://github.com/robotology/icub-models-generator/pull/208 should solve the problem.

AlexAntn commented 3 years ago

I have opened a PR here: https://github.com/robotology/icub-models-generator/pull/208

This PR removes the added sensors for the upper arms, which were the cause for the collisions with the shoulders and elbows. Nothing else in the model was changed, and I tested the model generated and it fixes the issue with the collisions.

Indeed, the real-time factor basically doubled without these collisions, so this should fix this issue!

GiulioRomualdi commented 3 years ago

Thank you @AlexAntn

GiulioRomualdi commented 3 years ago

The new model has been generated 8ad57bf2d7fad97d80438f41da0e309afbf90c4d. I've just tested and everything seems ok. Thank you for the support.

pattacini commented 3 years ago

Thanks heaps @AlexAntn 🥇