Hal errors result in assertion error

[computer-whisperer]

If you cause the hal to return an error, it will currently result in a slightly confusing AssertionError.

[virtuald]

Example?

[computer-whisperer]

Sorry for the vague bug report, I can't stick a traceback in here ATM, but it can be reproduced by adding the following lines of code to the beginning of a hal statusfunc and causing it to run.

status.value = PARAMETER_OUT_OF_RANGE
return False

[computer-whisperer]

Here is a traceback:

Traceback (most recent call last):
  File "/home/christian/PycharmProjects/robotpy-2015-fork/wpilib/wpilib/robotbase.py", line 154, in main
    robot.startCompetition()
  File "/home/christian/PycharmProjects/robotpy-2015-fork/wpilib/wpilib/iterativerobot.py", line 70, in startCompetition
    self.robotInit()
  File "/home/christian/PycharmProjects/robotpy-2015-fork/examples/test.py", line 9, in robotInit
    self.m1 = wpilib.Victor(0)
  File "/home/christian/PycharmProjects/robotpy-2015-fork/wpilib/wpilib/victor.py", line 48, in __init__
    super().__init__(channel)
  File "/home/christian/PycharmProjects/robotpy-2015-fork/wpilib/wpilib/safepwm.py", line 25, in __init__
    PWM.__init__(self, channel)
  File "/home/christian/PycharmProjects/robotpy-2015-fork/wpilib/wpilib/pwm.py", line 85, in __init__
    if not hal.allocatePWMChannel(self._port):
  File "/home/christian/PycharmProjects/robotpy-2015-fork/hal-base/hal/functions.py", line 31, in outer
    raise HALError(getHALErrorMessage(status))
  File "/home/christian/PycharmProjects/robotpy-2015-fork/hal-base/hal/functions.py", line 93, in getHALErrorMessage
    return _getHALErrorMessage(code).decode('utf_8')
  File "<string>", line 2, in getHALErrorMessage
AssertionError: isinstance(code, int); with code=c_int(-1028), type(code)=c_int

Locals at innermost frame:

{'code': c_int(-1028)}

This was exposed by adding

status.value = PARAMETER_OUT_OF_RANGE
return False

To the beginng of hal-sim/hal-impl/functions.py allocatePWMChannel() and running the test.py example.

This should have returned a HALError, not an AssertionError

[virtuald]

Thanks, much better. I think the assert is probably rightish, which means I'll think about ways to fix it.

What the assert should be saying is "parameter 'code' has an invalid type", and then the rest of the message. The isinstance part is showing the comparison that it did, and the rest is showing what the value that was incorrect was.

[computer-whisperer]

I understand what the traceback means. The point is that, rather than raising a HALError with the error message defined for that specific status value, it seems to trip up with that AssertionError first. I guess if this is intentional than it is ok, but it does return slightly vauge error messages. The original place I found this was in the mxp_port code, where this same error is thrown, with only a different status value.

[virtuald]

Well, the point is that the assert should never have happened, so the user (you) shouldn't see it.

This is a general philosophy about asserts, actually -- in real code, only use them in places where they should never be hit (eg, programming errors). If the error is expected, then a different error mechanism is more appropriate.

So, today I will fix the message to be more clear, and I will look at why the assert is happening and fix that as well.

[virtuald]

Cool, so there were a few errors here. The big one that I didn't realize was there is that the validation function I wrote doesn't accept ctypes values as inputs.

I decided to not change the validation mechanism, because nobody should be creating ctypes values anyways and passing them to HAL. Instead, I changed the status func to pass the value to HALGetErrorMessage.