-
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?
Conversation
PPenguin1
commented
Nov 7, 2025

|
|
||
| private static RobotContainer instance; | ||
|
|
||
| // 10010111011011011011010010110101010101010101010101010 |
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.
0011011000110111
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.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
110 111
|
|
||
| private static RobotContainer instance; | ||
|
|
||
| // 10010111011011011011010010110101010101010101010101010 |
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.
random comment
| 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 comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is right
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.
no trust it is I verified it
infinite back(board basketball) shots
| @@ -0,0 +1,29 @@ | |||
| package frc.robot.elevator; | |||
|
|
|||
| public class EC | |||
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.
BAD NAMING should be ElevatorConstants
| 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 comment
The reason will be displayed to describe this comment to others. Learn more.
these two should be private
| 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 comment
The reason will be displayed to describe this comment to others. Learn more.
needs the rest of them and it should be 0
| } | ||
| } | ||
|
|
||
| public void m() |
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.
m
| } | ||
|
|
||
| public void setI(EC c) | ||
| { |
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.
i like the kamazee strat
| mainMotor.elevatorMotor2.setControl(positionRequest.withPosition(position)); | ||
| } | ||
|
|
||
| /* |
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.
🔥
|
|
||
| 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 comment
The reason will be displayed to describe this comment to others. Learn more.
Elevator
|
|
||
| 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 comment
The reason will be displayed to describe this comment to others. Learn more.
i think you need to add isEstopped
| public shooter(ElevatorIO io) { | ||
| this.io = io; | ||
| positionRequest = new MotionMagicVoltage(0.0, isEStopped, 0.0, 0, isEStopped, isEStopped, 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 comment
The reason will be displayed to describe this comment to others. Learn more.
i dont think you have isEstopped
| */ | ||
| public double getPosition(ElevatorIOTalonFX mainMotor) | ||
| { | ||
| return Math.sinh(12); |
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.
this dont seem right
| { | ||
| elevatorMotor1.optimizeBusUtilization(); | ||
| elevatorMotor2.optimizeBusUtilization(); | ||
| elevatorMotor1.getPosition().setUpdateFrequency(400); |
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.
seems a bit high
|
|
||
| public ElevatorIOTalonFX() | ||
| { | ||
| elevatorMotor1 = new TalonFX(101); |
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.
should use Ports.ELEVATOR_MOTOR or smth
|
|
||
| private TalonFXConfiguration config = new TalonFXConfiguration(); | ||
|
|
||
| public VoltageOut voltageRequest; |
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.
private
| * 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 comment
The reason will be displayed to describe this comment to others. Learn more.
bad punctionation literally no periods
|
|
||
| public Command zeroCommand(ElevatorIOTalonFX mainMotor) | ||
| { | ||
| return null; |
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.
return null
| public class EC | ||
| { | ||
| 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 comment
The reason will be displayed to describe this comment to others. Learn more.
needs public
| 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 comment
The reason will be displayed to describe this comment to others. Learn more.
all of these should be public
| default void updateInputs(ElevatorIOInputs inputs) | ||
| {} | ||
|
|
||
| // when the |
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.
when what?
| apriltags = at_detector.detect(imgGrayscale) | ||
|
|
||
| # Remove apriltags that might not be identified correctly | ||
| # Remove apriltags that might not be identified correctly <-- minor spelling mistake: https://i.imgflip.com/8i9uuh.jpg |
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.
twinkle twinkle little star
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.
i will hit you with my car
| d = new TalonFX(4); | ||
| config = TalonFX.getDefaultConfig(); | ||
| current = 0.0; | ||
| voltage = 24; |
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.
not sure about that
| private TalonFX c; | ||
| private TalonFX d; | ||
| private TalonFX config; | ||
| private int voltage; |
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.
double
| // Define methods for elevator hardware interaction here | ||
| @autolog | ||
| class TalonFX { | ||
| public IIOMetadata houyucsdbgdgljkhbgvrfakubhvkbafihkuarvhckavjhjkaghuicbgvyuhkcjnbhvgvyuihljbkvhgmyhuijknjbvcgvfyuhjbvgcfhtuo8ipjhvgcfdry7iuojilbkhuonjbkdstrtfgy = new IIOMetadata() { |
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.
name could use some improvment
| Double engorspDouble; | ||
| Enum p; | ||
| BiPredicate<Double, Double> engorspBiPredicate; | ||
| string google; |
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.
google and other bad variables
| { | ||
| } | ||
|
|
||
| default wingding(double Voltage){ |
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.
wingding
| @@ -0,0 +1,34 @@ | |||
| public class Train { | |||
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.
class Train
| @@ -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 comment
The reason will be displayed to describe this comment to others. Learn more.
java
| @@ -0,0 +1,34 @@ | |||
| public class Train { | |||
| public class train extends car { | |||
| public String voltage; | |||
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.
voltage shouldn't be a string
| } | ||
|
|
||
| public void periodic() { | ||
| io.updateInputs(null); |
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.
shouldn't be null
|
|
||
| } | ||
|
|
||
| setVel(DriveTrainTalonFX logan){ |
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.
no return type or scope + Logan
| }; | ||
|
|
||
| public record IO() { | ||
| Boolean engorspBoolean; |
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.
english or spanish
|
|
||
| import javax.imageio.metadata.IIOMetadata; | ||
|
|
||
| protected interface ElevatorIO { |
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.
publci
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.
also this is drivetrain no televator
| protected interface ElevatorIO { | ||
| // Define methods for elevator hardware interaction here | ||
| @autolog | ||
| class TalonFX { |
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.
not called talonFX
evil
| @autolog | ||
| class TalonFX { | ||
| public IIOMetadata houyucsdbgdgljkhbgvrfakubhvkbafihkuarvhckavjhjkaghuicbgvyuhkcjnbhvgvyuihljbkvhgmyhuijknjbvcgvfyuhjbvgcfhtuo8ipjhvgcfdry7iuojilbkhuonjbkdstrtfgy = new IIOMetadata() { | ||
| "W","L","FR" |
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.
doesnt match other data
| } | ||
|
|
||
| public double getRotation(DriveTrainTalonFX logan) { | ||
| return logan.getPosition(); |
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.
should be getPosition not getRotation
| Enum p; | ||
| BiPredicate<Double, Double> engorspBiPredicate; | ||
| string google; | ||
| Double Voltage; |
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.
all of these are wrong but voltage
|
|
||
| private void configureSecondaryBindings() | ||
| {} | ||
| // if youre reading this im going to touch you |
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.
charles this is very inappropriate
| "W","L","FR" | ||
| }; | ||
|
|
||
| public record IO() { |
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.
all of these should be paramaters in the () with commas
| } | ||
| } | ||
|
|
||
| default stop |
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.
needs ()
| { | ||
| } | ||
|
|
||
| default wingding(double Voltage){ |
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.
wind ding
| @@ -0,0 +1,34 @@ | |||
| public class Train { | |||
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.
i like trains
| @@ -0,0 +1,34 @@ | |||
| public class Train { | |||
| public class train extends car { | |||
| public String voltage; | |||
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.
strings are gross
| public class am-14 U4_PW4_Cons | ||
| { | ||
| PublicKey key; | ||
| pabeless |
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.
pabeless
| } | ||
|
|
||
| } | ||
| } |
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.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like something is wrong with this file
|
|
||
| } | ||
|
|
||
| setVel(DriveTrainTalonFX logan){ |
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.
im not a motor
| private am-14U4_PW4_Cons() { | ||
| } | ||
|
|
||
| long mainMotor; |
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.
should be TalonFX
| @@ -0,0 +1,34 @@ | |||
| public class Train { | |||
| public class train extends car { | |||
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.
ngl train isnt a type of car so like
| 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 comment
The reason will be displayed to describe this comment to others. Learn more.
get talonfx config