Update GamepadRunIntake #6
Conversation
This GamepadRunIntake command operates the intake mechanism by checking the status of the right bumper and left bumper of the gamepad and intakes and outakes a cube respectively.
| @Override | ||
| protected void execute() { | ||
| if(Robot.oi.gamePad.getRightBumper()) { | ||
| Robot.intake.run(0.4); |
There was a problem hiding this comment.
Nit: For this number and the one below (0.4 and -0.3), please extract them into a constant (in the current class or in the Constants class), to avoid magic numbers. You can name them something like INTAKE_FORWARD_POWER or something more relevant.
| // Called once after isFinished returns true | ||
| @Override | ||
| protected void end() { | ||
| Robot.intake.stop(); |
There was a problem hiding this comment.
Hmmm, the comment above this method says that this will be called only when isFinished returns true, which it never does (line 41). This line is never reached. Is setting the intake power to 0 sufficient for stopping the intake?
There was a problem hiding this comment.
I think that isFinished() only returns false because it's suppose to start in teleopinit() and runs for the entirety of the teleop period (basically the rest of the match). In this case, end() is not really needed unless end() is called when the match ends. For setting the intake power to 0, I would hope that it would work to stop the intake.
|
I've left a few comments on the file you're adding. Please, review. A top-level nit I have is code formatting. Please consult your team lead for your team's rules on formatting, but I've noticed that you have inconsistent spacing (sometimes 4-space indents, sometimes 2-space indents) and no space after |
|
If you want to resolve these comments, you can make the changes requested on the files, commit them, and push them to your PR's branch ( Arvind and reviewers, please let me know if you have any questions about Git or the pull request review process. This is what I do at work, after all :) |
KTOmega
left a comment
KTOmega
left a comment
There was a problem hiding this comment.
Left some comments - please review.
Added changes regarding constants and formatting.
Made requested changes to setting intake to 0 if neither bumpers are pressed.