Add option for simple configuration for the plugins

[traversaro]

For sufficiently simple use the plugin should be configurable just from the sdf, without the need of a .ini file.

[traversaro]

Just realized how we can solve this issue in two lines of code. We normally load yarp-specific configuration files using the following syntax:

<yarpConfigurationFile>model://icub/conf/FT/gazebo_icub_left_leg_ft.ini</yarpConfigurationFile>

where the content of the tag is the path that is passed to the yarp::os::Property::fromConfigFile method.

We can add as an alternative option this tag:

<yarpConfigurationString>"(period 0.01)"</yarpConfigurationString> 

that passes its content directly to yarp::os::Property::fromString`.

[lrapetti]

@traversaro
Given that I may need this feature (see #393) I could take over this task, but I would like to discuss first what would be the relationship between yarpConfigurationFile and yarpConfigurationString:

• The two element can coexist and the properties is the result of combining the properties from the file and from the string (what to do in case a property is defined in both? Which one would have the priority?)
• The two element can coexist, but the properties are read only from one of the two (Which one would have the priority).
• Fail if the two properties coexist

(Probably this should be decided having in view the possibility of overriding a property as in #392)

[traversaro]

For a single plugin, I would say that the best option is that the two properties cannot coexist, i.e. an error is printed and the plugins does not start. When overriding, by idea was something like: Only yarpConfigurationString is permitted during overriding, and the loading is structured in this way (similar to https://github.com/robotology/yarp/blob/19cdb60478371d4cba8b9889c5f582d6bb5e6fc7/src/libYARP_OS/src/ResourceFinder.cpp#L204):
first yarpConfigurationFile is passed to yarp::os::Property::fromConfigFile, and then the overriding yarpConfigurationString using the fromString(.. , false) to just override the contained parameters (or perhaps even better using the fromCommand(..., false) that is what is used for parameter overriding in RFModules, so it is already extensively tested, and I guess the syntax --param1 1.0 is more intuitive then (param1 1.0).

I do not how much this is compatible with the libgazebo_yarp_configurationoverride.so plugin idea outline in #392 , while it could be doable modifying the plugin source code.

This is related also to #392 : we should make sure that all the plugins use this helper functions to load their conf files :

gazebo-yarp-plugins/libraries/singleton/src/ConfHelpers.cc

Line 59 in 3fa2c45

bool loadConfigModelPlugin(physics::ModelPtr _model,

so you will need to do all this modifications only once.

[lrapetti]

@traversaro

I would say that the best option is that the two properties cannot coexist, i.e. an error is printed and the plugins does not start. When overriding, by idea was something like: Only yarpConfigurationString is permitted during overriding

The reason why I was thinking at coexisting option is that for example we may need to just modify the gazeboYarpPluginsRobotName property of a control board, without redefining all the control board properties defined in the .ini (see an example of configuration file loaded by a control board plugin: gazebo_icub_left_hand).

(Anyway probably a further discussion may be needed on how the gazeboYarpPluginsRobotName is defined and used)

or perhaps even better using the fromCommand(..., false) that is what is used for parameter overriding in RFModules, so it is already extensively tested, and I guess the syntax --param1 1.0 is more intuitive then (param1 1.0)

Is this sintax able to cope with groups (nested properties)? (And in case NOT, do we need it?)

[traversaro]

The reason why I was thinking at coexisting option is that for example we may need to just modify (..)

Good point, I think the best option is that you look into it and come up with proposal from the insight you got.

Is this sintax able to cope with groups (nested properties)? (And in case NOT, do we need it?)

I am afraid it is not documented, but yes, in this way: --GROUP::param paramValue.

[lrapetti]

Good point, I think the best option is that you look into it and come up with proposal from the insight you got.

From my point of view the coexistence should be allowed so that:

• If there is no intersection between the properties passed using yarpConfigurationFile and yarpConfigurationString, they will both be add to plugin_parameters.
• For the intersecting parameters, the possibility are:

  1. Priority depending on the order: the property that is defined first in the sdf will be used. This would mimic the behaviour used in the yarp-gazebo-plugin for reading the properties (The plugins are currently read using GetElement(<property name>) which returns the first occurence of the property in the sdf, so if a property is defined twice only the first one gets read). The problems of this solution are:

     • dependence on the order in the sdf, that in general I think it should be avoided
     • when trying to override yarpConfigurationString or yarpConfigurationFile properties, then things get problematic since the property you are trying yo overriding may be hidden in the lower layer depending on the order they are defined.

  2. yarpConfigurationString gets the priority. This is the solution that I would like the most since we would get of annoying dependence on the property order. I think this can be justified by the fact that usually a configuration file is used to define most of the properties, and one may use a configuration string in order to just change some of them. Also in this case, when trying to override a property through a parent element, things get not easy:

     • when overriding using yarpConfigurationString it's fine.
     • when overriding using yarpConfigurationFile, some properties defined in the file may be hidden by a yarpConfigurationString in a nested model.

     But I think this situation is better then the corresponding one in case i, since here we are sure that using yarpConfigurationString the property will be overritten.

[lrapetti]

(The plugins are currently read using GetElement() which returns the first occurence of the property in the sdf, so if a property is defined twice only the first one gets read)

To explain better this point, how it currently works with a plugin is that with the following sdf:

    <plugin name='controlboard_left_arm_no_arm' filename='libgazebo_yarp_controlboard.so'>
      <yarpConfigurationFile>model://icub/conf/gazebo_icub_left_arm_no_hand_for_no_hand_model.ini</yarpConfigurationFile>
      <initialConfiguration>-0.52 0.52 0 0.785 0 0 0.698</initialConfiguration>
      <initialConfiguration>-2.0 0.52 0 0.785 0 0 0.698</initialConfiguration>
    </plugin>

The first initialConfiguration is used. This is fine because it doesn't make sense to have multiple definition of this property (Probably a WARNING could be introduced when this kind of situation happens and a configuration that should be defined once is defined multiple times).

[traversaro]

I like solution ii.

[lrapetti]

@traversaro

I like solution ii.

Me too I prefer this solution. I will try to implement it and test it in order to have a better understanding of possible issues.

I have also noticed that loadConfigModelPlugin() is almost unused (only used in jointsensors and inertialmtbPart). I will understand if there is any reason for not using, and in case I will do a PR adding it to all the plugins.

[traversaro]

I will understand if there is any reason for not using, and in case I will do a PR adding it to all the plugins.

I do not think there is any specific reason not to use it, I think it is just something that we never cleaned up.

[lrapetti]

I will understand if there is any reason for not using, and in case I will do a PR adding it to all the plugins.

I do not think there is any specific reason not to use it, I think it is just something that we never cleaned up.

PR at #394, further discussion on the use of loadConfigModelPlugin() will take place there.

[lrapetti]

PR for introducing yarpConfigurationParameter in #396

[lrapetti]

PR for introducing yarpConfigurationParameter in #396

Merged, @traversaro I think we can close this issue

[traversaro]

Yes!