robotdotnet / WPILib

DotNet implementation of WPILib for FIRST Robotics Competition (FRC)
27 stars 8 forks source link

HAL DriverStationHelper data not propogating to Simulator loop #54

Closed jkoritzinsky closed 8 years ago

jkoritzinsky commented 8 years ago

I've been attempting to write more tests for the AttributedCommandModel and I've stumbled upon a very annoying threading bug. If I send updated joystick info through the DriverStationHelper, it doesn't propogate through to the Driver Station thead. This is breaking my tests on RunCommandOnJoystick.

ThadHouse commented 8 years ago

This actually was purposely broken in https://github.com/robotdotnet/WPILib/commit/b482eeacb1d546a2d4c2de14cce6d975caa0785f . For some reason, every few times running tests the dictionary would throw a collection changed error. This is still going to be replaced by an official WPILib Sim DLL, but we currently don't have that, and we still need a simulator for tests. We can either write a new DS using the new simulator, mark it internal, and use it for tests, or we can create our own DS inside the test DLL, and replace the delegates for the DS data in HAL-Base. I don't know which would be easier to do however. I'm thinking rebuilding SimDS to be the new data style.

jkoritzinsky commented 8 years ago

If we were to rebuild it, we could use the System.Collections.Concurrent.ConcurrentDictionary since that's actually built to work in a concurrent environment, as opposed to Dictionary, which is most definitely not.

ThadHouse commented 8 years ago

I was thinking build it like the new simulator with no dictionary as all. And It looks like ConcurrentDictionary would have the same issue, as the error was actually happening in the UpdateHALData method. The dictionary would get a value changed inside of that foreach loop, and throw the error there. And it looks like that would still happen with ConcurrentDictionary.

jkoritzinsky commented 8 years ago

Good point. Let's build it like the new simulator then. Just have to be careful about thread safety.

ThadHouse commented 8 years ago

New simulator is done in https://github.com/robotdotnet/WPILib/commit/727c1503ac2075ca3a01cd89d575f9c939893587