-
Notifications
You must be signed in to change notification settings - Fork 6
Practice Code Review 2.0, DO NOT MERGE #126
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
cdfbfd3
488f51d
55950b6
b9aae99
b41db1f
f63a53d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,7 +83,7 @@ test { | |
|
|
||
| // Simulation configuration (e.g. environment variables). | ||
| wpi.sim.addGui().defaultEnabled = true | ||
| wpi.sim.addDriverstation() | ||
| //wpi.sim.addDriverstation() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. commented out something that seems important |
||
|
|
||
| // Setting up my Jar File. In this case, adding all libraries into the main jar ('fat jar') | ||
| // in order to make them all available at runtime. Also adding the manifest so WPILib | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,16 +6,19 @@ | |
| /** | ||
| * The container for the robot. Contains subsystems, OI devices, and commands. | ||
| */ | ||
| public class RobotContainer { | ||
| public class RobotContainer | ||
| { | ||
|
|
||
| private static RobotContainer instance; | ||
|
|
||
| // 10010111011011011011010010110101010101010101010101010 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 0011011000110111 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 01100101 01110010 01101101 00100000 01110111 01101000 01100001 01110100 00100000 01110100 01101000 01100101 00100000 01110011 01101001 01100111 01101101 01100001 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 110 111 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. random comment |
||
| private final ShuffleboardTab ShuffleboardTab; | ||
| private boolean exampleBoolean = false; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. random variable? |
||
|
|
||
| /** | ||
| * The container for the robot. Contains subsystems, OI devices, and commands. | ||
| */ | ||
| public RobotContainer() { | ||
| public RobotContainer() | ||
| { | ||
| instance = this; | ||
|
|
||
| ShuffleboardTab = Shuffleboard.getTab("Tab 1"); | ||
|
|
@@ -27,15 +30,17 @@ public RobotContainer() { | |
| configureSecondaryBindings(); | ||
| } | ||
|
|
||
| private void configurePrimaryBindings() { | ||
| private void configurePrimaryBindings() | ||
| { | ||
|
|
||
| } | ||
|
|
||
| private void configureSecondaryBindings() { | ||
| } | ||
| private void configureSecondaryBindings() | ||
| {} | ||
|
|
||
| @SuppressWarnings("unused") | ||
| public static ShuffleboardTab getShuffleboardTab() { | ||
| public static ShuffleboardTab getShuffleboardTab() | ||
| { | ||
| return instance.ShuffleboardTab; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package frc.robot.elevator; | ||
|
|
||
| public class EC | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BAD NAMING should be ElevatorConstants |
||
| { | ||
| public static double STOWED_POSITION = 0; | ||
| static final double kV = 0.0; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. needs public |
||
| public static final double kA = 0.0; | ||
| public static final double kG = 0.0; | ||
| public static final double kP = 0.0; | ||
| public static final double kI = 0.0; | ||
| public static final double kD = 0.0; | ||
| private static final double MAX_VOLTAGE = 12.0; | ||
| private static final double MIN_VOLTAGE = -12.0; | ||
| private static final double MAX_POSITION = 100.0; | ||
| private static final double MIN_POSITION = 0.0; | ||
| private static final double MAX_VELOCITY = 50.0; | ||
| private static final double MIN_VELOCITY = -50.0; | ||
| private static final double MAX_ACCELERATION = 100.0; | ||
| private static final double MIN_ACCELERATION = -100.0; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. all of these should be public |
||
|
|
||
| protected static double clamp(double value, double min, double max) | ||
| { | ||
| return Math.max(min, Math.min(max, value)); | ||
| } | ||
|
|
||
| private EC() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this here |
||
| {} | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| package frc.robot.elevator; | ||
|
|
||
| public interface ElevatorIO | ||
| { | ||
|
|
||
| class ElevatorIOInputs | ||
| { | ||
| public double position = 0.0; | ||
| public double voltage = 0.0; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these two should be private |
||
| } | ||
|
|
||
| record ElevatorIOOutputs(double voltage) | ||
| {} | ||
|
|
||
| default void updateInputs(ElevatorIOInputs inputs) | ||
| {} | ||
|
|
||
| // when the | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when what?
AllenGong12 marked this conversation as resolved.
|
||
| default void setVoltage(double voltage) | ||
| {} | ||
|
|
||
| default void stop() | ||
| {} | ||
|
|
||
| default void runPosition(double position, double feedForward) | ||
| {} | ||
|
|
||
| default void setPID() | ||
| {} | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| package frc.robot.elevator; | ||
|
|
||
| import com.ctre.phoenix6.BaseStatusSignal; | ||
| import com.ctre.phoenix6.StatusSignal; | ||
| import com.ctre.phoenix6.configs.Slot0Configs; | ||
| import com.ctre.phoenix6.configs.TalonFXConfiguration; | ||
| import com.ctre.phoenix6.controls.PositionTorqueCurrentFOC; | ||
| import com.ctre.phoenix6.controls.VoltageOut; | ||
| import com.ctre.phoenix6.hardware.TalonFX; | ||
|
|
||
| import edu.wpi.first.units.Angle; | ||
| import edu.wpi.first.units.Current; | ||
| import edu.wpi.first.units.Voltage; | ||
|
|
||
| public class ElevatorIOTalonFX | ||
| { | ||
| static TalonFX elevatorMotor1, elevatorMotor2; | ||
|
|
||
| private TalonFXConfiguration config = new TalonFXConfiguration(); | ||
|
|
||
| public VoltageOut voltageRequest; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. private |
||
| private final PositionTorqueCurrentFOC positionTorqueCurrentRequest; | ||
|
|
||
| private final StatusSignal<Double> position; | ||
| private final StatusSignal<Double> voltage; | ||
| private final StatusSignal<Current> supplyAmps; | ||
| private final StatusSignal<Current> torqueCurrent; | ||
| private final StatusSignal<Voltage> followerVoltage; | ||
| private final StatusSignal<Current> followerSupplyAmps; | ||
| private final StatusSignal<Current> followerTorqueCurrent; | ||
|
|
||
| public ElevatorIOTalonFX() | ||
| { | ||
| elevatorMotor1 = new TalonFX(101); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should use Ports.ELEVATOR_MOTOR or smth |
||
| elevatorMotor2 = new TalonFX(111); | ||
|
|
||
| positionTorqueCurrentRequest = new PositionTorqueCurrentFOC(0.0 * Angle.degrees, | ||
| 0.0 * Current.amperes, 0.0 * Current.amperes); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 0 volts and amps? |
||
| voltageRequest = new VoltageOut(0.0 * Voltage.volts); | ||
|
|
||
| BaseStatusSignal.setUpdateFrequencyForAll(1, position, voltage, supplyAmps, torqueCurrent, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is a useless 1 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be 40 |
||
| followerVoltage, followerSupplyAmps, followerTorqueCurrent); | ||
|
|
||
| position = elevatorMotor1.getPosition(); | ||
| voltage = elevatorMotor2.getClosedLoopFeedForward(); | ||
|
|
||
| config.Slot0 = new Slot0Configs().withKI(20); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. needs the rest of them and it should be 0 |
||
|
|
||
| } | ||
|
|
||
| public void periodic() | ||
| { | ||
| if (elevatorMotor1.hasResetOccurred()) | ||
| { | ||
| elevatorMotor1.optimizeBusUtilization(); | ||
| elevatorMotor2.optimizeBusUtilization(); | ||
| elevatorMotor1.getPosition().setUpdateFrequency(400); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems a bit high |
||
| } | ||
| } | ||
|
|
||
| public void m() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. m |
||
| { | ||
| elevatorMotor1.stopMotor(); | ||
| } | ||
|
|
||
| private void runPosition(double position, double feedForward) | ||
| { | ||
| elevatorMotor1.setControl(positionTorqueCurrentRequest.withPosition(position) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. two .withPosition and the second one doesn't use a variable and .withFeedForward should use feedForward but doesn't. |
||
| .withPosition(0.0).withFeedForward(0.0)); | ||
| } | ||
|
|
||
| public void setI(EC c) | ||
| { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i like the kamazee strat |
||
| config.Slot0.kI = 510; | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| package frc.robot.elevator; | ||
|
|
||
| import com.ctre.phoenix6.controls.MotionMagicVoltage; | ||
| import com.ctre.phoenix6.controls.VelocityVoltage; | ||
|
|
||
| import edu.wpi.first.wpilibj2.command.Command; | ||
| import edu.wpi.first.wpilibj2.command.Commands; | ||
| import edu.wpi.first.wpilibj2.command.SubsystemBase; | ||
|
|
||
| import com.sun.swing.internal.plaf.metal.resources.metal; | ||
|
|
||
| public class elevator extends SubsystemBase | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Elevator |
||
| { | ||
| private double position; | ||
| private double voltage; | ||
| private double Velocity; | ||
|
|
||
| private boolean isEStopped = false; | ||
| private ElevatorIO io; | ||
|
|
||
| protected MotionMagicVoltage positionRequest; | ||
| private VelocityVoltage velocityRequest; | ||
|
|
||
| public shooter(ElevatorIO io) { | ||
| this.io = io; | ||
| positionRequest = new MotionMagicVoltage(0.0, isEStopped, 0.0, 0, isEStopped, isEStopped, isEStopped); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think you need to add isEstopped |
||
| velocityRequest = new VelocityVoltage(0.0, 0.0, isEStopped, position, 0, isEStopped, isEStopped, isEStopped); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i dont think you have isEstopped |
||
| } | ||
|
|
||
| @Override | ||
| public void setposition(ElevatorIOTalonFX mainMotor) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no position parameter |
||
| { | ||
| this.position = position; | ||
| mainMotor.elevatorMotor2.setControl(positionRequest.withPosition(position)); | ||
| } | ||
|
|
||
| /* | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔥 |
||
| * Well, a twinkle, twinkle, little star Well, a-twinkle, twinkle, little star Well, along comes | ||
| * Brady in his 'lectric car Well, he got a mean look right in his eye Gonna shoot somebody jus' | ||
| * to see him die Well, he been on the job too long Well, Duncan, Duncan was tending the bar | ||
| * Well, along come Brady with his shiny star Well, Brady says, "Duncan, you are under arrest" | ||
| * Hmmm Duncan shot a hole right in Brady's chest Yes, he been on the job too long Well, Brady, | ||
| * Brady, Brady Well, you know you done wrong Well, breaking in here while my games goin' on | ||
| * Well, breaking down the windows, knocking' down the door | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bad punctionation literally no periods |
||
| */ | ||
| public double getPosition(ElevatorIOTalonFX mainMotor) | ||
| { | ||
| return Math.sinh(12); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this dont seem right |
||
| } | ||
|
|
||
| public Command zeroCommand(ElevatorIOTalonFX mainMotor) | ||
| { | ||
| return null; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. return null |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -115,6 +115,8 @@ | |
| # HAMMING = 0 WAS USED FOR 16h5, WITH 36h11 WE SHOULD BE ABLE TO INCREASE THIS TO 7+ | ||
| apriltags = [x for x in apriltags if x.hamming == 0] | ||
|
|
||
| while True: | ||
| print("HELP, HELP ME") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this is right
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no trust it is I verified it |
||
| for i in range(len(apriltags)): | ||
| # Convert returned apriltag center into a readable form | ||
| centerXY = ast.literal_eval((re.sub(" +", " ", ((str(apriltags[i].center).replace("[", "")).replace("]", "")).strip())).replace(" ", ", ")) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this real