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

Intake #1

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
3 changes: 3 additions & 0 deletions src/main/java/frc/robot/Constants.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,7 @@
* constants are needed, to reduce verbosity.
*/
public final class Constants {
public static final class Intake {
public static final double POWER = 0.8; // [%]
}
}
63 changes: 63 additions & 0 deletions src/main/java/frc/robot/Intake.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package frc.robot;

import com.ctre.phoenix.motorcontrol.ControlMode;
import com.ctre.phoenix.motorcontrol.can.TalonSRX;
import edu.wpi.first.wpilibj.Solenoid;
import edu.wpi.first.wpilibj2.command.SubsystemBase;

/**
* Add&named motors in Intake.
*/
public class Intake extends SubsystemBase {
private TalonSRX motor = new TalonSRX(Ports.Intake.MOTOR);
private Solenoid piston = new Solenoid(Ports.Intake.SOLENOID);
Copy link
Member

Choose a reason for hiding this comment

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

Better to call this "retractor" or something similar, so that it's clear what is the piston's purpose.


/**
* Add motor inverted.
* Add data of is Motor inverted
*/
public Intake() {
motor.setInverted(Ports.Intake.IS_MOTOR_INVERTED);
}

/**
* Set percentage for motor.
* @param power the speed times the torque.
Copy link
Member

Choose a reason for hiding this comment

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

The units are simply %

*/
public void powerWheels(double power) {
motor.set(ControlMode.PercentOutput, power);
}

/**
* A check to see if piston is open or close, true= open, false= close.
* @return
*/
public boolean isPistonEngaged() {
Copy link
Member

Choose a reason for hiding this comment

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

As written above, it's better to name this function isRetracted or something similar. That way, the function's purpose is clear. Make sure you do the renaming everywhere else appropriate in this class.
Also, the piston is a hardware component, which means it is an implementation detail. Thus, it shouldn't be exposed to external users of the class, such as commands. Users of the class care about what it does, not how it does it.
Please reach out if the explanation above is not clear 🙂

return piston.get();
}

/**
* Set piston position.
* @param engaged
*/
public void setPistonMode(boolean engaged) {
piston.set(engaged);
}

/**
* Change piston position.
*/
public void toggle() {
if (isPistonEngaged()) {
setPistonMode(false);
} else {
setPistonMode(true);
}

}

}




Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}
}
}
}

No need for all these blank lines 🙂

14 changes: 14 additions & 0 deletions src/main/java/frc/robot/Ports.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package frc.robot;


/**
* Data of MOTOR & SOLENOID.
* Data of boolean MOTOR inverted.
*/
public class Ports {
public static class Intake {
public static final int MOTOR = 0;
public static final int SOLENOID = 0;
public static final boolean IS_MOTOR_INVERTED = true;
}
}
13 changes: 11 additions & 2 deletions src/main/java/frc/robot/RobotContainer.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,26 @@

package frc.robot;

import com.revrobotics.ColorSensorV3;
import edu.wpi.first.wpilibj.GenericHID;
import edu.wpi.first.wpilibj.XboxController;
import edu.wpi.first.wpilibj2.command.Command;
import frc.robot.subsystems.ExampleSubsystem.ExampleSubsystem;
import edu.wpi.first.wpilibj2.command.button.JoystickButton;
import frc.robot.subsystems.intake.commands.Commandy;


/**
* This class is where the bulk of the robot should be declared. Since Command-based is a
* "declarative" paradigm, very little robot logic should actually be handled in the {@link Robot}
* periodic methods (other than the scheduler calls). Instead, the structure of the robot
* (including subsystems, commands, and button mappings) should be declared here.
*/
public class RobotContainer {
public XboxController Xbox = new XboxController(1);
public JoystickButton a = new JoystickButton(Xbox, XboxController.Button.kA.value);
public JoystickButton b = new JoystickButton(Xbox, XboxController.Button.kB.value);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public XboxController Xbox = new XboxController(1);
public JoystickButton a = new JoystickButton(Xbox, XboxController.Button.kA.value);
public JoystickButton b = new JoystickButton(Xbox, XboxController.Button.kB.value);
public final XboxController Xbox = new XboxController(1);
public final JoystickButton a = new JoystickButton(Xbox, XboxController.Button.kA.value);
public final JoystickButton b = new JoystickButton(Xbox, XboxController.Button.kB.value);

Copy link
Member

Choose a reason for hiding this comment

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

(Also, make them private)

private final Intake intake= new Intake();


// The robot's subsystems and commands are defined here...


Expand All @@ -37,6 +45,7 @@ public RobotContainer() {
* {@link edu.wpi.first.wpilibj2.command.button.JoystickButton}.
*/
private void configureButtonBindings() {
a.whileHeld(new Commandy(intake, Constants.Intake.POWER));
}


Expand Down
50 changes: 50 additions & 0 deletions src/main/java/frc/robot/subsystems/intake/commands/Commandy.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package frc.robot.subsystems.intake.commands;

import edu.wpi.first.wpilibj2.command.CommandBase;
import frc.robot.Intake;

/**
* Add Commandy function;
*/
public class Commandy extends CommandBase {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a command that only opens and closes the intake

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, and add another command that powers the wheels. Then, if needed, combine both commands in a command group.
Also, change the command's name because "commandy" is very not indicative.

private final Intake intake;
private final double power;

/**
* Add Requirements
* @param intake name to Intake.
*/
public Commandy(Intake intake, double power) {
this.intake = intake;
this.power = power;
addRequirements(intake);

}


@Override
public void initialize() {
super.initialize();
Copy link
Member

@katzuv katzuv Sep 12, 2021

Choose a reason for hiding this comment

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

No need for this call to super's methods and also the ones in the methods following. As you can see in WPILib's source code, these methods are empty in the parent class, so these calls do nothing (source code taken from here):

public interface Command {
  /** The initial subroutine of a command. Called once when the command is initially scheduled. */
  default void initialize() {}


  /** The main body of a command. Called repeatedly while the command is scheduled. */
  default void execute() {}


  /**
   * The action to take when the command ends. Called when either the command finishes normally, or
   * when it interrupted/canceled.
   *
   * <p>Do not schedule commands here that share requirements with this command. Use {@link
   * #andThen(Command...)} instead.
   *
   * @param interrupted whether the command was interrupted/canceled
   */
  default void end(boolean interrupted) {}


  /**
   * Whether the command has finished. Once a command finishes, the scheduler will call its end()
   * method and un-schedule it.
   *
   * @return whether the command has finished.
   */
  default boolean isFinished() {
    return false;
  }

intake.setPistonMode(false);
}


@Override
public void execute() {
super.execute();
intake.powerWheels(power);
}

@Override
public boolean isFinished() {
return super.isFinished();
}


@Override
public void end(boolean interrupted) {
super.end(interrupted);
intake.powerWheels(0);
// intake.setPistonMode(true);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package frc.robot.subsystems.intake.commands;

import edu.wpi.first.wpilibj2.command.CommandBase;
import edu.wpi.first.wpilibj2.command.InstantCommand;
import frc.robot.Intake;

public class ToggleIntake extends InstantCommand {

private Intake intake;

public ToggleIntake(Intake intake) {
Copy link
Member

Choose a reason for hiding this comment

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

The should also move to specific state, not only toggle.

this.intake = intake;
addRequirements(intake);
}

@Override
public void initialize() {
super.initialize();
intake.toggle();
}

}