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 fixed models. #27

Merged
merged 2 commits into from
Mar 15, 2020
Merged

Conversation

S-Dafarra
Copy link
Contributor

No description provided.

Copy link
Member

@traversaro traversaro left a comment

Choose a reason for hiding this comment

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

Thanks!

@gabrielenava do you think this can conflict with icub-gazebo-wholebody ?

@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<model>
<name>@ROBOT_NAME@_fixed (no hands)</name>
Copy link
Member

Choose a reason for hiding this comment

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

That (no hands) is not ideal, but I see that you got it from the existing model.config.in, so if we remove it we will remove it from both.

@S-Dafarra
Copy link
Contributor Author

I can also quickly add the models with the fixed feet

@S-Dafarra
Copy link
Contributor Author

cc @nunoguedelha

@gabrielenava
Copy link

@gabrielenava do you think this can conflict with icub-gazebo-wholebody ?

I don't think so, because in master and devel branches of icub-gazebo-wholebody the fixed iCub models are pointing to iCubGazeboSim. But it seems there are old branches that go towards this direction, see https://github.com/robotology/icub-gazebo-wholebody/tree/feature/useModelsFromCAD.

I propose an optional alternative: maybe we can edit the already existing fixed models to point the @ROBOT_NAME@ model, to reduce the number of duplicated models.

@traversaro
Copy link
Member

I can also quickly add the models with the fixed feet

Mhh, note that can be more tricky as if I recall correctly you need to hardcode the height of the waist in the fixed model sdf to correctly write the constraints on the feet.

@traversaro
Copy link
Member

@gabrielenava do you think this can conflict with icub-gazebo-wholebody ?

I propose an optional alternative: maybe we can edit the already existing fixed models to point the @ROBOT_NAME@ model, to reduce the number of duplicated models.

To be honest, I am not a big fan of this alternative, as it would mean that to change the model that you use you need to reconfigure the project at the CMake level, and we are trying to support as much as possible also the use case of consuming this models from binaries.

@gabrielenava
Copy link

gabrielenava commented Mar 13, 2020

To be honest, I am not a big fan of this alternative, as it would mean that to change the model that you use you need to reconfigure the project at the CMake level, and we are trying to support as much as possible also the use case of consuming this models from binaries.

I think if we add the fixed models to icub-models we'll not have any conflict, the only not so nice effect will be that the number of models spread between the three repos (icub-gazebo, icub-gazebo-wholebody, icub-models) will increase. But at the Gazebo level, if all the required paths are installed through the robotology-superbuild, it is not an issue to handle the different models location.

So it is a go for me :-)

@S-Dafarra
Copy link
Contributor Author

S-Dafarra commented Mar 13, 2020

Mhh, note that can be more tricky as if I recall correctly you need to hardcode the height of the waist in the fixed model sdf to correctly write the constraints on the feet.

Ah right. Does it make sense for it to touch the ground anyway?

@traversaro
Copy link
Member

Mhh, note that can be more tricky as if I recall correctly you need to hardcode the height of the waist in the fixed model sdf to correctly write the constraints on the feet.

Ah right. Does it make for it to touch the ground anyway?

Yes, it is done to make sure that the feet are fixed, but in a way that they are aligned with the fixed ground of Gazebo. We can also just fix them on mid air.

@S-Dafarra
Copy link
Contributor Author

Yes, it is done to make sure that the feet are fixed, but in a way that they are aligned with the fixed ground of Gazebo. We can also just fix them on mid air.

What if I avoid specifying again the pose? Does it take the one of the original model? If so, it would be enough to add the fixed links 🤔

@S-Dafarra
Copy link
Contributor Author

That (no hands) is not ideal, but I see that you got it from the existing model.config.in, so if we remove it we will remove it from both.

I moved most of the logic directly inside the CMakeLists so that the other confing.in file was not necessary. In this way, we would have to remove the (no hands) from a single place.

What if I avoid specifying again the pose? Does it take the one of the original model?

The answer is yes, it takes the one from the urdf. So I also added the "feet_fixed" models. I noted anyway that the iCubGazeboV2_5_plus starts a few mm above the ground. I did not attempt to fix this.

@S-Dafarra
Copy link
Contributor Author

I think this is ready to be merged (even if I cannot).

@traversaro
Copy link
Member

Perfect, thanks @S-Dafarra !

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.

None yet

3 participants