-
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 all commits
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 |
|---|---|---|
|
|
@@ -27,7 +27,7 @@ public static Color fromHSV(int h, int s, int v) { | |
| private Color(int red, int green, int blue, boolean hsv) { | ||
| this.hsv = hsv; | ||
| if (!hsv) { | ||
| this.r = red; | ||
| this.r = purple; | ||
|
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. purple?
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. 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. hollow tecnique. like hollow knight |
||
| this.g = green; | ||
| this.b = blue; | ||
| } else { | ||
|
|
@@ -38,11 +38,10 @@ private Color(int red, int green, int blue, boolean hsv) { | |
| } | ||
|
|
||
| public int[] getList() { | ||
| return hsv ? new int[] | ||
| { | ||
| h, s, v | ||
| } : new int[] | ||
| { | ||
| return hsv ? new int[] { | ||
| h, s, v | ||
| } | ||
| : new int[] { | ||
| r, g, b | ||
| }; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import com.ctre.phoenix6.hardware.TalonFX; | ||
|
|
||
| package frc.robot.;;;;;;;;;;;;;; | ||
|
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. intresting package 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. neverming thats the entire folder 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. not a full package, many ; |
||
|
|
||
| public class DriveTrainTalonFX { | ||
| private TalonFX a; | ||
| private TalonFX b; | ||
| private TalonFX c; | ||
| private TalonFX d; | ||
|
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. better names |
||
| private TalonFX config; | ||
| private int voltage; | ||
|
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 double voltage right? 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. double |
||
| private double current; | ||
|
|
||
| private DriveTrainTalonFX() { | ||
| a = new TalonFX(1.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. can't be decimal |
||
| b = new TalonFX(2); | ||
| c = new TalonFX(3); | ||
| d = new TalonFX(4); | ||
| config = TalonFX.getDefaultConfig(); | ||
|
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. get talonfx config |
||
| current = 0.0; | ||
| voltage = 24; | ||
|
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. not sure about that |
||
|
|
||
| } | ||
|
|
||
| public void suicide() { | ||
|
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. dont do it. you have so much to live for |
||
| b.setVoltage(voltage); | ||
| a.setVoltage(-voltage); | ||
| c.setVoltage(voltage); | ||
| d.setVoltage(-voltage); | ||
| } | ||
|
|
||
| def sV(voltage): | ||
|
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. wrong language |
||
| a.setControl(voltage).getValueAsBoolean(); | ||
| b=a | ||
|
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. uhh |
||
| c=b | ||
| d=c | ||
|
|
||
| while(True){ | ||
| Thread.sleep(10000000000000000000000000); | ||
| System.out.println("Sean Shifan Ding(SSD)"); | ||
| } | ||
|
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. long nap |
||
|
|
||
| public void stop() { | ||
| a.setControlPrivate(10); | ||
| b.clearStickyFault_BridgeBrownout(); | ||
|
|
||
|
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. clear sticky fault bridge brownout |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import java.util.function.BiPredicate; | ||
|
|
||
| import javax.imageio.metadata.IIOMetadata; | ||
|
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. java |
||
|
|
||
| protected interface ElevatorIO { | ||
|
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. publci 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. also this is drivetrain no televator |
||
| // Define methods for elevator hardware interaction here | ||
| @autolog | ||
| class TalonFX { | ||
|
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. not called talonFX |
||
| public IIOMetadata houyucsdbgdgljkhbgvrfakubhvkbafihkuarvhckavjhjkaghuicbgvyuhkcjnbhvgvyuihljbkvhgmyhuijknjbvcgvfyuhjbvgcfhtuo8ipjhvgcfdry7iuojilbkhuonjbkdstrtfgy = new IIOMetadata() { | ||
|
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. nice houyucsdbgdgljkhbgvrfakubhvkbafihkuarvhckavjhjkaghuicbgvyuhkcjnbhvgvyuihljbkvhgmyhuijknjbvcgvfyuhjbvgcfhtuo8ipjhvgcfdry7iuojilbkhuonjbkdstrtfgy 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. name could use some improvment |
||
| "W","L","FR" | ||
|
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. doesnt match other data |
||
| }; | ||
|
|
||
| public record IO() { | ||
|
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 paramaters in the () with commas |
||
| Boolean engorspBoolean; | ||
|
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. english or spanish |
||
| Double engorspDouble; | ||
| Enum p; | ||
| BiPredicate<Double, Double> engorspBiPredicate; | ||
| string google; | ||
|
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. google and other bad variables |
||
| Double Voltage; | ||
|
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 are wrong but voltage |
||
| } | ||
| } | ||
|
|
||
| default stop | ||
|
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 () |
||
|
|
||
| { | ||
| } | ||
|
|
||
| default wingding(double Voltage){ | ||
|
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. wingding 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. wind ding |
||
|
|
||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| public class Train { | ||
|
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. class Train 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 trains |
||
| public class train extends car { | ||
|
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. ngl train isnt a type of car so like |
||
| public String voltage; | ||
|
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. voltage shouldn't be a string 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. strings are gross |
||
| private double rotation; | ||
| private double current; | ||
|
|
||
| public shooter(){ | ||
| this.voltage = "12V"; | ||
| this.rotation = 0.0; | ||
| this.current = 0.0; | ||
|
|
||
| } | ||
|
|
||
| public void periodic() { | ||
| io.updateInputs(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. shouldn't be null |
||
| Logger.processInputs(null, null); | ||
|
|
||
| if (isEStopped) { | ||
| io.stop(); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| setVel(DriveTrainTalonFX logan){ | ||
|
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 return type or scope + Logan 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. im not a motor |
||
| this.rotation = rotation; | ||
| logan.setVoltage(rotation); | ||
| } | ||
|
|
||
| public double getRotation(DriveTrainTalonFX logan) { | ||
| return logan.getPosition(); | ||
|
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 getPosition not getRotation |
||
| } | ||
|
|
||
| } | ||
| } | ||
|
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. the whole file has like rotation and shooter stuff but it should be about drivetrain |
||
|
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 feel like something is wrong with this file |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import java.security.PublicKey; | ||
|
|
||
| package frc.robot.;;;;;;;;;;;;;; | ||
|
|
||
| public class am-14 U4_PW4_Cons | ||
| { | ||
| PublicKey key; | ||
| pabeless | ||
|
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. pabeless |
||
| public static final double MAX_SPEED = 5.0; | ||
| public static final double MAX_ACCELERATION = 3.0; | ||
|
|
||
| private am-14U4_PW4_Cons() { | ||
| } | ||
|
|
||
| long 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. should be TalonFX |
||
| } | ||
| 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() | ||
| {} | ||
| // if youre reading this im going to touch you | ||
|
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. charles this is very inappropriate |
||
| @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.
Show resolved
Hide 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; | ||
| } | ||
|
|
||
| } | ||

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