DeepBlueRobotics / lib199

Code that we reuse in different projects/years.
Other
2 stars 0 forks source link

Add sim support for Limelights #102

Open brettle opened 1 month ago

brettle commented 1 month ago

- [ ] Add SensorFactory.createLimelight() to return a Limelight while also providing creating an appropriate SimDevice for it either via subclassing or using Mockito. The SimDevice would include kInput values for things like tx, ty, etc.

CoolSpy3 commented 1 month ago

Given that we've already merged DeepBlueRobotics/DeepBlueSim#67, would it make more sense to just have any sim interface work over the existing NT API? We could add helper classes that set the specific NT values, exposing get/setTx/Ty(), but I don't really think that's necessary either, as they would basically just be one-liners/boilerplate.

brettle commented 1 month ago

I'm fine with not using the SimDevice/SimDeviceSim infrastructure here, but would still like to see a LimelightSim class that encapsulates getting/setting the relevant NT values. It would be the simulator-facing analog of the robot-facing LimeLightHelpers provided by the vendor. While most of the code would be one-liners/boilerplate, it would enable better compile time checking (because callers wouldn't be at risk of misspelling the names of the NT keys) and would make it easier to keep existing code working if the vendor's NT interface changes or is replaced. All that said, I agree such work is low priority.

brettle commented 1 month ago

Actually, one downside to using NT is that if the developer is on the robot network (which also contains the limelight) when they try to run a simulation, the simulated and real limelights will be fighting over the same keys unless other measures are taken. Might want to still have something like SensorFactory.createLimelight() that returns a different implementation during simulation, perhaps just one that uses a different NT table.