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

CMakeLists.txt cluttered with option parameters #84

Open
nunoguedelha opened this issue Jan 5, 2018 · 5 comments
Open

CMakeLists.txt cluttered with option parameters #84

nunoguedelha opened this issue Jan 5, 2018 · 5 comments

Comments

@nunoguedelha
Copy link
Contributor

@fiorisi @traversaro , as a general comment: you must have notived that the cmake files are starting to get cluttered and less readable after every change of this type (#83). I suggest 2 possible solutions:

  1. include all the alternate parameters in the yaml.in template file and use cmake to set a configuration switch in the same template. For instance if we consider assignedInertias, we include the parameters in the yaml.in template, along with the definition:
    increase_inertia: @INCREASE_INERTIA_FOR_GAZEBO@ ("true" or "false")

We set @INCREASE_INERTIA_FOR_GAZEBO@ through cmake. Then the processing of increasing the inertia accordingly would be done in the script firstgen.py.

  1. Use git to define the specific yaml.in template files. We would use a different master branch for each model (master_icubgenova02, master_icubgenova04, ...). The macro generate_icub_simmechanics would then checkout the desired branch through cmake, after the selected options NO_BACKPACK, BOGUS, INCREASE_INERTIA_FOR_GAZEBO, ICUB_PLUS. The immediate advantage would be the easier comparison between models, but there are some drawbacks to consider that I haven't been through yet...
@nunoguedelha
Copy link
Contributor Author

CC @diegoferigo

@fiorisi
Copy link
Member

fiorisi commented Jan 5, 2018

Yes, Probably we can modify the simmechanics-to-urdf tool to handle different robot versions. In this case, we could store all the parameters in the *.yaml file and remove the configurations from the CMake file.

@traversaro
Copy link
Member

As a personal opinion, I am definitely against option 2.
Option 2 has a lot of drawbacks:

  • No one expects the build system to mess with the git state of repository in which the build files are contained.
  • Propagating changes that belong to all the robots would become much more difficult.
  • How to handle pull requests? As multiple pull requests over all the branches?
  • My spider-sense tell me that this is dangerous (ok, probably this is not objective point :D ).

For option 1, I understand the problem but I need to think about it.

@traversaro
Copy link
Member

As a general option, I would prefer to leak icub-generation specific stuff to simmechanics-to-urdf, but indeed increasing the inertia for gazebo is kind of a generic problem.

@fiorisi
Copy link
Member

fiorisi commented Jan 6, 2018

What if simmechanics-to-urdf could parse more than one *.yaml file? In this case we could have one for common parameters and one for each robot. Maybe the same could be done for *.csv files.

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

No branches or pull requests

3 participants