[wip] Drivetrain Functionality

[brownkat6]

private static OI INSTANCE = new OI(); // declare buttons, joysticks here private Joystick leftFlight; private Joystick rightFlight;

• public double removeDeadband(double y){
• if (Math.abs(y)<=.05){
• return 0;
• }else{
• return y;
• }
• }
• public double getLeftY(){

What is being redefined?

On Fri, Jan 11, 2019 at 5:30 PM Caleb Xavier Berger < notifications@github.com> wrote:

@c-x-berger requested changes on this pull request.

See comments. In general, you need to do Ctrl+Alt+L on everything. (You can actually do this by clicking on src in the Project panel on the left and running the key chord.)

In src/main/java/frc/robot/OI.java https://github.com/BHSSFRC/3494_2019_repo/pull/1#discussion_r247268207:

 private static OI INSTANCE = new OI();

// declare buttons, joysticks here private Joystick leftFlight; private Joystick rightFlight;

• public double removeDeadband(double y){
• if (Math.abs(y)<=.05){
• return 0;
• }else{

Can you do Ctrl+Alt+L on this file? Consistent formatting makes code much easier to read.

In src/main/java/frc/robot/RobotMap.java https://github.com/BHSSFRC/3494_2019_repo/pull/1#discussion_r247268659:

@@ -23,6 +23,20 @@ // number and the module. For example you with a rangefinder: // public static final int rangefinderPort = 1; // public static final int rangefinderModule = 1; +

• public class PDP{

Again, Ctrl+Alt+L. Also, good idea having constants organized into subclasses.

In src/main/java/frc/robot/subsystems/Drivetrain.java https://github.com/BHSSFRC/3494_2019_repo/pull/1#discussion_r247269627:

@@ -17,13 +22,49 @@ private Drivetrain() {

• @param leftSpeed Speed of left side.
• @param rightSpeed Speed of right side. */
• public void tankDrive(double leftSpeed, double rightSpeed) {
• public void tankDrive(double leftPower, double rightPower)
• {

Formatting, formatting, formatting. { goes on the same line as the method declaration.

In src/main/java/frc/robot/subsystems/Drivetrain.java https://github.com/BHSSFRC/3494_2019_repo/pull/1#discussion_r247269780:

@@ -17,13 +22,49 @@ private Drivetrain() {

• @param leftSpeed Speed of left side.
• @param rightSpeed Speed of right side. */
• public void tankDrive(double leftSpeed, double rightSpeed) {

Please leave these as leftSpeed and rightSpeed or change the JavaDoc (comment) accordingly.

In src/main/java/frc/robot/subsystems/Drivetrain.java https://github.com/BHSSFRC/3494_2019_repo/pull/1#discussion_r247270116:

 }

• public static Drivetrain getInstance() {
• return INSTANCE;
• //this checks to make sure that if the robot is told to go forward, its actually traveling forward instead of backward
• private boolean checkTankDriveNotBroken(double leftPower, double rightPower){

This could use a more descriptive name, as well as proper JavaDoc (see tankDrive for an example.)

In src/main/java/frc/robot/subsystems/PDP.java https://github.com/BHSSFRC/3494_2019_repo/pull/1#discussion_r247273135:

@@ -0,0 +1,35 @@ +package frc.robot.subsystems; + + +import edu.wpi.first.wpilibj.PowerDistributionPanel; +import edu.wpi.first.wpilibj.command.Subsystem; +import frc.robot.RobotMap; + +public class PDP extends Subsystem {

I'm not sure how I feel about having things like the PDP (which to me, seem like sensors) as full-fledged Subsystems. WPI describes Subsystems as "parts of your robot that are independently controlled" https://wpilib.screenstepslive.com/s/currentCS/m/java/l/599735-simple-subsystems, which to me means this isn't the intended use case. The PDP isn't "independently controlled" - it's not really controlled at all.

That said, the PDP doesn't clearly "belong" to any system on the robot (like Talons etc.), so... I dunno. I'll give it more thought.

In src/main/java/frc/robot/OI.java https://github.com/BHSSFRC/3494_2019_repo/pull/1#discussion_r247273410:

 private static OI INSTANCE = new OI();

// declare buttons, joysticks here private Joystick leftFlight; private Joystick rightFlight;

• public double removeDeadband(double y){
• if (Math.abs(y)<=.05){
• return 0;
• }else{
• return y;
• }
• }
• public double getLeftY(){

Redefining these breaks the build. https://travis-ci.org/BHSSFRC/3494_2019_repo/builds/478553551

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/BHSSFRC/3494_2019_repo/pull/1#pullrequestreview-191884287, or mute the thread https://github.com/notifications/unsubscribe-auth/Ae0OrFw64PVA7oy_zdX2xB54-YksxixMks5vCRCMgaJpZM4Z8Pb6 .

[c-x-berger]

Looks mergable to me. I still have my issues, but they're either too nitpicky or too grand (e.g. sensor subsystems) to bother fixing before merge.

A good compromise on the sensors issue might be a grand unified "Sensors" susbsytem (GRUSS lol), however, that would probably lock up commands a fair amount (if command A only needs the NavX and command B only needs the PDP they'll still block each other.) And you still have subsystems using each other, which seems to defeat some of their purpose.

A case can definitely be made for saying that the NavX belongs to the drivetrain, I know that much.