search

robotology / cer-sim

Official URDF and SDF models of the R1 humanoid robot.
14 stars 9 forks source link

Explicitly set the VELOCITY_CONTROL::velocityControlImplementationType parameter to ensure compatibility with gazebo-yarp-plugins 4 #29

Closed traversaro closed 2 years ago

traversaro commented 2 years ago

In https://github.com/robotology/gazebo-yarp-plugins/pull/514 (changelog entry: https://github.com/robotology/gazebo-yarp-plugins/blob/master/CHANGELOG.md#351---2020-10-05), the VELOCITY_CONTROL::velocityControlImplementationType parameter was added to the gazebo_yarp_control plugin to permit the users to switch between velocity control implemented between the direct_velocity_pid implementation (the default used until then) and the integrator_and_position_pid implementation (that is not a velocity-based PID, but rather an integrator in series with the classical position control).

The parameter was added as optional in gazebo-yarp-plugins 3.5.1, but to avoid confusion it will be make compulsory in gazebo-yarp-plugins 4 . To avoid creating runtime error in gazebo-yarp-plugins 4, this PR adds the parameter to the configuration files in this repo, using the default value direct_velocity_pid. However, mantainers may want to switch to use integrator_and_position_pid` it if makes more sense in the future.

Related issues: https://github.com/robotology/gazebo-yarp-plugins/issues/577, https://github.com/robotology/gazebo-yarp-plugins/pull/578 .

traversaro commented 2 years ago

fyi @randaz81 @ste93

drdanz commented 2 years ago

@traversaro, @randaz81 Perhaps this should be on the devel branch?

drdanz commented 2 years ago

According to @randaz81 this should be on both branches. Merging...

drdanz commented 2 years ago

Thanks @traversaro

traversaro commented 2 years ago

@traversaro, @randaz81 Perhaps this should be on the devel branch?

As discussed with @randaz81, this is a feature that is deprecated since gazebo-yarp-plugins v3.5.1 (released ~1 year ago) so for those version of gazebo-yarp-plugins it will be remove a warning. For previous version of gazebo-yarp-plugins the additional parameter is ignored, so I think it is safer to just have it in master has it just fixes compatibility with gazebo-yarp-plugins 4.0.0 without creating problem with other versions.