wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.05k stars 612 forks source link

Creating 2 ADXRS450_Gyro devices in simulation hangs the robot #3957

Open virtuald opened 2 years ago

virtuald commented 2 years ago

Describe the bug Creating 2 ADXRS450_Gyro devices in simulation hangs the robot program.

To Reproduce

$ python
Python 3.10.1 (main, Dec  7 2021, 00:00:00) [GCC 11.2.1 20210728 (Red Hat 11.2.1-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import wpilib
>>> wpilib.ADXRS450_Gyro()
<wpilib._wpilib.ADXRS450_Gyro object at 0x7fbf28f091b0>
>>> wpilib.ADXRS450_Gyro()
^C^C^C^C^Z
[1]+  Stopped                 python

Expected behavior An exception saying "device already exists" or something to that effect.

Additional context

Obviously this falls under the category of "Well don't do that"... but I wonder whether there are other difficult to diagnose bugs in places where the simulation behavior hinges on whether (bool)simDevice is true?

ThadHouse commented 2 years ago

Its actually more invasive to do this mid season then I would like, so I think this will have to wait until 2023.

For future me, its because the low level does not use handles, so it doesn't have great checking. And then at the high level we only report an error on failure and not throw an initialization exception.

PeterJohnson commented 2 years ago

We likely have several classes affected by this, essentially what's happening is we use the simDevice return value as a proxy for is-simulation. A relatively low impact change would be to check the false case of that (e.g. failure to create a SimDevice) and throw an error only if simulation is enabled (which would indicate a double-allocation in simulation).

Starlight220 commented 1 year ago

I wasn't able to reproduce the hang in Java/C++.

In Java, we can check and throw in SimDevice.create, but in C++ SimDevice doesn't have access to frc/Errors.h.

virtuald commented 1 year ago

This still occurs on the current RobotPy beta.