-
Notifications
You must be signed in to change notification settings - Fork 48
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
Add configurationOverride Plugin #401
Conversation
Can you bump the patch version ( https://github.com/robotology/gazebo-yarp-plugins/blob/master/CMakeLists.txt#L11 ) to 101? See https://github.com/robotology/yarp/blob/master/.github/CONTRIBUTING.md#example for the rationale. |
f3819bd
to
2b1c6e6
Compare
overriden_plugin->AddElementDescription(new_element); | ||
overriden_element = overriden_plugin->GetElement(new_element->GetName()); | ||
|
||
yInfo() << "ConfigurationOverride : " << new_element->GetName() << " has been created in " << overriden_plugin_name; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that the existing plugin are already quite verbose in their output, but to be honest I am not a big fan of also adding this: the loading of complex models such as iCub
are already quite verbose (to the point that if an error occurs is almost impossible to catch it) and this will make the situation even works. @lrapetti what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we imagine using the plugin to change the initial configuration for all the control boards, I agree it would be better avoid having those yInfo()
. I can remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is indeed how it will be used in models such as icub_on_chair
. It can also be made optional based on some option, but the important thing for me is that it is not on by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will just remove them, I don't think it's worth to add special cases with the print option.
7e7740c
to
1753bcd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor comment, good to go for me. Waiting for @gabrielenava feedback on the docs.
Add the ConfigurationOverride plugin to the model sdf before the overriden plugin declaration (i.e. before the `<include>`) and define the `plugin_name` as the `name` of the plugin that will be modified. | ||
The properties of the ConfigurationOverride plugin will be then used in order to override the properties of the desired plugin. | ||
|
||
e.g. For changing the `<initialConfiguration>` of the `ControlBoard` (`controlboard_right_arm`), the model will be defined as follow: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: Note that gazebo-yarp-plugins
is completly independent from icub-gazedo
or icub-models
, so perhaps is is more clear if you explicitly say start the sentence as : "e.g. If you are using the icub gazebo model from icub-models
, and you want to change the <initialConfiguration>
etc etc" .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, thanks! I have changed the sentence and rebased
7086895
to
c248598
Compare
9b430b9
to
1023891
Compare
I tested the plugin for the icub on seesaw and icub standup models. In particular, these models were previously loaded by means of a <?xml version="1.0"?>
<sdf version="1.4">
<world name="default">
<!-- Light -->
<include>
<uri>model://sun</uri>
</include>
<!-- Ground Plane -->
<include>
<uri>model://ground_plane</uri>
</include>
<!-- Seesaw -->
<include>
<uri>model://seesaw</uri>
</include>
<!-- iCub above the seesaw -->
<include>
<uri>model://icub_on_seesaw</uri>
</include>
</world>
</sdf> Where the <?xml version="1.0"?>
<sdf version="1.4">
<world name="default">
<!-- Light -->
<include>
<uri>model://sun</uri>
</include>
<!-- Ground Plane -->
<include>
<uri>model://ground_plane</uri>
</include>
<!-- Seesaw -->
<include>
<uri>model://seesaw</uri>
</include>
<!-- iCub above the seesaw -->
<model name="iCub">
<plugin name='torso_configuration_override' filename='libgazebo_yarp_configurationoverride.so'>
<yarpPluginConfigurationOverride plugin_name='controlboard_torso'> </yarpPluginConfigurationOverride>
<initialConfiguration> 0 0 0.0824</initialConfiguration>
</plugin>
<plugin name='larm_configuration_override' filename='libgazebo_yarp_configurationoverride.so'>
<yarpPluginConfigurationOverride plugin_name='controlboard_left_arm_no_arm'> </yarpPluginConfigurationOverride>
<initialConfiguration>-0.6278 0.5231 0.0010 0.8727 0 0 0</initialConfiguration>
</plugin>
<plugin name='rarm_configuration_override' filename='libgazebo_yarp_configurationoverride.so'>
<yarpPluginConfigurationOverride plugin_name='controlboard_right_arm_no_arm'> </yarpPluginConfigurationOverride>
<initialConfiguration>-0.6278 0.5231 0.0010 0.8727 0 0 0</initialConfiguration>
</plugin>
<plugin name='lleg_configuration_override' filename='libgazebo_yarp_configurationoverride.so'>
<yarpPluginConfigurationOverride plugin_name='controlboard_left_leg'> </yarpPluginConfigurationOverride>
<initialConfiguration>0.2094 0.0873 0 -0.1745 0.0479 -0.0898</initialConfiguration>
</plugin>
<plugin name='rleg_configuration_override' filename='libgazebo_yarp_configurationoverride.so'>
<yarpPluginConfigurationOverride plugin_name='controlboard_right_leg'> </yarpPluginConfigurationOverride>
<initialConfiguration>0.2094 0.0873 0 -0.1745 0.0479 -0.0898</initialConfiguration>
</plugin>
<include>
<uri>model://icub</uri>
<pose>-0.05 0 0.6926 0 0 3.14</pose>
</include>
</model>
</world>
</sdf> Now, the model loaded in the world file is the original I tested the plugin on Gazebo. To be sure I was not using anymore the previous configuration I removed my INSTALL folder and I installed the iCub models again. Here are the results of running Gazebo + So the plugin is working fine. I created a devel branch in |
Thanks @lrapetti and @gabrielenava . This is a feature that deserves some recognition, we do not mantain a changelog for gazebo-yarp-plugins unfortunatly (any volunteer for doing that is highly welcome : ) ) but we should remember to mention this when we will announce the next release. |
Great! Thanks @gabrielenava and @traversaro |
Accordingly to what discussed in #392, with this PR I am adding a plugin that enables to modify the value of properties of another plugin attached to the same
model
.The plugin was initially thought to set the initial position of a model from a world or from a wrapping model, without the need of generating multiple
sdf
with the same model just to change some parameters (e.g.icub
andicub_standup
).The details on the use of the plugin are contained in the
README.MD
.