search

roboticslab-uc3m / yarp-devices

A place for YARP devices
https://robots.uc3m.es/yarp-devices/
9 stars 7 forks source link

Incoherent behavior of joint/group checkMotionDone() overloads #214

Closed rsantos88 closed 5 years ago

rsantos88 commented 5 years ago

There is an evident contradiction between the commands get don and get dons, giving different results:

$ yarp rpc /teo/head/rpc:i
>>get don 1
Response: [is] don 1 [tsta] 2 1558536496.7235 [ok]
>>get don 0
Response: [is] don 1 [tsta] 3 1558536498.5466 [ok]
>>get dons
Response: [is] dons 0 [tsta] 4 1558536501.30877 [ok]

The same results with other lims:

$ yarp rpc /teo/rightArm/rpc:i
>>get don 0
Response: [is] don 1 [tsta] 5547 1558535762.90932 [ok]
>>get don 1
Response: [is] don 1 [tsta] 5548 1558535765.5031 [ok]
>>get don 2
Response: [is] don 1 [tsta] 5549 1558535766.82937 [ok]
>>get don 3
Response: [is] don 1 [tsta] 5550 1558535769.54835 [ok]
>>get don 4
Response: [is] don 1 [tsta] 5551 1558535771.78888 [ok]
>>get don 5
Response: [is] don 1 [tsta] 5552 1558535773.75959 [ok]
>>get don 6
Response: [is] don 1 [tsta] 5553 1558535777.45085 [ok]
>>get dons
Response: [is] dons 0 [tsta] 5554 1558535821.17145 [ok]

Note: sometimes the first result are also shown with garbage:

$ yarp rpc /teo/rightArm/rpc:i 
>>get dons
Response: [is] dons 18 [tsta] 0 1558536392.02125 [ok]
>>get dons
Response: [is] dons 0 [tsta] 1 1558536394.39516 [ok]
>>get dons
Response: [is] dons 0 [tsta] 2 1558536395.83463 [ok]
>>get dons
Response: [is] dons 0 [tsta] 3 1558536396.49037 [ok]
PeterBowman commented 5 years ago

https://github.com/roboticslab-uc3m/yarp-devices/blob/e26db48e8a0d6582df8ca18583b55f8b3bfba898/libraries/YarpPlugins/CanBusControlboard/IPositionControlImpl.cpp#L82-L94

We are traversing all nodes wrapped by CanBusControlboard, that is: all 6 iPOS drivers per arm + their 6 corresponding CuiAbsolute nodes + 1 LacqueyFetch + 1 iPOS in the robot head, even if the latter gets wrapped by a different controlboardwrapper2 and routed to a different YARP port. This is wrong, but does not explain the results observed and described above. Also, this is dangerous given that we are potentially meddling with bytes in memory beyond the expected size of user-allocated arrays passed on as inputs as in here.

Similarly, if we take a look at the implementation of ControlBoardWrapper::checkMotionDone(bool flags*) (ref), we learn that it expects the input pointer to be interpreted as an array of booleans, size equals number of controlled joints. However, the documentation states (ref):

flag is a pointer to return value ("and" of all joints)

This is how we implement it in our CanBusControlboard, not accounting for the shortcomings explained earlier.

PeterBowman commented 5 years ago

Reported upstream at https://github.com/robotology/yarp/issues/2027.

PeterBowman commented 5 years ago

Reported upstream at robotology/yarp#2027.

Merged into current master branch, scheduled for YARP 3.1.2 release.

Similarly, if we take a look at the implementation of ControlBoardWrapper::checkMotionDone(bool flags*) (ref), we learn that it expects the input pointer to be interpreted as an array of booleans, size equals number of controlled joints. However, the documentation states (ref):

flag is a pointer to return value ("and" of all joints)

This is how we implement it in our CanBusControlboard, not accounting for the shortcomings explained earlier.

As explained by YARP folks at https://github.com/robotology/yarp/issues/2027, the expected behavior matches our implementation, that is, interpret the bool * flags as an output parameter referring to a single and-joined boolean value that encompasses readiness status for all joints.

We are traversing all nodes wrapped by CanBusControlboard, that is: all 6 iPOS drivers per arm + their 6 corresponding CuiAbsolute nodes + 1 LacqueyFetch + 1 iPOS in the robot head, even if the latter gets wrapped by a different controlboardwrapper2 and routed to a different YARP port. This is wrong, but does not explain the results observed and described above. Also, this is dangerous given that we are potentially meddling with bytes in memory beyond the expected size of user-allocated arrays passed on as inputs as in here.

Given said outcome of the upstream issue, all we need to do now is to focus on https://github.com/roboticslab-uc3m/yarp-devices/issues/211.