CanBusMotionControl: corrected compilation warnings + potential bugs

[marcoaccame]

Purpose of the PR

This PR addresses what in issue #754 and fixes:

• compilation warnings
• potential bugs
  • CanBusMotionControl::setControlModeRaw() and similar methods did not return false when they receive an invalid control mode.
  • in some c-string manipulation there was the danger to write out of memory in case the string is very long.

Details of the changes

There are two changes:

• fix of the behaviour / use of function CanBusMotionControl::from_modevocab_to_modeint() used inside ::setControlModeRaw() to transform a yarp::conf::vocab32_t into a icubCanProto_controlmode_t.
• fix of the creation of a string to be printed with yDebug() inside method void CanBusMotionControl::run()

In the first case, the return value of from_modevocab_to_modeint() was compared vs VOCAB_CM_UNKNOWN which was never returned because the function truncated its value (the warning was just saying that).
Solved by returning icubCanProto_controlmode_unknownError instead.

In the second case, some sprintf() did not have check vs writing out of memory in the case of particularly long strings. Solved by using a std::string which gets the memory it needs.

Tests

The changes are trivial and for this reason I did not test them on the robot.

[pattacini]

Thanks a lot @marcoaccame 👍🏻

[traversaro]

Ok for me. One thing that is worth noting is that probably that code did not use any std::string or similar to avoid the use of dynamic memory allocation (that is a quite non-determistic operation and expensive, from the real time point of view) in a frequently invoked loop. If however you do not think this is a problem in this case, I am ok with the PR.

[marcoaccame]

Ok for me. One thing that is worth noting is that probably that code did not use any std::string or similar to avoid the use of dynamic memory allocation. ....

Good point @traversaro.

The change to avoid the use of heap would be trivial: move the string out of the scope of the loop and reserve for it a reasonable chunk of memory. Any other operation on the string would not involve heap operations.

std::string mystring;
mystring.reserve(512);

[pattacini]

The change to avoid the use of heap would be trivial: move the string out of the scope of the loop and reserve for it a reasonable chunk of memory. Any other operation on the string would not involve heap operations.

Agreed 👍🏻
@marcoaccame could you proceed with the change?

[marcoaccame]

@marcoaccame could you proceed with the change?

Hi @pattacini, I have just completed the change.

[pattacini]

Just waiting the CI and then merging.

[pattacini]

This fix landed in devel but we're going to churn out 2021.08 soon.