icubWholeBodyActuators::setControlMode controlMode is not correctly updated on all joints

[francesco-romano]

Method icubWholeBodyActuators::setControlMode called for all joints. The controlMode (currentCtrlModes) map update can be skipped: see the following code:

switch(controlMode)
    {
        case CTRL_MODE_POS:
            FOR_ALL(itBp, itJ)
                if(currentCtrlModes[LocalId(itBp->first,*itJ)]!=controlMode)
                    ok = ok && icmd[itBp->first]->setPositionMode(itBp->first==TORSO ? 2-(*itJ) : *itJ);
            break;
.....
    }
    if(ok)
    {
        FOR_ALL(itBp, itJ)
            currentCtrlModes[LocalId(itBp->first, *itJ)] = controlMode;
        if(ref!=0)
            ok = ok && setControlReference(ref);
    }

If for the i-th joint the set*Mode returns false, then all subsequent updates are skipped, and the variable ok is false. But the first i joints are still in another control mode. I think the map update should be done contextually to the set function call.

[andreadelprete]

I totally agree. Since we are discussing this method I have a couple doubts.

• When a joint fails to change control mode, should the method keep changing the control mode of the next joints?
• Should the operation be atomic? That is, in case one joint fails to switch to the new control mode, should the control modes of the previous joints return to their old values?

[francesco-romano]

The real point is the second question/issue you are posing. And I think this issue is not related only to this single method but to the whole library. For example I'm trying to implement the ''set'' of the PIDs (at least for the torque control) and I'm facing the same dilemma..

I'm curious to hear opinions from others too (at least @iron76, @lornat75 )

[lornat75]

I am not sure I understand the problem, especially for PIDs. Switching control modes should be done on a per-joint basis. There was an automatic switch upon idle/fault for coupled joints.

Am I missing something?

BTW: with Marco R we have been preparing a revision of the control modes specifications (see iCub Facility redmine), this is clearly a good time for discussing changes

[francesco-romano]

What I meant is the following: from the perspective of an application (which uses the ‘’middleware’’ wholebodyinterface) and which commands some operations (e.g. changes of PIDs, changes in the control mode, …) to a set of joints, what should it expect if only one joint fails in executing the command? So.. for example, if I set the control mode to 5 joints and one does not obey the command, should I cancel the operation or simply signal the issue to the calling application and leave some joints in one control mode and the others in another control mode?

[randaz81]

In my opinion, the application (and not the middleware) should take the decision, since the user may use strategies which may be different from case to case. Another possibility is to call exit(-1) and terminate the execution of the calling application. This is extremely strict but also very safe: it guarantees that an application will never execute unexpected "branches" (e.g. trying to control the torque of a joint still set in velocity mode). But I personally prefer returning an error code if just one joint of the set fails (and trust in a scrupulous user which checks for it...)

2014-01-29 Francesco Romano notifications@github.com

What I meant is the following: from the perspective of an application (which uses the ''middleware'' wholebodyinterface) and which commands some operations (e.g. changes of PIDs, changes in the control mode, ...) to a set of joints, what should it expect if only one joint fails in executing the command? So.. for example, if I set the control mode to 5 joints and one does not obey the command, should I cancel the operation or simply signal the issue to the calling application and leave some joints in one control mode and the others in another control mode?

Reply to this email directly or view it on GitHubhttps://github.com/robotology/codyco/issues/15#issuecomment-33564217 .

[francesco-romano]

I like the idea of leaving the decision to the application. But at this point I think a boolean value in not enough. Maybe a bool (as a return value) and a proper error object (as output parameter filled only if the return value is false) is more adequate… (just an idea)

On 29/gen/2014, at 10:48, randaz81 notifications@github.com wrote:

In my opinion, the application (and not the middleware) should take the decision, since the user may use strategies which may be different from case to case. Another possibility is to call exit(-1) and terminate the execution of the calling application. This is extremely strict but also very safe: it guarantees that an application will never execute unexpected "branches" (e.g. trying to control the torque of a joint still set in velocity mode). But I personally prefer returning an error code if just one joint of the set fails (and trust in a scrupulous user which checks for it...)

2014-01-29 Francesco Romano notifications@github.com

What I meant is the following: from the perspective of an application (which uses the ''middleware'' wholebodyinterface) and which commands some operations (e.g. changes of PIDs, changes in the control mode, ...) to a set of joints, what should it expect if only one joint fails in executing the command? So.. for example, if I set the control mode to 5 joints and one does not obey the command, should I cancel the operation or simply signal the issue to the calling application and leave some joints in one control mode and the others in another control mode?

Reply to this email directly or view it on GitHubhttps://github.com/robotology/codyco/issues/15#issuecomment-33564217 .

— Reply to this email directly or view it on GitHub.

[randaz81]

Unless you need to return a complex data type (e.g. a struct), usually a signed long int is the preferred return type. You can then use an enum or a macro (this is typically done in an errors.h file) to assign a "name" to each code which should be compliant to the convention:

• all negative numbers are error codes.
• all positive numbers (including 0) are success codes. In this way you can just test the sign to obtain a true/false in a if condition. Some libraries (SDL, windowsDirectX, openGL) also provide a "string getError(int err)" method, which converts the numeric code to a printable string describing the error.

Another possibility is using try-catch-throw statements (that I completely dislike).

2014-01-29 Francesco Romano notifications@github.com

I like the idea of leaving the decision to the application. But at this point I think a boolean value in not enough. Maybe a bool (as a return value) and a proper error object (as output parameter filled only if the return value is false) is more adequate... (just an idea)

On 29/gen/2014, at 10:48, randaz81 notifications@github.com wrote:

In my opinion, the application (and not the middleware) should take the decision, since the user may use strategies which may be different from case to case. Another possibility is to call exit(-1) and terminate the execution of the calling application. This is extremely strict but also very safe: it guarantees that an application will never execute unexpected "branches" (e.g. trying to control the torque of a joint still set in velocity mode). But I personally prefer returning an error code if just one joint of the set fails (and trust in a scrupulous user which checks for it...)

2014-01-29 Francesco Romano notifications@github.com

What I meant is the following: from the perspective of an application (which uses the ''middleware'' wholebodyinterface) and which commands some operations (e.g. changes of PIDs, changes in the control mode, ...) to a set of joints, what should it expect if only one joint fails in executing the command? So.. for example, if I set the control mode to 5 joints and one does not obey the command, should I cancel the operation or simply signal the issue to the calling application and leave some joints in one control mode and the others in another control mode?

Reply to this email directly or view it on GitHub< https://github.com/robotology/codyco/issues/15#issuecomment-33564217> .

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHubhttps://github.com/robotology/codyco/issues/15#issuecomment-33572344 .

[francesco-romano]

I proposed that idea because I am not experienced in “robot” (or high-performance, or real-time) programming. I have more experience in more high-level frameworks.

So.. I think the question is.. What information do we want to give to the caller? If all we want to give is just a generic error (I don’t think the string function can be used inside the application) then the enum is a good choice. If want to provide more information (which joint failed for example) then struct object is needed. But as I said before I am not experienced on the typical requirement of a control application and my approach can be acceptable in a generic software framework, but a bit heavy on time-stringent applications.

BTW: exceptions are not an option for me.

On 29/gen/2014, at 11:54, randaz81 notifications@github.com wrote:

Unless you need to return a complex data type (e.g. a struct), usually a signed long int is the preferred return type. You can then use an enum or a macro (this is typically done in an errors.h file) to assign a "name" to each code which should be compliant to the convention:

• all negative numbers are error codes.
• all positive numbers (including 0) are success codes. In this way you can just test the sign to obtain a true/false in a if condition. Some libraries (SDL, windowsDirectX, openGL) also provide a "string getError(int err)" method, which converts the numeric code to a printable string describing the error.

Another possibility is using try-catch-throw statements (that I completely dislike).

2014-01-29 Francesco Romano notifications@github.com

I like the idea of leaving the decision to the application. But at this point I think a boolean value in not enough. Maybe a bool (as a return value) and a proper error object (as output parameter filled only if the return value is false) is more adequate... (just an idea)

On 29/gen/2014, at 10:48, randaz81 notifications@github.com wrote:

In my opinion, the application (and not the middleware) should take the decision, since the user may use strategies which may be different from case to case. Another possibility is to call exit(-1) and terminate the execution of the calling application. This is extremely strict but also very safe: it guarantees that an application will never execute unexpected "branches" (e.g. trying to control the torque of a joint still set in velocity mode). But I personally prefer returning an error code if just one joint of the set fails (and trust in a scrupulous user which checks for it...)

2014-01-29 Francesco Romano notifications@github.com

What I meant is the following: from the perspective of an application (which uses the ''middleware'' wholebodyinterface) and which commands some operations (e.g. changes of PIDs, changes in the control mode, ...) to a set of joints, what should it expect if only one joint fails in executing the command? So.. for example, if I set the control mode to 5 joints and one does not obey the command, should I cancel the operation or simply signal the issue to the calling application and leave some joints in one control mode and the others in another control mode?

Reply to this email directly or view it on GitHub< https://github.com/robotology/codyco/issues/15#issuecomment-33564217> .

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHubhttps://github.com/robotology/codyco/issues/15#issuecomment-33572344 .

— Reply to this email directly or view it on GitHub.

[serena-ivaldi]

I prefer the unsigned int option as suggested by Randaz

[traversaro]

Slightly related: adding a verbose option in the interface or at least in its implementation could be useful during debugging.

[traversaro]

We thought about the following thing: all functions have the return error code, and an optional output parameter. This optional parameter is a pointer to an error class. This object is "filled" only in case of error, and only if the caller passed the error object. In this way the possible outcomes of the function are:

• no error: returns 0, and no error object is filled.
• error and caller does not ask for the error information: function returns a (negative) error code and there in no further overhead for the error message.
• error and the caller asked for error information: negative return code and error object filled.

In our opinion this is the best option, because we need to return more information than a mere error code: e.g. joint which failed, which operation is failed, etc..

[traversaro]

Deprecating as we are migrating to the ControlBoardRemapper class, whose handling of single joint failures will be in line with the rest of YARP software.