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

Parametrize the YAML ? #235

Closed
Nicogene opened this issue Apr 5, 2024 · 15 comments · Fixed by icub-tech-iit/creo2urdf#91 or #240
Closed

Parametrize the YAML ? #235

Nicogene opened this issue Apr 5, 2024 · 15 comments · Fixed by icub-tech-iit/creo2urdf#91 or #240
Assignees
Labels
domain-software Related to Software prj-ergocub Related to ErgoCub Project team-fix Related to Team Fix

Comments

@Nicogene
Copy link
Member

Nicogene commented Apr 5, 2024

Task description

In icub-models-generator the yaml are actually yaml.in that via cmake fill some placeholders in order to create the configuration files for more versions of iCub.

Now that the generation of the urdf may be automatize via creo2urdf:

Maybe it makes sense to start use also here the yaml.in?

This would help for example in:

Since the only difference will be in the weight of the head and its inertia.

Definition of Done

We built the cmake infrastructure for handling the yaml.in and the repo has been refactored

cc @xela-95 @traversaro @pattacini

@Nicogene Nicogene added domain-software Related to Software prj-ergocub Related to ErgoCub Project team-fix Related to Team Fix labels Apr 5, 2024
@Nicogene Nicogene self-assigned this Apr 5, 2024
@traversaro
Copy link
Member

Totally agree, but I am not sure if cmake is the right tool for this. I have an idea, I will write it as soon I am at the laptop.

@traversaro
Copy link
Member

Ok, I looked into how ergocub-software is using this functionality. It seems to me that different variants of the robot only add options to the base .yaml file, so a possible basic implementation is just to have a base .yaml file use for all models, and some "additional" .yaml for all the specialized models with the additional options. These .yaml files can be merged at the file level (just concatenate the two files before feeding them to creo2urdf), or directly at file or .yaml level inside creo2urdf.

If instead for any reason we want to go for templating the .json files, I would look for regular template engine such as jinja2 (for C++ or Python) or mustache, instead of using CMake as a template engine.

@Nicogene
Copy link
Member Author

Nicogene commented Apr 15, 2024

Ok, I looked into how ergocub-software is using this functionality. It seems to me that different variants of the robot only add options to the base .yaml file, so a possible basic implementation is just to have a base .yaml file use for all models, and some "additional" .yaml for all the specialized models with the additional options. These .yaml files can be merged at the file level (just concatenate the two files before feeding them to creo2urdf), or directly at file or .yaml level inside creo2urdf.

Today I start looking this option, in paricular I found this interesting utility for merging/managing yaml files:

I started playing around with it, I created to "addons" yaml, the gazebo_addon.yaml and the minContact_addon.yaml.

These two yaml contains ONLY the assignedInertias and assignedCollisionGeom respectively

And with this command:

yq eval-all '. as $item ireduce ({}; . *+ $item )' ERGOCUB_all_options.yaml gazebo_addon.yaml > ERGOCUB_all_options_gazebo.yaml

I obtained this file:

The idea is that for obtaining the ERGOCUB_all_options_minContacts we should:

yq eval-all '. as $item ireduce ({}; . *+ $item )' ERGOCUB_all_options_gazebo.yaml minContacts_addon.yaml > ERGOCUB_all_options_minContacs.yaml

for obtaining this other file:

In this way the information should not be duplicated

In this way, we concatenate both the "addons".

I have also struggled a little bit to add custom commands in the CMakeLists in order to generate all the yaml files when generating, my idea was to create a macro that functions of the version of the robot (1_0, 1_1) invoke those command, for now I don't have nothing that works but I will keep trying

cc @traversaro @pattacini

@traversaro
Copy link
Member

I have also struggled a little bit to add custom commands in the CMakeLists in order to generate all the yaml files when generating, my idea was to create a macro that functions of the version of the robot (1_0, 1_1) invoke those command, for now I don't have nothing that works but I will keep trying

I am probably missing why we need to use CMake for the generation of the model. If I am not wrong, the invocation of creo2urdf is not using CMake, but raw ps1 scripts, so probably we can just add the call to yq in the powershell script, right?

@pattacini
Copy link
Member

I can confirm that yq is a quite good tool. I've already used it in the past for https://github.com/icub-tech-iit/gh-action-repo-selective-sync/.

@Nicogene
Copy link
Member Author

Nicogene commented Apr 15, 2024

I have also struggled a little bit to add custom commands in the CMakeLists in order to generate all the yaml files when generating, my idea was to create a macro that functions of the version of the robot (1_0, 1_1) invoke those command, for now I don't have nothing that works but I will keep trying

I am probably missing why we need to use CMake for the generation of the model. If I am not wrong, the invocation of creo2urdf is not using CMake, but raw ps1 scripts, so probably we can just add the call to yq in the powershell script, right?

My idea is to still keep the possibility to generate the ergocub-software urdf launching creo2urdf with the GUI, then I was thinking about adding a custom command in cmake for generating the all the yamls that can be used, alternatively I could create a ps script for just generate the yaml needed by creo2urdf

@traversaro
Copy link
Member

My idea is to still keep the possibility to generate the ergocub-software urdf launching creo2urdf with the GUI, then I was thinking about adding a custom command in cmake for generating the all the yamls that can be used, alternatively I could create a ps script for just generate the yaml needed by creo2urdf

Can't we generate the .yaml as part of the creo2urdf generation process? I imagine that anyone touching those file is interested in testing them by generating the URDF via creo2urdf, so I am not sure if there is any benefit in splitting the generation process in different manual steps.

@Nicogene
Copy link
Member Author

After a T2T chat, we agreed that passing multiple yamls to creo2urdf can make the process cumbersome while using it via GUI, we thought that it would be nice that the entry point remains one yaml, that yaml can be "complete" as they are now or it can include more "sub-yamls".
Unfortunately, the include is not supported by the yaml format, but we can implement it in creo2urdf (cc @mfussi66):

@Nicogene
Copy link
Member Author

Nicogene commented Apr 17, 2024

I managed to load multiple yaml files and merge them all, now the yaml are:

ERGOCUB_all_options.yaml

includes:[base.yaml]

ERGOCUB_all_options_gazebo.yaml

includes:[base.yaml, gazebo_addon.yaml]

ERGOCUB_all_options_minContacts.yaml

includes:[base.yaml, gazebo_addon.yaml, minContacts_addon.yaml]

I wrote on a file the result of merging it the resulting yaml contains all the information, the only problem is that when in the included yaml some parameters are repeated, i.e. assignedInertias is both in base.yaml and gazebo_addons.yaml, in fact there are multiple assignedInertias in the resulting yaml, but when reading it only the first is taken in account.

We need to think about how to manage when a parameter is defined multiple times

@Nicogene
Copy link
Member Author

Nicogene commented Apr 17, 2024

ChatGPT says that an yaml like this

fruits:
  - apple
  - banana
  - orange
fruits: pineapple

If parsed with yaml-cpp result as follows:

Multiple occurrences of key 'fruits':
- apple
- banana
- orange
- pineapple

But from what I am experiencing in creo2urdf I am getting instead this behavior:

fruits:
  - apple
  - banana
  - orange

cc @mfussi66 @traversaro

@traversaro
Copy link
Member

Are you sure that the behaviour in presence of duplicate keys is described in the yaml spec and not implementation defined? If it is the latter, then this may be an implementation detail of yaml-cpp.

For dealing with duplicate keys, probably we can just iterate on the keys and merge them if duplicate? Is that enough or we need also to ensure that yaml elements that are grandchild of the root need to be merged?

@traversaro
Copy link
Member

Are you sure that the behaviour in presence of duplicate keys is described in the yaml spec and not implementation defined? If it is the latter, then this may be an implementation detail of yaml-cpp.

It seems this behaviour is a yaml-cpp issue: jbeder/yaml-cpp#60 .

@Nicogene
Copy link
Member Author

Are you sure that the behaviour in presence of duplicate keys is described in the yaml spec and not implementation defined? If it is the latter, then this may be an implementation detail of yaml-cpp.

I am pretty sure that ChatGPT is guessing but it is completely wrong :D And yes it seems that duplication of keys is allowed by yaml-cpp but not by the standard.

For dealing with duplicate keys, probably we can just iterate on the keys and merge them if duplicate. Is that enough or we need also to ensure that yaml elements that are grandchild of the root need to be merged?

I found several snippets that claimed to merge yamls but none is doing what we need, for now, just jq is correctly merging the yamls. I could look into their code

@Nicogene
Copy link
Member Author

In:

I added the possibility to specify yamls to be included, it works both with "old-style" yamls and "composite" yamls, I tested it converting the ergocub urdfs and there were no difference with the ones generated with the "old-style" ones.

We should agree on how using this functionality, is it ok this structure?

cc @pattacini @traversaro

@Nicogene
Copy link
Member Author

Here my first proposal of refactoring:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain-software Related to Software prj-ergocub Related to ErgoCub Project team-fix Related to Team Fix
Projects
None yet
3 participants