Is my Java code ok?

[Lesafian]

Hi, I'm head programmer of Team 6077 (rookie team). I was wondering if one of you could be kind enough to look through my code (Java) to make sure it follows the rules, and theoretically works.

package org.usfirst.frc.team6077.robot;

import edu.wpi.first.wpilibj.SampleRobot;
import edu.wpi.first.wpilibj.Solenoid;
import edu.wpi.first.wpilibj.TalonSRX;
import edu.wpi.first.wpilibj.Victor;
import edu.wpi.first.wpilibj.command.Scheduler;
import edu.wpi.first.wpilibj.RobotDrive;
import edu.wpi.first.wpilibj.Compressor;
import edu.wpi.first.wpilibj.DoubleSolenoid;
import edu.wpi.first.wpilibj.Joystick;
import edu.wpi.first.wpilibj.smartdashboard.SendableChooser;
import edu.wpi.first.wpilibj.smartdashboard.SmartDashboard;

public class Robot extends SampleRobot {

        // RobotDrive Declaration
	RobotDrive drive;

	// Compressor Declaration
	Compressor compressor;

	// Solenoid Declaration
	DoubleSolenoid doublesolenoid;
	Solenoid solenoid;

	// Joystick Declaration
	Joystick xboxController;

	// Motor Controller Declaration
	TalonSRX armMotor;
	TalonSRX slideMotor;
	Victor drivemotor1;
	Victor drivemotor2;

	// Autonomous Choices
	final String defaultAuto = "Default";
	final String customAuto = "Autonomous Choice 1";
	final String customAuto2 = "Autonomous Choice 2";
	final String customAuto3 = "Autonomous Choice 3";
	final String customAuto4 = "Autonomous Choice 4";
	final String customAuto5 = "Autonomous Choice 5";

	// Chooser
	SendableChooser chooser;

	public Robot() {
		drivemotor1 = new Victor(0);
		drivemotor2 = new Victor(2);
		
		compressor = new Compressor(0);
		
		drive = new RobotDrive(0, 2);
		drive.setExpiration(0.1);
		
		xboxController = new Joystick(0);

		armMotor = new TalonSRX(5);
		slideMotor = new TalonSRX(6);

		doublesolenoid = new DoubleSolenoid(0, 1);
		solenoid = new Solenoid(2);

		chooser = new SendableChooser();

	}

	public void robotInit() {
		compressor.setClosedLoopControl(true);
		chooser.addDefault("Default Auto", defaultAuto);
		chooser.addObject("Autonomous Choice 1", customAuto);
		chooser.addObject("Autonomous Choice 2", customAuto2);
		chooser.addObject("Autonomous Choice 3", customAuto3);
		chooser.addObject("Autonomous Choice 4", customAuto4);
		chooser.addObject("Autonomous Choice 5", customAuto5);
		SmartDashboard.putData("Autonomous Chooser", chooser);
	}

	public void autonomous() {
		drive.setSafetyEnabled(false);
		String autoSelected = (String) chooser.getSelected();
		System.out.println("Autonomous Mode selected: " + autoSelected);

		switch (autoSelected) {

		case customAuto5:
			break;
		case customAuto4:
			break;
		case customAuto3:
			break;
		case customAuto2:
			break;
		case customAuto:
			break;
		case defaultAuto:
		default:
			break;
		}
		Scheduler.getInstance().run();
	}

	public void operatorControl() {
		drive.setSafetyEnabled(true);
		while (isOperatorControl() && isEnabled()) {

			// Move robot using left and right joystick
			drive.tankDrive(xboxController.getRawAxis(1), xboxController.getRawAxis(5));

			// Lift and lower the arms using the right and left bumper.

			if (xboxController.getRawButton(5)) {
				armMotor.set(1);

			} else if (xboxController.getRawButton(4)) {
				armMotor.set(-1);
			}
			else {
				armMotor.set(0);
			}

			// Move sliding mechanism forwards and backwards
			
			if (Math.abs(xboxController.getRawAxis(5)) > .1) {
				slideMotor.set(1);

			} else if (Math.abs(xboxController.getRawAxis(5)) < -.1) {
				slideMotor.set(-1);
			}
			else {
				slideMotor.set(0);
			}

			// Open and close the claw
			

			if (xboxController.getRawButton(2)) {
				doublesolenoid.set(DoubleSolenoid.Value.kForward);
			}
			else if (xboxController.getRawButton(1)) {
				doublesolenoid.set(DoubleSolenoid.Value.kReverse);
			}
			else {
				doublesolenoid.set(DoubleSolenoid.Value.kOff);
			}

		}
	}

	public void test() {
	}
}

Thanks in advance!

[MaGiC_PiKaChU]

the last line where you set your solenoid to kOff: it will cut the input pressure to your mechanism, meaning it will not hold the last position. you likely don't want this line

[Lesafian]

Originally Posted by MaGiC_PiKaChU

the last line where you set your solenoid to kOff: it will cut the input pressure to your mechanism, meaning it will not hold the last position. you likely don't want this line

Thank you, I'll fix that! As far as everything else does it look ok?

[Joey1939]

I see that you initialized the RobotDrive with the port numbers of your speed controllers. I believe that this will work correctly because you are using victors, but it would be better practice to initialize it with the two speed controllers you already initialized.

drive = new RobotDrive(drivermotor1, drivemotor2);

[RayG]

On the tank drive line, you want to be using axis 2 and 5 for left and right Y axis'.

[MaGiC_PiKaChU]

Originally Posted by Lesafian

Thank you, I'll fix that! As far as everything else does it look ok?

i guess the goal with using an axis for your sliding mechanism was to control the speed? Then you should use your joystick axis value instead of 1 and -1 (full speed)

also, your condition testing with Math.abs removes the negative sign, so your second if test will never work

This is how it could be made

// Move sliding mechanism forwards and backwards
			
                        //if your axis is not between -0.1 and 0.1
			if (Math.abs(xboxController.getRawAxis(5)) > .1) {
                               
                                //set your motor output to the axis value
				slideMotor.set(xboxController.getRawAxis(5));

			
			}else {
				slideMotor.set(0);
                        }

[Lesafian]

Won't this only work if I move the stick up? Shouldn't I need both > and < .1 if I'd like to both increase and decrease

Example of what I mean:

			if (Math.abs(xboxController.getRawAxis(5)) > .1) {
                               
                                //set your motor output to the axis value
				slideMotor.set(xboxController.getRawAxis(5));

			
			}else if (Math.abs(xboxController.getRawAxis(5)) < .1 {
                                slideMotor.set(xboxController.getRawAxis(5));
                        }

[soundfx]

Won't this only work if I move the stick up? Shouldn't I need both > and < .1 if I'd like to both increase and decrease

MaGiC_PiKaChU is correct. His code will test the absolute value for the threshold, then set the motor power to the joystick. Yours would probably work just as well, but the else if is redundant since it will never evaluate to true.

[Stimpy1901]

For us, although the code said "build successful" for code similar as shown below, it will throw an error (on the robot) and the driver station noted no code on the robot.

		drivemotor1 = new Victor(0);
		drivemotor2 = new Victor(2);
...
		drive = new RobotDrive(0, 2);

So use the correction shared by Joey:

Originally Posted by Joey1939

drive = new RobotDrive(drivermotor1, drivemotor2);

Once we did this change, everything worked fine.

[MaGiC_PiKaChU]

Originally Posted by Lesafian

Won't this only work if I move the stick up? Shouldn't I need both > and < .1 if I'd like to both increase and decrease

Example of what I mean:

			if (Math.abs(xboxController.getRawAxis(5)) > .1) {
                               
                                //set your motor output to the axis value
				slideMotor.set(xboxController.getRawAxis(5));

			
			}else if (Math.abs(xboxController.getRawAxis(5)) < .1 {
                                slideMotor.set(xboxController.getRawAxis(5));
                        }

No. The math.abs function transforms your value to positive.

if (Math.abs(xboxController.getRawAxis(5) < .1) is equivalent to if(xboxController.getRawAxis(5) < .1 && xboxController.getRawAxis(5) > -.1)

Let's say you put Math.abs(-0.05). that will output 0.05. I recommend you try it out and print in the Riolog. That saves you a few lines in your code, instead of testing for both positive and negative values.

[mmaunu]

Originally Posted by MaGiC_PiKaChU

No. The math.abs function transforms your value to positive.

if (Math.abs(xboxController.getRawAxis(5) < .1) is equivalent to if(xboxController.getRawAxis(5) < .1 || xboxController.getRawAxis(5) > -.1)

That should be an && (and) in your compound statement instead of an or.

[MaGiC_PiKaChU]

Originally Posted by mmaunu

That should be an && (and) in your compound statement instead of an or.

you're right. Fixed it