New to FRC programming... How does this look?

I am completely new at FRC programming, and I’m not sure what really is the best way to go about it. Anyone care to give suggestions?

package edu.wpi.first.wpilibj.templates;
import edu.wpi.first.wpilibj.*;

public class RobotTemplate extends IterativeRobot {

    public RobotDrive drivetrain;
    public Joystick leftStick;
    public Joystick rightStick;
    public Joystick KTrigger;
    public Gyro gyro; //gyro
    public Watchdog getWatchdog;
    
    

    public void robotInit(){
        drivetrain = new RobotDrive(1, 2, 3, 4);  
        leftStick = new Joystick(1);    
        rightStick = new Joystick(2);
        KTrigger = new Joystick(3);
    }


    public void autonomousPeriodic(){
        getWatchdog.kill();
        int x = 1;
        do{    
            gyro.reset();

            int i=0;
            while(i<4){    
                drivetrain.drive(0.5,0.0); //START GOING STRAIGHT
                Timer.delay(4.0);
                drivetrain.drive(0.0,0.0); //STOP
                Timer.delay(4.0);
                gyro.reset();
                double angle = 0;
                do {
                    drivetrain.drive(0.0, 0.15); //TURN 
                    Timer.delay(0.5);
                    angle += gyro.getAngle(); //
                    gyro.reset();
                    log("Gyro reads "+angle);
                }while(angle<90); //Keep going in increments of .15 until gyro returns more than 90 degrees
                drivetrain.drive(0.0,0.0); //STOP
                Timer.delay(4.0);
                i++;
            }
        }while(x>0);
    } 

    public void teleopPeriodic() {
        getWatchdog().kill();
        while (isOperatorControl() && isEnabled()){
        Timer.delay(0.002);
                while(KTrigger.getTrigger()){
                    drivetrain.drive(0.0,0.0);
                    Timer.delay(1.0);
                }
            double leftYAxis = leftStick.getAxis(Joystick.AxisType.kY);
            double leftXAxis = leftStick.getAxis(Joystick.AxisType.kX);
            double rightYAxis = rightStick.getAxis(Joystick.AxisType.kY);
            double rightXAxis = rightStick.getAxis(Joystick.AxisType.kX);
            double powerValue = ((leftXAxis+rightXAxis)/2); // FIND RIGHT CALCULATION FOR 0-100% (0.0-1.0)
            double turnValue = ((leftYAxis+rightYAxis)/2);//FIND RIGHT CALCULATION FOR 0.0-1.0
            drivetrain.drive(powerValue,turnValue);
        }
    }
    public static String log(String aMessage){
        System.out.println(aMessage);
        return aMessage;
    }
}

You need to initialize all used objects in RobotInit() – You’re calling data from the gyro, but it’s never been initialized, so you’ll get Null Pointers from that; same for your Watchdog.

You don’t need to use the watchdog class.

During teleopPeriodic: Timer.delay(0.002) only slows down the robot, same for Timer.delay(1.0) – it makes that thread sleep and you can’t do anything until it wakes up. You’d be better off using if(KTrigger.getTrigger())drivetrain.drive(0,0);} – Since teleopPeriodic is looped anyway, the “while” loop is redundant.

Just some quick suggestions :slight_smile:

Thanks so much for the help :slight_smile:
Look better?

package edu.wpi.first.wpilibj.templates;
import edu.wpi.first.wpilibj.*;
 
public class RobotTemplate extends IterativeRobot {
 
    public RobotDrive drivetrain;
    public Joystick joystick;
    public Joystick KTrigger;
    public Gyro gyro; //gyro
   
   
 
    public void robotInit(){
        drivetrain = new RobotDrive(1, 2, 3, 4); 
        joystick = new Joystick(1);   
        KTrigger = new Joystick(1);
        gyro = new Gyro(1);
    }
 
 
    public void autonomousContinous(){
        int x = 1;
        do{   
            gyro.reset();
 
            int i=0;
            while(i<4){   
                drivetrain.drive(0.5,0.0); //START GOING STRAIGHT
                Timer.delay(4.0);
                drivetrain.drive(0.0,0.0); //STOP
                Timer.delay(4.0);
                gyro.reset();
                double angle = 0;
                do {
                    drivetrain.drive(0.0, 0.15); //TURN
                    Timer.delay(0.5);
                    angle += gyro.getAngle(); //
                    gyro.reset();
                    log("Gyro reads "+angle);
                }while(angle<90); //Keep going in increments of .15 until gyro returns more than 90 degrees
                drivetrain.drive(0.0,0.0); //STOP
                Timer.delay(4.0);
                i++;
            }
        }while(x>0);
    }
 
    public void teleopContinous() {
        while (isOperatorControl() && isEnabled()){
            if(KTrigger.getTrigger())
                {drivetrain.drive(0,0);
                log("Brake enabled");}
            double YAxis = joystick.getAxis(Joystick.AxisType.kY);
            double XAxis = joystick.getAxis(Joystick.AxisType.kX);
            drivetrain.drive(YAxis,XAxis);
        }
    }
    public static String log(String aMessage){
        System.out.println(aMessage);
        return aMessage;
    }
}

This may just be one of the things that I do and find useful, but creating constants for port numbers can be extremely useful for someone who has not read your code and is not using your robot.

Things like

private static final int DRIVE_LEFT_FRONT = 1;
Jaguar frontLeftDrive = new Jaguar(DRIVE_LEFT_FRONT);

will be a lot easier for others to understand. (And when you want to make a change you can just find your constants and type in a new number)

Also, to find the x axis/y axis value just type joystick.getX() or joystick.getY(). You also do not need to set the value of the joystick to a double before supplying it to the drive() method.


    public static String log(String aMessage){
        System.out.println(aMessage);
        return aMessage;
    }

The above would probably just be better off as a void where you don’t have to bother with the return type. You’ll just be telling yourself something that you already know.


double angle = 0;

do {
     drivetrain.drive(0.0, 0.15); //TURN
     drivetrainTimer.delay(0.5);
     angle += gyro.getAngle(); //
     gyro.reset();
     log("Gyro reads "+angle);
}while(angle<90); //Keep going in increments of .15 until gyro 

I don’t use the standard drive() methods, but IIRC the first value is the forward speed and the second the turning offset speed-wise, not an angle. So unless you know 0.5 second increments is going to end up close to 90, you could be over by a bit.
And only reset your gyro when you need to, and not when its moving. You want to stop and wait a little so that you have a steady zero value.

Like omalleyj said, resetting gyro in the loop isn’t a good way, especially when it’s moving.

I would do something simple like this:


// Reset gyro before the main loop, or with a button.
double angle = gyro.getAngle();
if (angle < 90) {
    drivetrain.drive(0.0, 0.15); // I don't know how turn works, I've only done tank drive and mecanum drive in the past.
}