Control Panel Subsystem #16
Conversation
Replacing with updated version
Finished draft for control panel. Will test and start debugging.
I fixed the errors within code syntax and added intake. Inputs need to be added
…plates for Intake and Wheel
Finished ControlPanel
…g/2020_robot_software into controlPanelDevelopment
|
|
||
|
|
||
|
|
||
|
|
| private DigitalInput CpDpadL; | ||
| private DigitalInput CpDpadR; | ||
| private DigitalInput CpA; | ||
| private DigitalInput Cpjoy; |
There was a problem hiding this comment.
These are not initialized, not used?
| switch(CpDeployOn){ | ||
| case 0: | ||
| CpDeployOn = 1; | ||
| break; | ||
| case 1: | ||
| CpDeployOn = 1; | ||
| break; | ||
| } |
There was a problem hiding this comment.
I don't think you need the switch here
CpDeployOn = 1 looks like it is all you need
| switch(CpDeployOn){ | ||
| case 0: | ||
| CpDeployOn = 1; | ||
| break; | ||
| case 1: | ||
| CpDeployOn = 1; | ||
| break; | ||
| } | ||
| } | ||
| if ((source == CpDpadDWN) && CpDWNbool){ | ||
| switch(CpDeployOn){ | ||
| case 3: | ||
| CpDeployOn = 2; | ||
| break; | ||
| case 2: | ||
| CpDeployOn = 2; | ||
| break; | ||
| } | ||
| } |
| case 3: | ||
| CpDeployOn = 2; | ||
| break; | ||
| case 2: | ||
| CpDeployOn = 2; |
| if (IsDown == true){ | ||
| if (CpDeployOn == 2){ |
There was a problem hiding this comment.
This could be a single if statement
| private DigitalInput CpFWDspin; | ||
| private DigitalInput CpBWDspin; | ||
| private DigitalInput CpINTspin; | ||
| private DigitalInput CpENCspin; |
There was a problem hiding this comment.
Try to avoid abbreviating variables names, this can be confusing
Also remove the Cp s they are unnecessary
| import org.wildstang.framework.io.inputs.DigitalInput; | ||
| import org.wildstang.framework.CoreUtils; | ||
|
|
||
| public class Turret implements Subsystem { |
There was a problem hiding this comment.
The turret does not belong in this pull request... please remove
|
|
||
| //other booleans/ints | ||
| private int DeployOn; | ||
| // 1 = Up, 0 = Off-Down, 2 = Down, 3 = Off-Up |
There was a problem hiding this comment.
The cp doesn't have to be fully deployed or retracted, so there is no need for the up and down states
There was a problem hiding this comment.
Best practice would be to use an enum for these states
| if ((source == DpadUP) && UPbool){ | ||
| switch(DeployOn){ | ||
| case 0: | ||
| case 1: | ||
| DeployOn = 1; | ||
| break; | ||
| } | ||
| } | ||
| if ((source == DpadDWN) && DWNbool){ | ||
| switch(DeployOn){ | ||
| case 3: | ||
| case 2: | ||
| DeployOn = 2; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
As per the comment above this should really be if down is pressed retract, if up is pressed deploy
There was a problem hiding this comment.
If neither there should also be an idle state
| UPbool = DpadUP.getValue(); | ||
| DWNbool = DpadDWN.getValue(); | ||
| FWDbool = FWDspin.getValue(); | ||
| BWDbool = BWDspin.getValue(); | ||
| ENCbool = ENCspin.getValue(); | ||
| INTbool = INTspin.getValue(); |
There was a problem hiding this comment.
If you are going to predefine values for the button state, use descriptive names. Never use the variable type in a name. For example downPressed would be a decent name
| private double UPWAIT = 5.25; | ||
| private int Spins = 0; | ||
| private boolean start = false; | ||
| private double CMDspin; |
There was a problem hiding this comment.
This seems mixed between being a state and a speed. I recommend using a speed for the motor for the control panel mech
There was a problem hiding this comment.
Then is we are running in encoder controlled mode, have another variable that tracks the number of encoder cycles remaining
| if (FWDbool){ | ||
| CMDspin = 1; //if FWD, spin forward | ||
| } | ||
| else{ | ||
| if(BWDbool){ | ||
| CMDspin = -1; //otherwise, if BWD, spin backwards | ||
| } | ||
| else{ | ||
| if (CMDspin != 6) //CMD 6 means encoder is running | ||
| CMDspin = 0; //if nothing pressed and encoder not running, turn motor off | ||
| } | ||
| } |
There was a problem hiding this comment.
As recommended above have 2 variables, one for speed and one for number of encoder ticks remaining
| if ((source == ENCspin) && ENCbool){ | ||
| Spins += (1500); | ||
| spinOn = true; | ||
| CMDspin = 6; | ||
| Spinner.getSensorCollection().setQuadraturePosition(0, 0); | ||
| } |
There was a problem hiding this comment.
This should be with the other spin mech code
Also recommend adjusting the speed as you approach the goal
| } | ||
|
|
||
| @Override | ||
| public void update() { |
There was a problem hiding this comment.
This function should really just be setting the motors to their appropriate speeds. Also the encoder and limit switched will be attached through the Talons
|
Code doesn't compile, please determine the error and resolve |
…g/2020_robot_software into controlPanelDevelopment
| if (booldeployUp&&(!booldeployDown)){ | ||
| deploySpeed = 1; //deploy goes up | ||
| } | ||
| if (booldeployDown&&(!booldeployUp)){ | ||
| deploySpeed = -1; //deploy goes down | ||
| } | ||
| if ((!booldeployDown)&&(!booldeployUp)){ | ||
| deploySpeed = 0; | ||
| } |
There was a problem hiding this comment.
What happens here if you have 2 buttons pressed?
I'd remove the checks for the opposite button not being pressed and switch to and if-elseif-else block.
Regardless, it shouldn't really be possible to press both on the d-pad.
| if (boolforwardSpin&&(!boolbackwardSpin)){ | ||
| spinSpeed = 1; | ||
| presetSpins = 0; | ||
| } | ||
| if (boolbackwardSpin&&(!boolforwardSpin)){ | ||
| spinSpeed = -1; | ||
| presetSpins = 0; | ||
| } | ||
| if ((!boolbackwardSpin)&&(!boolforwardSpin)){ | ||
| spinSpeed = 0; | ||
| } |
This reverts commit c8cbe1d.
Before it would have not spun when the presser spin was pressed unless it had passed the preset spin point, in which case it would get stuck spinning.
More changes expected. hopefully no errors.
they were important.
#16