Potentially unsafe static_cast of double to int in SetIO service handler

[gavanderhoorn]

@miguelprada wrote (in ros-industrial/universal_robot#415 (comment)):

It makes me think whether ur_modern_driver should use std::lround instead of static_cast<uint8_t> to transform tool voltages just to be safer.

[gavanderhoorn]

Labelled as help wanted as this would be a good first issue to work on.

[reidchristopher]

Seems like a change would be unnecessary. A message sent should use the enumerations for 0, 12, and 24, meaning there should never be any values like 11.99 or 23.99 that would give reason for that change. Please do correct me if I misunderstand.

[gavanderhoorn]

A message sent should use the enumerations for 0, 12, and 24, meaning there should never be any values like 11.99 or 23.99 that would give reason for that change.

I guess the important bit here is "should use the enumerations".

Fractional values are not expected, I agree, but perhaps it might make sense to -- instead of simply static_casting the value to an integer -- be defensive and check that one of the acceptable values is being passed in.

Prior to ros-industrial/universal_robot#415 there was no set of constants that could be used, so clients could be specifying any value.

[gavanderhoorn]

@reidchristopher: would this be something you see opportunity to work on? Otherwise I'm going to remove the wrid19 label from the issue and remove it from the board.

[reidchristopher]

The value is actually being checked by URCommander::setToolVoltage and will return that the service failed if the value is not exactly 0, 12, or 24. At the end of the day, I have to admit it should do no harm to change the casting to rounding, other than the one off chance that individuals believe they are setting the voltage to a slightly lower value than they are.

[reidchristopher]

Within the last few seconds, I realized the checking logic is flawed and will always return false. That at the very least needs to be fixed and I would be happy to do so

[reidchristopher]

Fixed the logic and added the rounding in #327