RobotCasserole1736 / RobotCasserole2020

Software for Robot Casserole's 2020 FIRST Infinite Recharge competition season
MIT License
1 stars 0 forks source link

Control Panel Fixup #48

Closed gerth2 closed 4 years ago

gerth2 commented 4 years ago

A number of issues exist in the control panel class which need correction:

1) The general flow of all embedded code should be ReadInputs->Process->SetOutputs. This code reads inputs last. Re-order the update function to match standard flow.

2) Though maybe functional, this is a weird pattern for detecting the "rising edge" of a boolean value.. The usual pattern is if(cur_val == true && prev_val == false){

3) The names of variables involving XButton and YButton are inaccurate. These variables represent the command from the operator, not necessarily any particular button.. What if we change which button is actually used (very likely, there's an issue to do this). The variable name will be confusing. Update the variable names to reflect their true purpose.

4) The 3-rotation code uses the same degreesToTurn() function as the rotate-to-color operation. I don't see how this will work - in the case of three rotation commands, the class will need to add the correct number of degrees to the wheel manipulator command. No color sensor interaction should be needed.

5) Where is the rotation command sent to the manipulator? I don't see it - add it if it is not there yet.

6) Has the fact that the field's sensor is offset from our robot's sensor been considered? For example, if the gameData commands red, do we need to move to red? I have some suspicion that this magical math may be part of it, but I'm not sure. If so, please add comments to clearly explain how the math works. Assume someone else will be coming through and doing operations like adding a new color, or changing the color order - what do they need to know to do their job correctly?

gerth2 commented 4 years ago

Documenting a conversation from discord:

Color order was wrong as of 1/28/2020

AdenThomas commented 4 years ago

Number 1, 2, 3, 4 and the additional comment by Chris are solved thus far. Number 6 should also be done now. Testing needed for all of this.

AdenThomas commented 4 years ago

Finished but needs testing and code reviewed