Handle yarpDeviceName in controlboard and multicamera plugins
#559
Conversation
drdanz
force-pushed
the
yarpDeviceName
branch
2 times, most recently
from
d39b6ba
to
c57940f
Compare
|
Could you update the docs in https://github.com/robotology/gazebo-yarp-plugins/blob/40ef333672b9cc85cdf50255d229b6bb462237cb/plugins/robotinterface/README.md#how-to-specify-existing-yarp-devices-to-which-to-attach with the changes w.r.t. the use of |
| return; | ||
| } | ||
| yInfo() << "GazeboYarpControlBoard: Registered YARP device with instance name:" << scopedDeviceName; | ||
| } |
There was a problem hiding this comment.
Sorry, before reading the documentation I did not understood that this was breaking the previous pattern of specifying the YARP instance name with networks. This would break all the existing non-gazebo_yarp_robotinterface use of the device map contained in the GazeboYarpPlugins::Handler. In particular, this would break all the existing gazebo_yarp_controlboard plugins that access existing devices via gazebo_yarp_controlboard itself, see for example https://github.com/robotology/icub-models/blob/1c9dd81f5db605f00bdc26ddec5af614e41c98b5/iCub/conf_icub3/gazebo_icub_left_hand.ini or similar configuration files. Unfortunatly this is not really feasible, unless we first update all such uses in our repo and we clearly document this breaking change for external users. I think that keeping the old behaviour if yarpDeviceName is not specified is probably an acceptable compromise.
There was a problem hiding this comment.
I don't think that is breaking anything... I updated the documentation according to robotology/yarp#2482 since you can pass a "device" param or a "networks" paramlist to the attach action... The name to be used is the same.
The network linked here, is a different network i.e. the list of devices expected by the driver... when the attach action is executed, this list is matched with the names in the PolyDriverList, which is generated from the paramlist in the action.
There was a problem hiding this comment.
Unless I am wrong, now if yarpDeviceName is not set, GazeboYarpPlugins::Handler::getHandler()->setDevice is not called. In existing models such as iCubGazeboV2_5_visuomanip in icub-models, this means that for example for the left_hand_finger device in https://github.com/robotology/icub-models/blob/1c9dd81f5db605f00bdc26ddec5af614e41c98b5/iCub/conf/gazebo_icub_left_hand_thumb.ini#L21, GazeboYarpPlugins::Handler::getHandler()->setDevice is not called anymore. This means that then, the wrapper with port prefix /icubSim/left_hand instantiated by https://github.com/robotology/icub-models/blob/1c9dd81f5db605f00bdc26ddec5af614e41c98b5/iCub/conf_icub3/gazebo_icub_left_hand.ini is not able to find and attach to the left_hand_finger device in
/icubSim/left_hand will not correctly stream and control the joints corresponding to left_hand_finger .
There was a problem hiding this comment.
This was for coherence with the other devices using yarpDeviceName... Probably this is wrong everywhere, if the option is not set, it should just use the old naming style...
If you agree, I will update all the devices...
There was a problem hiding this comment.
I changed the logic in all plugins... They should now be registering with the default name if yarpDeviceName is not set
There was a problem hiding this comment.
Not sure of the implications in changing that on all plugins instead of just in gazebo_yarp_controlboard that was the only one using this functionality between gazebo_yarp_robotinterface, however as long as the documentation is updated to match the actual behaviour of the plugin I am ok with this (keeping the change just in gazebo_yarp_controlboard permitted to easily document what was happening).
drdanz
force-pushed
the
yarpDeviceName
branch
2 times, most recently
from
3cebc4b
to
7792d10
Compare
|
Hi @drdanz, thanks for the edit in the docs, can I further modify it? |
|
Thanks @traversaro |
Depends on #558 (not really, just the changelog, to avoid merge conflicts)