Implement Control Panel subsystem in java #9
Conversation
BrandonWerbel
requested review from
ArchdukeTim,
AndrewLester,
JosephCarpenter and
BobbyMi11er
|
Make sure to fix your merge conflicts. |
| colorNames.add("B"); | ||
| colorNames.add("G"); | ||
| colorNames.add("R"); | ||
| colorNames.add("Y"); | ||
|
|
||
| colors.add(new Color(0.175, 0.469, 0.356)); | ||
| colors.add(new Color(0.188, 0.503, 0.310)); | ||
| colors.add(new Color(0.325, 0.453, 0.221)); | ||
| colors.add(new Color(0.269, 0.550, 0.181)); |
There was a problem hiding this comment.
Use a HashMap for these, don't use two different lists.
There was a problem hiding this comment.
@ArchdukeTim The first time I was writing this, I looked into HashMaps, but I wasn't able to find an ordered HashMap. If you know a way to do this, please let me know.
There was a problem hiding this comment.
I looked at LinkedHashMaps, but I couldn't find a way to get a key/object at a certain index, or find the index of a certain key/object. Any thoughts?
| solenoidState = DoubleSolenoid.Value.kReverse; | ||
| } | ||
|
|
||
| public void getFMSColor(){ |
There was a problem hiding this comment.
This isn't a good name for this since it doesn't return the FMS color. Maybe calculateTurnToColor would be better
| cpMotor = new VictorSPX(2); | ||
| cpSolenoid = new DoubleSolenoid(5, 4); |
There was a problem hiding this comment.
I wonder if the team should get in the habit of having a constants class that holds all the port numbers for the different electronics. It may be useful later on to ensure you don't change something in one class but forget to change it in another (e.g if the physical cables were swapped around)
There was a problem hiding this comment.
Maybe open an issue for it? That seems like something to focus a different pull request on.
No description provided.