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

Add Gazebo Sim support #230

Merged
merged 28 commits into from
Mar 28, 2024

Conversation

xela-95
Copy link
Contributor

@xela-95 xela-95 commented Mar 22, 2024

This PR introduces the support to the new Gazebo.

Reference issue: robotology/gz-sim-yarp-plugins#130

Tools

Tool Version
CREO 9.0.8.0
creo2urdf v0.4.6 installed from binaries
cad-mechanics 5d31e
cad-libraries 851b1

@xela-95
Copy link
Contributor Author

xela-95 commented Mar 22, 2024

CC @traversaro

@xela-95
Copy link
Contributor Author

xela-95 commented Mar 25, 2024

I'm thinking about where I should put the gz-sim-yarp-plugins.

As potential alternatives, I see:

  • Add them to the existing ergoCub yaml files ERGOCUB_all_options_gazebo and ERGOCUB_all_options_minContacts with the pro of having the same number of output models and the con of having error logs both in gazebo-classic and gazebo since some of the plugins will not be found (I'm currently testing that these errors do not cause the simulation to stop);
  • Generate a new set of ergoCub models only for the new Gazebo: in this case the existing models are left untouched and we do not have misleading errors logged by Gazebo, with the burden of having more files to maintain in the long period.

@traversaro @Nicogene what do you think?

@traversaro
Copy link
Member

I would go for the first option. I know that error may be confusing, but the explosion of models seems even worse. We can work with upstream to minimize the effect of this errors (see gazebosim/sdformat#93) in the long run, but for now I would just keep one set of models, let's see what @Nicogene thinks about this.

@Nicogene
Copy link
Member

I would go for the first option. I know that error may be confusing, but the explosion of models seems even worse. We can work with upstream to minimize the effect of this errors (see gazebosim/sdformat#93) in the long run, but for now I would just keep one set of models, let's see what @Nicogene thinks about this.

I totally agree with it also because right now the models are generated by hand, and it would be complicated to augment the number of generated models

@xela-95
Copy link
Contributor Author

xela-95 commented Mar 25, 2024

Ok thanks! I will proceed as suggested then ✌🏻

@xela-95
Copy link
Contributor Author

xela-95 commented Mar 25, 2024

After a long day of trial & error, ergoCubGazeboV1_1 works both on gazebo-classic and gazebo!!! 🥳

Now it's just a matter of porting these modifications to the other models.

@xela-95
Copy link
Contributor Author

xela-95 commented Mar 27, 2024

@traversaro looking at the table in https://github.com/icub-tech-iit/ergocub-software/blob/4a7ce3a1472b16c7a3372623199c3ccbaa761a3d/urdf/creo2urdf/data/ergocub1_1/README.md, there should be for each version of ergoCub a different pair of .yml and .csv files, while now there is only a single .csv: ERGOCUB_joint_all_parameters.csv. Is this correct?

@traversaro
Copy link
Member

@traversaro looking at the table in https://github.com/icub-tech-iit/ergocub-software/blob/4a7ce3a1472b16c7a3372623199c3ccbaa761a3d/urdf/creo2urdf/data/ergocub1_1/README.md, there should be for each version of ergoCub a different pair of .yml and .csv files, while now there is only a single .csv: ERGOCUB_joint_all_parameters.csv. Is this correct?

Yes, I think that was just a typo, feel free to correct.

@xela-95
Copy link
Contributor Author

xela-95 commented Mar 27, 2024

I finished porting the main changes both to version 0 and 1 of ergoCub. I'm having some issue while trying to import the models of ergoCub version 0 (the ones generated from the yaml files in https://github.com/xela-95/ergocub-software/tree/feature/add-gz-sim-support/urdf/creo2urdf/data/ergocub1_0). It is also strange that the URDF generated with CREO is a lot different than the previous version, while for the models of version 1 the diff in the urdf are just the ones I added in the yaml.

@traversaro
Copy link
Member

It is also strange that the URDF generated with CREO is a lot different than the previous version, while for the models of version 1 the diff in the urdf are just the ones I added in the yaml.

I guess this small numeric errors (they seems all to be <= 1e-10 can be due):

  • Minor changes in creo2urdf dependencies
  • Minor changes in creo version
  • Minor changes in version of cad-mechanics used

Anyhow, unless we start locking all of these variables (that at the moment is not feasible, I guess) we can safely ignore them.

@traversaro
Copy link
Member

I'm having some issue while trying to import the models of ergoCub version 0 (the ones generated from the yaml files in https://github.com/xela-95/ergocub-software/tree/feature/add-gz-sim-support/urdf/creo2urdf/data/ergocub1_0).

Which errors? What I noticed is that you also added the controlboard of the hands, but those require coupling that is currently not supported in gz-sim-yarp-plugins, is that intentional?

@traversaro
Copy link
Member

Anyhow, unless we start locking all of these variables (that at the moment is not feasible, I guess) we can safely ignore them.

Note that something that we can easily is at least to document in each PR that changes the models the version numbers of Creo, creo2urdf (reporting if compiled from sources or if the .dll from the release is used) and the checkout of cad-mechanics and cad-libraries used.

@traversaro
Copy link
Member

By the way, there are also some tests failures in the tests:

 Test project /home/runner/work/ergocub-software/ergocub-software/build
    Start 1: ergoCubGazeboV1ConsistencyCheck
1/4 Test #1: ergoCubGazeboV1ConsistencyCheck .....***Failed    0.02 sec
ergocub-model-test error:r_index_dist got direction of  x -0.947223 y -0.290266 z -0.136064 instead of expected  x 0.947223 y 0.290266 z 0.136064

    Start 2: ergoCubGazeboV1_1ConsistencyCheck
2/4 Test #2: ergoCubGazeboV1_1ConsistencyCheck ...   Passed    0.03 sec
    Start 3: ergoCubSN000ConsistencyCheck
3/4 Test #3: ergoCubSN000ConsistencyCheck ........***Failed    0.03 sec
ergocub-model-test error:r_index_dist got direction of  x -0.947223 y -0.290266 z -0.136064 instead of expected  x 0.947223 y 0.290266 z 0.136064

    Start 4: ergoCubSN001ConsistencyCheck
4/4 Test #4: ergoCubSN001ConsistencyCheck ........   Passed    0.03 sec

That is quite strange, it seems like the kind of errors we had with simmechanics-to-urdf, where the direction of the axis was randomly changing. Anyhow, given the problems with the ergoCubSN000 and ergoCubGazeboV1, probably we can drop them from the first PR?

@xela-95
Copy link
Contributor Author

xela-95 commented Mar 28, 2024

It is also strange that the URDF generated with CREO is a lot different than the previous version, while for the models of version 1 the diff in the urdf are just the ones I added in the yaml.

I guess this small numeric errors (they seems all to be <= 1e-10 can be due):

* Minor changes in creo2urdf dependencies

* Minor changes in creo version

* Minor changes in version of cad-mechanics used

Anyhow, unless we start locking all of these variables (that at the moment is not feasible, I guess) we can safely ignore them.

Yes I agree for the numeric errors. But other than that there are other modifications that I'm investigating, like the name of the .stl files have been changed, and also material and collision attributes.

@xela-95
Copy link
Contributor Author

xela-95 commented Mar 28, 2024

I'm having some issue while trying to import the models of ergoCub version 0 (the ones generated from the yaml files in https://github.com/xela-95/ergocub-software/tree/feature/add-gz-sim-support/urdf/creo2urdf/data/ergocub1_0).

Which errors? What I noticed is that you also added the controlboard of the hands, but those require coupling that is currently not supported in gz-sim-yarp-plugins, is that intentional?

Sorry I added that without thinking to the missing coupling features, I will remove them to all models then.

Maybe it's also good to open an issue on gz-sim-yarp-plugins to track the plugins that ergocub-software currently misses.

@traversaro
Copy link
Member

Yes I agree for the numeric errors. But other than that there are other modifications that I'm investigating, like the name of the .stl files have been changed, and also material and collision attributes.

Ack, I guess you did not committed that? It is possible that the cad-mechanics model was changed, but the change was not propagated to the repo. It is happening for both SN000 and SN001 or just SN000? If it is just SN001, I would just drop it. To decouple the problems of "updating cad-mechanics" from the core of this, one possible strategy is just to use the cad-mechanics commit that was used to generate the version of the models currently present in the repo. For ergoCubSN001/ergoCubGazeboV1_1* is is quite recent (#226), perhaps @Nicogene remember which cad-mechanics checkout was used. For the other models #220 it was older, perhaps @Nicogene has some hint of the cad-mechanics source models used, but as I mentioned I would drop ergoCubSN000/ergoCubGazeboV1 from the initial PR if they are giving problems.

@xela-95
Copy link
Contributor Author

xela-95 commented Mar 28, 2024

Ack, I guess you did not committed that? It is possible that the cad-mechanics model was changed, but the change was not propagated to the repo. It is happening for both SN000 and SN001 or just SN000? If it is just SN001, I would just drop it. To decouple the problems of "updating cad-mechanics" from the core of this, one possible strategy is just to use the cad-mechanics commit that was used to generate the version of the models currently present in the repo. For ergoCubSN001/ergoCubGazeboV1_1* is is quite recent (#226), perhaps @Nicogene remember which cad-mechanics checkout was used. For the other models #220 it was older, perhaps @Nicogene has some hint of the cad-mechanics source models used, but as I mentioned I would drop ergoCubSN000/ergoCubGazeboV1 from the initial PR if they are giving problems.

The models that change significantly are the three coming from the ergoCub1_0 folder.

Up to now I always discarded the meshes that have been generated from CREO, committing only the urdf files, because I though the meshes did not change, but for the models in the ergoCub1_0 folder the names of the mesh files are different.

@traversaro
Copy link
Member

The models that change significantly are the three coming from the ergoCub1_0 folder.

Perfect, I would discard them for now. Those are also the one for which tests are failing.

@xela-95
Copy link
Contributor Author

xela-95 commented Mar 28, 2024

The models that change significantly are the three coming from the ergoCub1_0 folder.

Perfect, I would discard them for now. Those are also the one for which tests are failing.

OK! also with @Nicogene we agreed that I will maintain the modifications to the yaml files in case in future someone will need to generate new urdf versions, but for this PR I will discard them.

@Nicogene
Copy link
Member

perhaps @Nicogene remember which cad-mechanics checkout was used. For the other models #220 it was older, perhaps @Nicogene has some hint of the cad-mechanics source models used, but as I mentioned I would drop ergoCubSN000/ergoCubGazeboV1 from the initial PR if they are giving problems.

It may be dfedc37d82224bc627083e0fc0fb9e17d7bcf8ae but not 100% sure, I don't update cad-mechanics frequently

That is quite strange, it seems like the kind of errors we had with simmechanics-to-urdf, where the direction of the axis was randomly changing. Anyhow, given the problems with the ergoCubSN000 and ergoCubGazeboV1, probably we can drop them from the first PR?

About this point I aligned w/ @xela-95 T2T, the urdf of ergoCub 1.0/SN000 has never been generated with creo2urdf, in the past it has been generated w/ simmechanics_to_urdf doing the procedure CREO9->CREO7 for the issues we know, and since it had a high-cost fix on CAD side we used to do patches via software:

All this time I have been keeping up-to-date the yaml to be usable when we will regenerate the model via creo2urdf, but for sure before it required fixes on simulation model CAD that right now we do not have the workforce to do.

So I suggested to @xela-95 to commit also the changes to the 1.0/SN000 yaml and add by hand the blobs to the 1.0/SN000 urdf.
The errors you are getting are probably related to some wrong reverseRotationAxis.

cc @maggia80 @pattacini

@traversaro
Copy link
Member

traversaro commented Mar 28, 2024

About this point I aligned w/ @xela-95 T2T, the urdf of ergoCub 1.0/SN000 has never been generated with creo2urdf, in the past it has been generated w/ simmechanics_to_urdf doing the procedure CREO9->CREO7 for the issues we know, and since it had a high-cost fix on CAD side we used to do patches via software:

*

Can you add this information in https://github.com/icub-tech-iit/ergocub-software/tree/master/urdf/creo2urdf/data/ergocub1_0#readme or https://github.com/icub-tech-iit/ergocub-software/tree/master?tab=readme-ov-file#urdf-generation ? Nothing too complex, just to avoid that in the future people loose time to try to use creo2urdf with ergocub1_0 . Personally I lost track of this, and it may be helpful for future users to actually document how the ergocub1_0 models are actually generated.

@xela-95
Copy link
Contributor Author

xela-95 commented Mar 28, 2024

Note that something that we can easily is at least to document in each PR that changes the models the version numbers of Creo, creo2urdf (reporting if compiled from sources or if the .dll from the release is used) and the checkout of cad-mechanics and cad-libraries used.

Added tool versions

@Nicogene
Copy link
Member

@xela-95 @traversaro I have added a Pull request template but I am not sure it works:

@Nicogene
Copy link
Member

About this point I aligned w/ @xela-95 T2T, the urdf of ergoCub 1.0/SN000 has never been generated with creo2urdf, in the past it has been generated w/ simmechanics_to_urdf doing the procedure CREO9->CREO7 for the issues we know, and since it had a high-cost fix on CAD side we used to do patches via software:

*

Can you add this information in master/urdf/creo2urdf/data/ergocub1_0#readme or master?tab=readme-ov-file#urdf-generation ? Nothing too complex, just to avoid that in the future people loose time to try to use creo2urdf with ergocub1_0 . Personally I lost track of this, and it may be helpful for future users to actually document how the ergocub1_0 models are actually generated.

I added this info in the relative README

@traversaro
Copy link
Member

Thanks!

@xela-95
Copy link
Contributor Author

xela-95 commented Mar 28, 2024

Ok now the PR is ready for the review :)

@xela-95 xela-95 marked this pull request as ready for review March 28, 2024 13:16
@xela-95
Copy link
Contributor Author

xela-95 commented Mar 28, 2024

Rebased on master

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 a lot @xela-95 !

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.

I didn't have the chance to test it but if this does not interfere with gazebo classic we can merge it

@Nicogene Nicogene merged commit 4940220 into icub-tech-iit:master Mar 28, 2024
4 checks passed
@traversaro
Copy link
Member

I didn't have the chance to test it but if this does not interfere with gazebo classic we can merge it

We are in process of adding support in robotology-superbuild and conda-forge for gz-sim-yarp-plugins, once it is ready it should be easy to check:

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.

3 participants