Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Elevator #8

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Elevator #8

wants to merge 14 commits into from

Conversation

Jhausgaard
Copy link

Made a few changes regarding "lift" and making it "bar" and "lift" actually refer to the entire "elevator"

MasterThrasher and others added 8 commits January 26, 2019 11:50
…wn, but just about everything will change with the "lift" system as the names of the motors are changing.
…ith the overall elevator and four bar lift to "Lift" and anything that was previously "Lift" to "Bar."
…tLiftSetpoint (both of them) work with it.
.idea/misc.xml Outdated
@@ -3,5 +3,5 @@
<component name="JavaScriptSettings">
<option name="languageLevel" value="JSX" />
</component>
<component name="ProjectRootManager" version="2" languageLevel="JDK_11" default="false" project-jdk-name="11" project-jdk-type="JavaSDK" />
<component name="ProjectRootManager" version="2" languageLevel="JDK_1_8" default="false" project-jdk-name="1.8" project-jdk-type="JavaSDK" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be reverted.

@@ -20,6 +21,10 @@
public class Robot extends TimedRobot {
private static final String kDefaultAuto = "Default";
private static final String kCustomAuto = "My Auto";

private int ticks = 0;
private long lastTime = System.currentTimeMillis();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These need comments as to what they are for....

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed

@@ -4,7 +4,7 @@
import org.techvalleyhigh.frc5881.deepspace.robot.Robot;

/**
* Changes the elevator height
* Changes the elevators height accordingly to the expected game piece to be receiving
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs better explanation... I still don't know what this does from this description.

@@ -18,18 +18,16 @@ public ElevatorFlip() {
@Override
protected void initialize() {
System.out.println("Elevator flip initialized");
Robot.elevator.elevatorFlip();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can't be here... If this command is on a button (and I think it is) this code will execute as soon as you attach the command to the button -- NOT when the button is pressed. That's what execute() is for.

}

@Override
protected void execute() {
if(!Robot.elevator.isFired){
Robot.elevator.elevatorFlip();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be put back -- sort of. (Not sure what isFired is but....)

You need to put a boolean in the command to track to see if the command has already looped once through execute, (Default it to false, if false call .elevatorFlip() and set it to true, and in end() set it back to false. Just like you did in ElevatorDown.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be pulled out, so this is ok... but what is isFired doing? (And why is it being directly accessed?)

@@ -1,15 +1,13 @@
package org.techvalleyhigh.frc5881.deepspace.robot.commands.groups;

import edu.wpi.first.wpilibj.command.CommandGroup;
import org.techvalleyhigh.frc5881.deepspace.robot.commands.arm.ArmFlip;
import org.techvalleyhigh.frc5881.deepspace.robot.commands.elevator.ElevatorFlip;

/**
* Will change the mode of the arm.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment uses wrong term.

@@ -18,8 +18,8 @@
public class Climber extends Subsystem {
public static DoubleSolenoid frontSolenoid = new DoubleSolenoid(20, 2, 3);
public static DoubleSolenoid backSolenoid = new DoubleSolenoid(20, 4, 5);
public static WPI_TalonSRX leftMotor = new WPI_TalonSRX(5);
public static WPI_TalonSRX rightMotor = new WPI_TalonSRX(6);
//public static WPI_TalonSRX leftMotor = new WPI_TalonSRX(16);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are all the lines here commented out?

@@ -17,8 +20,9 @@
// Define motors
public static WPI_TalonSRX frontLeftMotor = new WPI_TalonSRX(1);
public static WPI_TalonSRX frontRightMotor = new WPI_TalonSRX(2);
public static WPI_TalonSRX backLeftMotor = new WPI_TalonSRX(3);
// public static WPI_TalonSRX backLeftMotor = new WPI_TalonSRX(3);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this commented out?

backLeftMotor.setName("Drive", "Back Left");
backLeftMotor.set(ControlMode.Follower, 1);
LiveWindow.add(backLeftMotor);
// backLeftMotor.setName("Drive", "Back Left");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, why commented out?

}

@Override
protected void execute() {
if(!Robot.elevator.isFired){
Robot.elevator.elevatorFlip();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be pulled out, so this is ok... but what is isFired doing? (And why is it being directly accessed?)


public ElevatorDown() {
public class LiftDown extends Command {
private boolean isFirstRun = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed now

@@ -26,19 +26,23 @@ protected void initialize() {
*/
@Override
protected void execute() {
Robot.elevator.elevatorDown();
if(isFirstRun) {
Robot.elevator.elevatorDown();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the condition and move to initialize()

@@ -33,7 +33,7 @@ protected void execute() {
*/
@Override
protected boolean isFinished() {
return Robot.elevator.isSetpointReached();
return Robot.navX.getRawGyroY() < 30;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This expression should probably be an isRobotTipping() method somewhere in case we want to change the angle...

public class ElevatorUp extends Command {
public ElevatorUp() {
public class LiftUp extends Command {
private boolean isFirstRun;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed now

@@ -24,7 +26,10 @@ protected void initialize() {
*/
@Override
protected void execute() {
Robot.elevator.elevatorUp();
if(isFirstRun) {
Robot.elevator.elevatorUp();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove condition and move to initialize()

Jacob and others added 5 commits February 15, 2019 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants