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

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

Closed
GiulioRomualdi opened this issue Aug 30, 2021 · 15 comments · Fixed by #103
Closed

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

GiulioRomualdi opened this issue Aug 30, 2021 · 15 comments · Fixed by #103

Comments

@GiulioRomualdi
Copy link
Member

I'm currently using the latest master of icub-models c6e295a

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 robotology/icub-models-generator#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

@GiulioRomualdi GiulioRomualdi changed the title [iCubGazeboV3] Some collisions are dected in the arms [iCubGazeboV3] Some collisions are detected in the arms Aug 30, 2021
@traversaro
Copy link
Member

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
Copy link
Member

(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
Copy link
Member

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

@GiulioRomualdi
Copy link
Member Author

GiulioRomualdi commented Aug 30, 2021

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

v1.20.0...v1.21.0

@Nicogene
Copy link
Member

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

@AlexAntn
Copy link

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
Copy link
Member

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
Copy link

AlexAntn commented Aug 31, 2021

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

@GiulioRomualdi the PR robotology/icub-models-generator#208 should solve the problem.

@AlexAntn
Copy link

I have opened a PR here: robotology/icub-models-generator#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
Copy link
Member Author

Thank you @AlexAntn

@GiulioRomualdi
Copy link
Member Author

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

@pattacini
Copy link
Member

Thanks heaps @AlexAntn 🥇

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 a pull request may close this issue.

5 participants