search

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

Parametrize the YAML ? #235

Closed Nicogene closed 4 months ago

Nicogene commented 5 months ago

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

traversaro commented 5 months ago

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

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

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

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

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

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

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

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

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

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

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

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: https://github.com/jbeder/yaml-cpp/issues/60 .

Nicogene commented 4 months ago

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

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

Here my first proposal of refactoring: