Incoherent behavior of joint/group checkMotionDone() overloads

[rsantos88]

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]

yarp-devices/libraries/YarpPlugins/CanBusControlboard/IPositionControlImpl.cpp

Lines 82 to 94 in e26db48

	bool roboticslab::CanBusControlboard::checkMotionDone(bool *flag)
	{
	CD_DEBUG("\n");
	*flag = true;
	bool ok = true;
	for(int j=0; j<nodes.size(); j++)
	{
	bool tmpFlag;
	ok &= this->checkMotionDone(j,&tmpFlag);
	*flag &= tmpFlag;
	}
	return ok;
	}

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]

Reported upstream at robotology/yarp#2027.

[PeterBowman]

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 robotology/yarp#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 #211.