Skip to content

Conversation

@AdamEXu
Copy link

@AdamEXu AdamEXu commented Jan 16, 2026

No description provided.

Copilot AI review requested due to automatic review settings January 16, 2026 01:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request implements a swerve drive subsystem for an FRC robot, supporting two robot variants (ABOT with TalonFX motors and BBOT with Spark motors). It includes hardware abstraction layers, motor controller configurations, and teleop control, along with configuration updates for cameras, AprilTag detection, and pathfinding systems.

Changes:

  • Added swerve drive subsystem with support for dual robot variants using different motor controllers
  • Implemented hardware classes for TalonFX and Spark-based swerve modules with motor configurations
  • Updated TypeScript configuration files for cameras, AprilTag detection, LIDAR, and pathfinding
  • Added teleop command for joystick-controlled swerve driving with gyro-relative field orientation

Reviewed changes

Copilot reviewed 10 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/main/java/frc/robot/subystem/SwerveSubsystem.java Main swerve subsystem managing four swerve modules with robot variant selection
src/main/java/frc/robot/hardware/WheelMoverTalonFX.java TalonFX motor controller implementation for swerve modules
src/main/java/frc/robot/hardware/WheelMoverSpark.java Spark motor controller implementation for swerve modules
src/main/java/frc/robot/hardware/WheelMoverBase.java Base class providing common interface for swerve module implementations
src/main/java/frc/robot/constant/swerve/*.java Swerve constants for both robot variants with motor ports, PID values, and geometry
src/main/java/frc/robot/constant/BotConstants.java Robot variant and mode configuration
src/main/java/frc/robot/command/SwerveMoveTeleop.java Teleop command for joystick control with deadbanding
src/main/java/frc/robot/RobotContainer.java Robot container with swerve subsystem initialization and button bindings
src/config/* Updated camera, AprilTag, LIDAR, and pathfinding configurations
build.gradle Added configuration generation and Pi count constant
.gitignore Added gradle-tmp directory to ignore list

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +213 to +215
Logger.recordOutput(base + "requestedMps", mpsSpeed.in(Units.MetersPerSecond));
Logger.recordOutput(base + "requestedAngle", requestedAngle.in(Units.Degrees));
Logger.recordOutput(base + "requestedDistance", distance.in(Units.Meters));
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The logging is incorrect. Line 213 logs the actual speed (mpsSpeed from getSpeed()) as "requestedMps", but it should log the parameter requestedMps. Similarly, line 215 logs the actual distance as "requestedDistance", but this should be logging a requested value or have a more appropriate label like "actualDistance".

Suggested change
Logger.recordOutput(base + "requestedMps", mpsSpeed.in(Units.MetersPerSecond));
Logger.recordOutput(base + "requestedAngle", requestedAngle.in(Units.Degrees));
Logger.recordOutput(base + "requestedDistance", distance.in(Units.Meters));
Logger.recordOutput(base + "requestedMps", requestedMps.in(Units.MetersPerSecond));
Logger.recordOutput(base + "requestedAngle", requestedAngle.in(Units.Degrees));
Logger.recordOutput(base + "actualDistance", distance.in(Units.Meters));

Copilot uses AI. Check for mistakes.

turnCANcoder = new CANcoder(CANCoderEncoderChannel);
CANcoderConfiguration config = new CANcoderConfiguration();
config.MagnetSensor.MagnetOffset = -CANCoderMagnetOffset;
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

There is an inconsistency in how the CANCoder magnet offset is applied. WheelMoverTalonFX negates the offset (line 55: -CANCoderMagnetOffset), while WheelMoverSpark does not negate it (line 94: magnetOffset). This could lead to incorrect encoder readings for one of the robot variants, causing the swerve modules to point in the wrong direction.

Suggested change
config.MagnetSensor.MagnetOffset = -CANCoderMagnetOffset;
config.MagnetSensor.MagnetOffset = CANCoderMagnetOffset;

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,259 @@
package frc.robot.subystem;
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The package name "subystem" is misspelled. It should be "subsystem" to follow standard Java naming conventions.

Suggested change
package frc.robot.subystem;
package frc.robot.subsystem;

Copilot uses AI. Check for mistakes.
public static final int kRearRightTurningMotorPort = 23;
public static final int kRearLeftTurningMotorPort = 22;

// whether the turning enoders are flipped
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The word "enoders" is misspelled in the comment. It should be "encoders".

Suggested change
// whether the turning enoders are flipped
// whether the turning encoders are flipped

Copilot uses AI. Check for mistakes.
public static final int kRearLeftTurningMotorPort = 10;
public static final int kRearRightTurningMotorPort = 12;

// whether the turning enoders are flipped
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The word "enoders" is misspelled in the comment. It should be "encoders".

Suggested change
// whether the turning enoders are flipped
// whether the turning encoders are flipped

Copilot uses AI. Check for mistakes.
public int kRearLeftTurningMotorPort;
public int kRearRightTurningMotorPort;

// whether the turning enoders are flipped
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The word "enoders" is misspelled in the comment. It should be "encoders".

Suggested change
// whether the turning enoders are flipped
// whether the turning encoders are flipped

Copilot uses AI. Check for mistakes.
Comment on lines +206 to +216

private void logEverything(LinearVelocity requestedMps, Angle requestedAngle) {
String base = "Wheels/" + port + "/";
LinearVelocity mpsSpeed = getSpeed();
Angle newRotationRad = getAngle();
Distance distance = getDistance();

Logger.recordOutput(base + "requestedMps", mpsSpeed.in(Units.MetersPerSecond));
Logger.recordOutput(base + "requestedAngle", requestedAngle.in(Units.Degrees));
Logger.recordOutput(base + "requestedDistance", distance.in(Units.Meters));
}
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The private method logEverything is defined but never called. WheelMoverSpark overrides the drive method to call logEverything, but WheelMoverTalonFX does not. Either override the drive method to call logEverything, or remove this unused method.

Suggested change
private void logEverything(LinearVelocity requestedMps, Angle requestedAngle) {
String base = "Wheels/" + port + "/";
LinearVelocity mpsSpeed = getSpeed();
Angle newRotationRad = getAngle();
Distance distance = getDistance();
Logger.recordOutput(base + "requestedMps", mpsSpeed.in(Units.MetersPerSecond));
Logger.recordOutput(base + "requestedAngle", requestedAngle.in(Units.Degrees));
Logger.recordOutput(base + "requestedDistance", distance.in(Units.Meters));
}

Copilot uses AI. Check for mistakes.
@AdamEXu
Copy link
Author

AdamEXu commented Jan 20, 2026

added a bunch of stuff

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.

3 participants