robotpy / robotpy-wpilib

Moved to https://github.com/robotpy/mostrobotpy
https://robotpy.github.io
Other
169 stars 59 forks source link

hal.HALControlWord correct arguments #29

Closed computer-whisperer closed 9 years ago

computer-whisperer commented 9 years ago

When running the example robot script under examples/test.py, with the workarounds described in #28 , I encounter the following error:

Traceback (most recent call last):
  File "/home/christian/PycharmProjects/modded-command/robot.py", line 24, in <module>
    wpilib.RobotBase.main(MyRobot)
  File "/home/christian/robotpy-2015/lib/python3.4/site-packages/wpilib/robotbase.py", line 129, in main
    RobotBase.initializeHardwareConfiguration()
  File "/home/christian/robotpy-2015/lib/python3.4/site-packages/wpilib/robotbase.py", line 124, in initializeHardwareConfiguration
    RobotState.impl = DriverStation.getInstance()
  File "/home/christian/robotpy-2015/lib/python3.4/site-packages/wpilib/driverstation.py", line 41, in getInstance
    DriverStation.instance = DriverStation()
  File "/home/christian/robotpy-2015/lib/python3.4/site-packages/wpilib/driverstation.py", line 56, in __init__
    self.controlWord = hal.HALControlWord()
TypeError: __init__() missing 1 required positional argument: 'd'

I can't tell what hal.HALControlWord() is supposed to require on hal-roborio, but the implementation in hal-sim appears to require the argument d, and seems to pull data from it, as if it were a dictionary.

I can resolve the issue by replacing hal.HALControlWord() with hal.GetHALControlWord() in wpilib/wpilib/driverstation.py, but I don't know how this fix affects hal-roborio

virtuald commented 9 years ago

At some point the DriverStation classes should be refactored so it can catch errors like this. Currently the DS test uses magic mocks, and doesn't use simulated HAL. It is a bit tricky to test, however.

computer-whisperer commented 9 years ago

What is magic mock?

virtuald commented 9 years ago

https://docs.python.org/3/library/unittest.mock.html#unittest.mock.MagicMock

computer-whisperer commented 9 years ago

Ah, thanks.

PeterJohnson commented 9 years ago

We should be able to initialize it to all 0 this way (it's how C++ and Java work, they do NOT call HALGetControlWord). However, this initialization approach is currently broken on the RoboRIO, due to the refactor (82bc781e) which made HALControlWord a pointer rather than the actual structure. We should unbreak this and just make the GetControlWord function definition different on the two HALs (e.g. RoboRIO with POINTER, simulation without--I believe there's a flag to check this?).

virtuald commented 9 years ago

Hm. Isn't there a wrapper that fixes this? goes to check...

virtuald commented 9 years ago

Oh, I see the problem. Sorry about that. Will fix.

PeterJohnson commented 9 years ago

Now Java at least calls HALGetControlWord() at startup. Interesting.