icub-tech-iit / ergocub-software

Main collector of ergoCub specific SW
https://icub-tech-iit.github.io/ergocub-software/
BSD 3-Clause "New" or "Revised" License
13 stars 18 forks source link

Add Gazebo Sim support #230

Closed xela-95 closed 5 months ago

xela-95 commented 5 months ago

This PR introduces the support to the new Gazebo.

Reference issue: https://github.com/robotology/gz-sim-yarp-plugins/issues/130

Tools

Tool Version
CREO 9.0.8.0
creo2urdf v0.4.6 installed from binaries
cad-mechanics 5d31e
cad-libraries 851b1
xela-95 commented 5 months ago

CC @traversaro

xela-95 commented 5 months ago

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

As potential alternatives, I see:

@traversaro @Nicogene what do you think?

traversaro commented 5 months ago

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 https://github.com/gazebosim/sdformat/issues/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 commented 5 months ago

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 commented 5 months ago

Ok thanks! I will proceed as suggested then ✌🏻

xela-95 commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

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):

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

traversaro commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 (https://github.com/icub-tech-iit/ergocub-software/pull/226), perhaps @Nicogene remember which cad-mechanics checkout was used. For the other models https://github.com/icub-tech-iit/ergocub-software/pull/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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

perhaps @Nicogene remember which cad-mechanics checkout was used. For the other models https://github.com/icub-tech-iit/ergocub-software/pull/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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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

Nicogene commented 5 months ago

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 commented 5 months ago

Thanks!

xela-95 commented 5 months ago

Ok now the PR is ready for the review :)

xela-95 commented 5 months ago

Rebased on master

traversaro commented 5 months ago

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: