Event lakeview#12
Merged
Merged
Conversation
…2_Rebuilt into event_lakeview
…2_Rebuilt into event_lakeview
…2_Rebuilt into event_lakeview
…2_Rebuilt into event_lakeview
📝 WalkthroughWalkthroughUpdates robot configuration and control systems including a new UI template for AdvantageKit, new autonomous routines, increased path velocity constraints (4.0→4.5 m/s across 30+ paths), new command groups for climber safety, expanded current limit protections across motor subsystems, and enhanced driver/co-driver controls with rumble and toggle features. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/frc/alotobots/Constants.java (1)
32-38:⚠️ Potential issue | 🟠 Major
Mode.SIMbecomes unreachable and breaks simulation-specific logic.Setting
simMode = Mode.REPLAYon line 32 forces all non-real launches intoREPLAYmode, making the five existing checks forMode.SIMunreachable:
- Robot.java (lines 213, 221): SIM initialization and periodic simulation updates never run
- RobotContainer.java (lines 494, 501): SIM field reset and display never execute
- SwerveDriveSubsystem.java (line 211): Gyro disconnect alert cannot suppress on SIM
This also changes the tuner selection on line 65—desktop runs now always use
TunerConstants2026()instead of allowing theSIMpath to useTunerConstants2025().If
REPLAYis the intended default, remove theMode.SIMenum and its dead code paths. Otherwise, changesimModetoMode.SIMand gateREPLAYbehind a launch flag.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/frc/alotobots/Constants.java` around lines 32 - 38, The constant simMode is set to Mode.REPLAY which makes Mode.SIM code paths unreachable; change simMode to Mode.SIM (or alternatively gate Mode.REPLAY behind a launch flag) so that currentMode = RobotBase.isReal() ? Mode.REAL : simMode correctly yields SIM for desktop/simulation runs; update references that depend on Mode.SIM (Robot.java SIM initialization/periodic branches, RobotContainer SIM field reset/display, SwerveDriveSubsystem gyro disconnect suppression) and ensure tuner selection logic uses TunerConstants2025 for SIM and TunerConstants2026 only when REPLAY is explicitly requested.src/main/deploy/pathplanner/paths/Postions.path (1)
1-54:⚠️ Potential issue | 🟡 MinorFilename typo: "Postions.path" should be "Positions.path".
The filename contains a typo ("Postions" instead of "Positions"), which is inconsistent with the folder name on line 48 (
"folder": "Positions"). This could cause confusion when searching for or referencing this path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/deploy/pathplanner/paths/Postions.path` around lines 1 - 54, The file is misnamed "Postions.path" (typo) while its internal "folder" is "Positions"; rename the file from "Postions.path" to "Positions.path" and update any references/imports that point to "Postions.path" so they use "Positions.path" instead, and verify the string "folder": "Positions" remains unchanged in the file.
🧹 Nitpick comments (11)
src/main/deploy/pathplanner/paths/New New New New New Path.path (1)
1-1: Rename placeholder file to a meaningful name.The filename
New New New New New Path.pathappears to be a placeholder or iterative test name. Consider renaming it to describe its purpose (e.g., based on the linked waypoints "Neutral Zone Depot 2" → "Neutral Zone Depot 1").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/deploy/pathplanner/paths/New` New New New New Path.path at line 1, Rename the placeholder file "New New New New New Path.path" to a descriptive name reflecting its purpose and linked waypoints (for example "NeutralZoneDepot2_to_NeutralZoneDepot1.path" or similar), update any references to this filename in the repo if present, and ensure the new name follows project naming conventions; target the file currently containing "{" (the placeholder path file) and use the waypoint names "Neutral Zone Depot 2" and "Neutral Zone Depot 1" to construct the new filename.src/main/java/frc/alotobots/rebuilt/subsystems/launcher/turret/constants/TurretTalonFXSConstants.java (1)
41-41: Minor: Add leading zero for consistency.The constant
.972is missing a leading zero. All other decimal constants in this file use leading zeros (e.g.,0.0,0.36,1.75).Suggested fix
- public static final double TURRET_VELOCITY_KP = .972; + public static final double TURRET_VELOCITY_KP = 0.972;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/frc/alotobots/rebuilt/subsystems/launcher/turret/constants/TurretTalonFXSConstants.java` at line 41, The constant TURRET_VELOCITY_KP in class TurretTalonFXSConstants is written as .972 without a leading zero; update its literal to 0.972 to match the file's numeric formatting style and maintain consistency with other constants (e.g., 0.0, 0.36, 1.75).src/main/deploy/pathplanner/paths/Positions.path (1)
35-36: This velocity edit is a no-op while default constraints stay enabled.With
useDefaultConstraintsstilltrueon Line 53, PathPlanner uses the project defaults for this path, and its docs note that updating the defaults updates any paths set to use them. So changing Line 36 here is either redundant or ineffective for a path-specific override. If this path really needs its own 4.5 m/s cap, setuseDefaultConstraintstofalse; otherwise keepSettings.jsonas the single source of truth. (pathplanner.dev)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/deploy/pathplanner/paths/Positions.path` around lines 35 - 36, The change to "maxVelocity" inside the "globalConstraints" block is ineffective while useDefaultConstraints remains true; either disable the defaults or remove the redundant override: if you want this path to use a path-specific cap, set useDefaultConstraints to false and keep globalConstraints.maxVelocity = 4.5 for this path (reference symbols: useDefaultConstraints, globalConstraints, maxVelocity), otherwise revert the maxVelocity edit and maintain the project-wide value in Settings.json as the single source of truth.src/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/ShooterSubsystem.java (1)
122-136: Minor naming inconsistency.The method
sysIdRvsQuasiStatic(line 134) uses uppercaseSin "Static", whilesysIdFwdQuasistatic(line 130) uses lowercases. Consider making these consistent.♻️ Consistent naming
public Command sysIdFwdQuasistatic() { return sysIdRoutine.quasistatic(SysIdRoutine.Direction.kForward); } - public Command sysIdRvsQuasiStatic() { + public Command sysIdRvsQuasistatic() { return sysIdRoutine.quasistatic(SysIdRoutine.Direction.kReverse); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/ShooterSubsystem.java` around lines 122 - 136, The two similar methods in ShooterSubsystem have inconsistent naming: sysIdFwdQuasistatic vs sysIdRvsQuasiStatic; rename sysIdRvsQuasiStatic to sysIdRvsQuasistatic to match the lowercase "s" style (or vice‑versa if you prefer the Uppercase S, but keep both consistent), and update all references/usages and any Javadoc/comments accordingly (methods: sysIdFwdQuasistatic, sysIdRvsQuasistatic; class: ShooterSubsystem; calls to sysIdRoutine.quasistatic(SysIdRoutine.Direction...)).src/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/io/ShooterIOTalonFX.java (1)
227-230: Consider explicitly stopping the follower motor for robustness.While Phoenix6 documentation confirms that
stopMotor()on the leader sets the output to neutral/0, and the Follower should copy this state, explicitly stoppingmotorRightis recommended for clarity and safety. Phoenix6 best practices suggest being explicit about stopping followers rather than relying on implicit propagation of the leader's neutral state.`@Override` public void stop() { motorLeft.stopMotor(); motorRight.stopMotor(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/io/ShooterIOTalonFX.java` around lines 227 - 230, The stop() method currently only calls motorLeft.stopMotor(); explicitly also call motorRight.stopMotor() to ensure the follower is stopped; update the ShooterIOTalonFX.stop() method to invoke motorLeft.stopMotor() and motorRight.stopMotor() (referencing the motorLeft and motorRight fields) so the follower is explicitly stopped rather than relying on implicit leader propagation.src/main/java/frc/alotobots/rebuilt/commands/groups/ClimberUpWithTurretSafety.java (1)
28-33: Verify parallel execution is safe with turret timeout.The turret command has a 1-second timeout while the climber extends in parallel. If the turret doesn't reach the safety position before timeout, the climber will continue extending regardless.
Consider whether a
SequentialCommandGroup(turret reaches position first, then climber extends) would provide a safer guarantee against potential turret-climber collision, or confirm that 1 second is sufficient for the turret to reach the safety angle under all conditions.Alternative: Sequential execution for guaranteed safety
-public class ClimberUpWithTurretSafety extends ParallelCommandGroup { +public class ClimberUpWithTurretSafety extends SequentialCommandGroup { public ClimberUpWithTurretSafety( ClimberSubsystem climberSubsystem, TurretSubsystem turretSubsystem) { addCommands( new TurretRunToPosition( - turretSubsystem, TurretConstants.Setpoints.SAFETY_TURRET_ANGLE_CLIMB_UP) - .withTimeout(Seconds.of(1)), + turretSubsystem, TurretConstants.Setpoints.SAFETY_TURRET_ANGLE_CLIMB_UP), new ClimberRunToExtension(climberSubsystem, ClimberConstants.Limits.MAX_CLIMB_EXTENSION)); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/frc/alotobots/rebuilt/commands/groups/ClimberUpWithTurretSafety.java` around lines 28 - 33, The current ClimberUpWithTurretSafety group uses addCommands() to run TurretRunToPosition(withTimeout(Seconds.of(1))) in parallel with ClimberRunToExtension, which can let the climber continue if the turret times out; change this to guarantee the turret reaches the safety angle before the climber extends by making the turret command run first and only then starting the climber (e.g., replace the parallel addCommands usage in ClimberUpWithTurretSafety with a SequentialCommandGroup that runs TurretRunToPosition (no timeout or a safe, validated timeout) followed by ClimberRunToExtension), or alternatively, if you intend to keep parallelism, validate and document that the 1-second timeout on TurretRunToPosition is provably sufficient under all operating conditions.src/main/java/frc/alotobots/rebuilt/subsystems/kicker/io/KickerIOTalonFX.java (1)
118-138: Duplicate PIDSlot assignment in updateInputs.The
inputs.kickerMotorPIDSlotis assigned twice with identical logic (lines 118-124 and 132-138). The second assignment is redundant and should be removed.♻️ Remove duplicate assignment
inputs.kickerMotorConnected = kickerConnectedDebounce.calculate(kickerSignals.isOK()); inputs.kickerMotorVelocity = kickerVelocity.getValue(); inputs.kickerMotorAcceleration = kickerAcceleration.getValue(); inputs.kickerMotorVolts = kickerAppliedVoltage.getValue(); inputs.kickerMotorCurrent = kickerAppliedCurrent.getValue(); - - inputs.kickerMotorPIDSlot = - switch (currentPidSlot.getValue()) { - case 0 -> PIDSlots.DEFAULT_VELOCITY; - default -> throw new IllegalStateException( - "Bad things happened in Shooter and You check if you set the PID SLOTS RIGHT" - + currentPidSlot.getValue()); - }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/frc/alotobots/rebuilt/subsystems/kicker/io/KickerIOTalonFX.java` around lines 118 - 138, In updateInputs (KickerIOTalonFX) there's a duplicated assignment to inputs.kickerMotorPIDSlot: the same switch block appears twice; remove the redundant second assignment (the one after setting kickerMotorCurrent/kickerAppliedCurrent) so inputs.kickerMotorPIDSlot is only set once, leaving the first switch block intact and keeping the other input assignments (kickerMotorConnected, kickerMotorVelocity, kickerMotorAcceleration, kickerMotorVolts, kickerMotorCurrent) unchanged.src/main/java/frc/alotobots/rebuilt/subsystems/launcher/LaunchCalculatorConstants.java (1)
51-65: Freeze the target option lists.These are public constants, but
LinkedListleaves them mutable at runtime. An accidental add/remove elsewhere will change nearest-target selection globally.♻️ Proposed refactor
-import java.util.LinkedList; import java.util.List; @@ public static final List<Translation2d> PASSING_TARGET_OPTIONS_BLUE = - new LinkedList<>() { - { - add(PASSING_TARGET_DEPOT_BLUE.toTranslation2d()); - add(PASSING_TARGET_OUTPOST_BLUE.toTranslation2d()); - } - }; + List.of( + PASSING_TARGET_DEPOT_BLUE.toTranslation2d(), + PASSING_TARGET_OUTPOST_BLUE.toTranslation2d()); public static final List<Translation2d> PASSING_TARGET_OPTIONS_RED = - new LinkedList<>() { - { - add(PASSING_TARGET_DEPOT_RED.toTranslation2d()); - add(PASSING_TARGET_OUTPOST_RED.toTranslation2d()); - } - }; + List.of( + PASSING_TARGET_DEPOT_RED.toTranslation2d(), + PASSING_TARGET_OUTPOST_RED.toTranslation2d());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/frc/alotobots/rebuilt/subsystems/launcher/LaunchCalculatorConstants.java` around lines 51 - 65, PASSING_TARGET_OPTIONS_BLUE and PASSING_TARGET_OPTIONS_RED are currently mutable LinkedList instances; make them immutable so callers cannot add/remove entries at runtime by constructing them as immutable lists (e.g., via List.of(...) or wrapping with Collections.unmodifiableList) and assign the result to the same public static final List<Translation2d> constants; update the initializer for each constant to produce an unmodifiable list containing PASSING_TARGET_DEPOT_...toTranslation2d() and PASSING_TARGET_OUTPOST_...toTranslation2d() so the nearest-target selection cannot be changed accidentally.src/main/deploy/pathplanner/paths/New New New Path.path (1)
36-36: LGTM!Velocity constraint updated consistently.
Consider renaming this file to something more descriptive (e.g.,
Outpost_to_Depot_Shoot.path) to improve maintainability and clarity for future reference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/deploy/pathplanner/paths/New` New New Path.path at line 36, Rename the path file currently named "New New New Path.path" to a more descriptive name such as "Outpost_to_Depot_Shoot.path" to improve maintainability; update any references to this filename in the repository (imports, deployment manifests, or path lists) to point to the new name and verify the path loader still reads the file and the "maxVelocity": 4.5 setting remains unchanged.src/main/deploy/pathplanner/paths/New New New New New New Path.path (1)
1-53: Rename or remove this placeholder path asset before merge.A deploy-time path named
New New New New New New Pathlooks like a scratch artifact. Leaving it undersrc/main/deploy/pathplanner/pathsmakes it easy to select accidentally when building or editing autos. If this route is real, give it a stable descriptive name; otherwise, remove it from deploy assets.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/deploy/pathplanner/paths/New` New New New New New Path.path around lines 1 - 53, This path asset file currently named "New New New New New New Path.path" is a placeholder and should be removed or renamed to a stable, descriptive name; either delete this file from src/main/deploy/pathplanner/paths if it's not used, or rename the file to a meaningful route name (and update any references) and adjust metadata if needed (e.g., keep "folder": "Positions" or change it to the proper folder); check for usages of linkedName values ("Neutral Zone Outpost 1", "Neutral Zone Outpost 2") in any path-selection code and update those references after renaming or removing the file.src/main/deploy/pathplanner/autos/DepotStartNeutralClimbRaceToMiddle.auto (1)
57-185: Complex race block with deeply nested wiggle sequence.The race block combines a 10-second timeout, the launcher command, and an extensive wiggle sequence (8 alternating Wiggle 1/2 paths with a mid-sequence intake deployment). While functional, the deep nesting and repetition increase maintenance complexity.
Consider extracting the wiggle sequence into a separate named command or auto segment for better reusability if this pattern is used elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/deploy/pathplanner/autos/DepotStartNeutralClimbRaceToMiddle.auto` around lines 57 - 185, The race block contains a repeated deep "Wiggle 1"/"Wiggle 2" sequence interleaved with "DeployIntakeAndIntake" and should be extracted into a reusable named command to reduce duplication; create a new named command (e.g., "WiggleSequenceLong") that encapsulates the eight alternating "path" entries plus the internal race that runs "DeployIntakeAndIntake" with its 0.75s wait, then replace the long sequential block inside the race (the sequence of "path" entries and the nested race) with a single {"type":"named","data":{"name":"WiggleSequenceLong"}} entry so the race keeps "LauncherTargetHubDynamicAndShoot" and the 10s wait but references the new named segment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@BatteryAkitTemplate.json`:
- Around line 53-54: The JSON keys under LaunchCalculator have a typo and
incorrect hierarchy: replace occurrences of
"/RealOutputs/LaunchCalculator/Perameters" and its child
"/RealOutputs/LaunchCalculator/Perameters/turretAngle" with the correct paths
used by the Java logs, e.g. "/RealOutputs/LaunchCalculator/Hub/Parameters" or
"/RealOutputs/LaunchCalculator/Passing/Parameters" and
"/RealOutputs/LaunchCalculator/Hub/Parameters/turretAngle" (or the Passing
equivalent) so that the spelling "Parameters" is fixed and the Hub/Passing
segment is included to match the Java logging schema.
In `@src/main/deploy/pathplanner/autos/DepotStartNeutralClimbRaceToMiddle.auto`:
- Around line 114-131: The 0.75s wait after the "DeployIntakeAndIntake" named
command is marginal; update the wait command's data.waitTime from 0.75 to 1.0 to
give safe margin for motion-magic and debounce, i.e., locate the race block
containing the named command "DeployIntakeAndIntake" and its following wait
command and change waitTime to 1.0 (or alternatively run match-condition tests
to verify 0.75s is reliably sufficient before leaving as-is).
In
`@src/main/java/frc/alotobots/rebuilt/subsystems/launcher/LaunchCalculator.java`:
- Around line 322-328: The branch in LaunchCalculator that selects target from
PASSING_TARGET_OPTIONS_BLUE/RED silently defaults to Blue when
DriverStation.getAlliance() is empty; update the selection in the method
containing the forwardEstimatedPose logic to reuse the same warning/logging path
used by getAllianceCorrectedTarget(): detect when getAlliance() is empty, emit
the same diagnostic warning (using the same logger or warning helper) indicating
the fallback to Blue and then proceed to choose the nearest target from
PASSING_TARGET_OPTIONS_BLUE or PASSING_TARGET_OPTIONS_RED based on the actual
alliance value; reference the DriverStation.getAlliance(),
forwardEstimatedPose.getTranslation().nearest(...), and
getAllianceCorrectedTarget() behavior when implementing the warning.
In
`@src/main/java/frc/alotobots/rebuilt/subsystems/launcher/LaunchCalculatorConstants.java`:
- Around line 153-159: calculateLookahead() uses FUEL_HUB_TIME_OF_FLIGHT_MAP in
LaunchCalculatorConstants but the map only covers ~2.38–3.68 m while hub
validity is 1–6 m, causing out-of-range lookups; fix by either expanding
FUEL_HUB_TIME_OF_FLIGHT_MAP to include entries spanning the full 1–6 meter
window or add safe clamping/interpolation in calculateLookahead() to handle
distances outside the current map (e.g., clamp distance to min/max map keys or
implement extrapolation), update the logic in calculateLookahead() and/or the
static initializer for FUEL_HUB_TIME_OF_FLIGHT_MAP accordingly so every valid
hub distance has a defined TOF lookup path.
- Around line 35-36: The MAXIMUM_PASSING_SHOOTING_DISTANCE constant is set to 13
m but the calibrated passing table used by
LaunchCalculator.getPassingTargetParameters() only goes to 12.5 m, allowing
12.5–13 m shots to be considered valid using extrapolated values; either reduce
MAXIMUM_PASSING_SHOOTING_DISTANCE to match the table top (set
MAXIMUM_PASSING_SHOOTING_DISTANCE to 12.5 m) or extend the calibrated table
entries to cover up to 13 m so the table fully defines validity and returned
parameters. Update the constant or the calibrated table accordingly and ensure
any unit wrappers (e.g., Meters.of(...)) and references in LaunchCalculator
remain consistent.
In
`@src/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/constants/ShooterTalonFXConstants.java`:
- Around line 48-50: SHOOTER_VELOCITY_KA is declared in the outer PIDConstants
scope but should be grouped with other velocity PID params; move the static
final double SHOOTER_VELOCITY_KA = 0.0039303 declaration into the inner
VelocityPIDConstants class within ShooterTalonFXConstants so all
velocity-related constants are colocated, remove the duplicate/old declaration
from PIDConstants, and keep the constant name and value unchanged so any future
references to SHOOTER_VELOCITY_KA still resolve.
---
Outside diff comments:
In `@src/main/deploy/pathplanner/paths/Postions.path`:
- Around line 1-54: The file is misnamed "Postions.path" (typo) while its
internal "folder" is "Positions"; rename the file from "Postions.path" to
"Positions.path" and update any references/imports that point to "Postions.path"
so they use "Positions.path" instead, and verify the string "folder":
"Positions" remains unchanged in the file.
In `@src/main/java/frc/alotobots/Constants.java`:
- Around line 32-38: The constant simMode is set to Mode.REPLAY which makes
Mode.SIM code paths unreachable; change simMode to Mode.SIM (or alternatively
gate Mode.REPLAY behind a launch flag) so that currentMode = RobotBase.isReal()
? Mode.REAL : simMode correctly yields SIM for desktop/simulation runs; update
references that depend on Mode.SIM (Robot.java SIM initialization/periodic
branches, RobotContainer SIM field reset/display, SwerveDriveSubsystem gyro
disconnect suppression) and ensure tuner selection logic uses TunerConstants2025
for SIM and TunerConstants2026 only when REPLAY is explicitly requested.
---
Nitpick comments:
In `@src/main/deploy/pathplanner/autos/DepotStartNeutralClimbRaceToMiddle.auto`:
- Around line 57-185: The race block contains a repeated deep "Wiggle 1"/"Wiggle
2" sequence interleaved with "DeployIntakeAndIntake" and should be extracted
into a reusable named command to reduce duplication; create a new named command
(e.g., "WiggleSequenceLong") that encapsulates the eight alternating "path"
entries plus the internal race that runs "DeployIntakeAndIntake" with its 0.75s
wait, then replace the long sequential block inside the race (the sequence of
"path" entries and the nested race) with a single
{"type":"named","data":{"name":"WiggleSequenceLong"}} entry so the race keeps
"LauncherTargetHubDynamicAndShoot" and the 10s wait but references the new named
segment.
In `@src/main/deploy/pathplanner/paths/New` New New New New New Path.path:
- Around line 1-53: This path asset file currently named "New New New New New
New Path.path" is a placeholder and should be removed or renamed to a stable,
descriptive name; either delete this file from src/main/deploy/pathplanner/paths
if it's not used, or rename the file to a meaningful route name (and update any
references) and adjust metadata if needed (e.g., keep "folder": "Positions" or
change it to the proper folder); check for usages of linkedName values ("Neutral
Zone Outpost 1", "Neutral Zone Outpost 2") in any path-selection code and update
those references after renaming or removing the file.
In `@src/main/deploy/pathplanner/paths/New` New New New New Path.path:
- Line 1: Rename the placeholder file "New New New New New Path.path" to a
descriptive name reflecting its purpose and linked waypoints (for example
"NeutralZoneDepot2_to_NeutralZoneDepot1.path" or similar), update any references
to this filename in the repo if present, and ensure the new name follows project
naming conventions; target the file currently containing "{" (the placeholder
path file) and use the waypoint names "Neutral Zone Depot 2" and "Neutral Zone
Depot 1" to construct the new filename.
In `@src/main/deploy/pathplanner/paths/New` New New Path.path:
- Line 36: Rename the path file currently named "New New New Path.path" to a
more descriptive name such as "Outpost_to_Depot_Shoot.path" to improve
maintainability; update any references to this filename in the repository
(imports, deployment manifests, or path lists) to point to the new name and
verify the path loader still reads the file and the "maxVelocity": 4.5 setting
remains unchanged.
In `@src/main/deploy/pathplanner/paths/Positions.path`:
- Around line 35-36: The change to "maxVelocity" inside the "globalConstraints"
block is ineffective while useDefaultConstraints remains true; either disable
the defaults or remove the redundant override: if you want this path to use a
path-specific cap, set useDefaultConstraints to false and keep
globalConstraints.maxVelocity = 4.5 for this path (reference symbols:
useDefaultConstraints, globalConstraints, maxVelocity), otherwise revert the
maxVelocity edit and maintain the project-wide value in Settings.json as the
single source of truth.
In
`@src/main/java/frc/alotobots/rebuilt/commands/groups/ClimberUpWithTurretSafety.java`:
- Around line 28-33: The current ClimberUpWithTurretSafety group uses
addCommands() to run TurretRunToPosition(withTimeout(Seconds.of(1))) in parallel
with ClimberRunToExtension, which can let the climber continue if the turret
times out; change this to guarantee the turret reaches the safety angle before
the climber extends by making the turret command run first and only then
starting the climber (e.g., replace the parallel addCommands usage in
ClimberUpWithTurretSafety with a SequentialCommandGroup that runs
TurretRunToPosition (no timeout or a safe, validated timeout) followed by
ClimberRunToExtension), or alternatively, if you intend to keep parallelism,
validate and document that the 1-second timeout on TurretRunToPosition is
provably sufficient under all operating conditions.
In
`@src/main/java/frc/alotobots/rebuilt/subsystems/kicker/io/KickerIOTalonFX.java`:
- Around line 118-138: In updateInputs (KickerIOTalonFX) there's a duplicated
assignment to inputs.kickerMotorPIDSlot: the same switch block appears twice;
remove the redundant second assignment (the one after setting
kickerMotorCurrent/kickerAppliedCurrent) so inputs.kickerMotorPIDSlot is only
set once, leaving the first switch block intact and keeping the other input
assignments (kickerMotorConnected, kickerMotorVelocity, kickerMotorAcceleration,
kickerMotorVolts, kickerMotorCurrent) unchanged.
In
`@src/main/java/frc/alotobots/rebuilt/subsystems/launcher/LaunchCalculatorConstants.java`:
- Around line 51-65: PASSING_TARGET_OPTIONS_BLUE and PASSING_TARGET_OPTIONS_RED
are currently mutable LinkedList instances; make them immutable so callers
cannot add/remove entries at runtime by constructing them as immutable lists
(e.g., via List.of(...) or wrapping with Collections.unmodifiableList) and
assign the result to the same public static final List<Translation2d> constants;
update the initializer for each constant to produce an unmodifiable list
containing PASSING_TARGET_DEPOT_...toTranslation2d() and
PASSING_TARGET_OUTPOST_...toTranslation2d() so the nearest-target selection
cannot be changed accidentally.
In
`@src/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/io/ShooterIOTalonFX.java`:
- Around line 227-230: The stop() method currently only calls
motorLeft.stopMotor(); explicitly also call motorRight.stopMotor() to ensure the
follower is stopped; update the ShooterIOTalonFX.stop() method to invoke
motorLeft.stopMotor() and motorRight.stopMotor() (referencing the motorLeft and
motorRight fields) so the follower is explicitly stopped rather than relying on
implicit leader propagation.
In
`@src/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/ShooterSubsystem.java`:
- Around line 122-136: The two similar methods in ShooterSubsystem have
inconsistent naming: sysIdFwdQuasistatic vs sysIdRvsQuasiStatic; rename
sysIdRvsQuasiStatic to sysIdRvsQuasistatic to match the lowercase "s" style (or
vice‑versa if you prefer the Uppercase S, but keep both consistent), and update
all references/usages and any Javadoc/comments accordingly (methods:
sysIdFwdQuasistatic, sysIdRvsQuasistatic; class: ShooterSubsystem; calls to
sysIdRoutine.quasistatic(SysIdRoutine.Direction...)).
In
`@src/main/java/frc/alotobots/rebuilt/subsystems/launcher/turret/constants/TurretTalonFXSConstants.java`:
- Line 41: The constant TURRET_VELOCITY_KP in class TurretTalonFXSConstants is
written as .972 without a leading zero; update its literal to 0.972 to match the
file's numeric formatting style and maintain consistency with other constants
(e.g., 0.0, 0.36, 1.75).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e170f34f-e5df-43df-a7aa-c303c97492d2
📒 Files selected for processing (77)
BatteryAkitTemplate.jsonsrc/main/deploy/pathplanner/autos/DepotStartNeutralClimbEarlyShoot.autosrc/main/deploy/pathplanner/autos/DepotStartNeutralClimbExtensiveGVSUTesting.autosrc/main/deploy/pathplanner/autos/DepotStartNeutralClimbRaceToMiddle.autosrc/main/deploy/pathplanner/paths/Center Climb.pathsrc/main/deploy/pathplanner/paths/CenterStart_Shoot.pathsrc/main/deploy/pathplanner/paths/Depot Climb 1.pathsrc/main/deploy/pathplanner/paths/Depot Climb 2.pathsrc/main/deploy/pathplanner/paths/Depot Start,Preload,Neutral 1.pathsrc/main/deploy/pathplanner/paths/Depot Start,Preload,Neutral 2.pathsrc/main/deploy/pathplanner/paths/Depot Start,Preload,Neutral 3.pathsrc/main/deploy/pathplanner/paths/Depot Start,Preload,Neutral 4.pathsrc/main/deploy/pathplanner/paths/DepotStartNeutralClimb 2.pathsrc/main/deploy/pathplanner/paths/DepotStartNeutralClimb 3.pathsrc/main/deploy/pathplanner/paths/DepotStartNeutralClimb 3Part1ofearlyshoot.pathsrc/main/deploy/pathplanner/paths/DepotStartNeutralClimb 3Part2ofearlyshoot.pathsrc/main/deploy/pathplanner/paths/DepotStartNeutralClimb 4.pathsrc/main/deploy/pathplanner/paths/DepotStartNeutralClimb 5.pathsrc/main/deploy/pathplanner/paths/DepotStartNeutralClimb.pathsrc/main/deploy/pathplanner/paths/DepotStart_Shoot.pathsrc/main/deploy/pathplanner/paths/New New New New New New Path.pathsrc/main/deploy/pathplanner/paths/New New New New New Path.pathsrc/main/deploy/pathplanner/paths/New New New New Path.pathsrc/main/deploy/pathplanner/paths/New New New Path.pathsrc/main/deploy/pathplanner/paths/New New Path.pathsrc/main/deploy/pathplanner/paths/New Path.pathsrc/main/deploy/pathplanner/paths/Outpost Climb 2.pathsrc/main/deploy/pathplanner/paths/Outpost Start,Preload,Neutral 1.pathsrc/main/deploy/pathplanner/paths/Outpost Start,Preload,Neutral 2.pathsrc/main/deploy/pathplanner/paths/Outpost Start,Preload,Neutral 3.pathsrc/main/deploy/pathplanner/paths/Outpost Start,Preload,Neutral 4.pathsrc/main/deploy/pathplanner/paths/Outpost to Shoot.pathsrc/main/deploy/pathplanner/paths/OutpostStartNeutralClimb 2.pathsrc/main/deploy/pathplanner/paths/OutpostStartNeutralClimb 3.pathsrc/main/deploy/pathplanner/paths/OutpostStartNeutralClimb 4.pathsrc/main/deploy/pathplanner/paths/OutpostStartNeutralClimb 5.pathsrc/main/deploy/pathplanner/paths/OutpostStartNeutralClimb.pathsrc/main/deploy/pathplanner/paths/OutpostStart_Shoot.pathsrc/main/deploy/pathplanner/paths/Positions.pathsrc/main/deploy/pathplanner/paths/Postions.pathsrc/main/deploy/pathplanner/paths/Shoot to Outpost.pathsrc/main/deploy/pathplanner/paths/Wiggle 1.pathsrc/main/deploy/pathplanner/paths/Wiggle 2.pathsrc/main/deploy/pathplanner/settings.jsonsrc/main/java/frc/alotobots/Constants.javasrc/main/java/frc/alotobots/OI.javasrc/main/java/frc/alotobots/RobotContainer.javasrc/main/java/frc/alotobots/library/subsystems/bling/constants/BlingConstants.javasrc/main/java/frc/alotobots/library/subsystems/swervedrive/constants/mk5i2026/TunerConstants2026.javasrc/main/java/frc/alotobots/rebuilt/commands/groups/ClimberUpWithTurretSafety.javasrc/main/java/frc/alotobots/rebuilt/commands/groups/LauncherTargetPassingDynamicAndShoot.javasrc/main/java/frc/alotobots/rebuilt/subsystems/belt/constants/BeltConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/belt/constants/BeltTalonFXConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/belt/io/BeltIOTalonFX.javasrc/main/java/frc/alotobots/rebuilt/subsystems/climber/constants/ClimberTalonFXConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/climber/io/ClimberIOTalonFX.javasrc/main/java/frc/alotobots/rebuilt/subsystems/intake/extendo/IntakeExtendoSubsystem.javasrc/main/java/frc/alotobots/rebuilt/subsystems/intake/extendo/constants/IntakeExtendoConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/intake/extendo/constants/IntakeExtendoTalonFXConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/intake/extendo/io/IntakeExtendoIOTalonFX.javasrc/main/java/frc/alotobots/rebuilt/subsystems/intake/roller/constants/IntakeRollerTalonFXConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/intake/roller/io/IntakeRollerIOTalonFX.javasrc/main/java/frc/alotobots/rebuilt/subsystems/kicker/constants/KickerConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/kicker/constants/KickerTalonFXConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/kicker/io/KickerIOTalonFX.javasrc/main/java/frc/alotobots/rebuilt/subsystems/launcher/LaunchCalculator.javasrc/main/java/frc/alotobots/rebuilt/subsystems/launcher/LaunchCalculatorConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/launcher/deflector/constants/DeflectorConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/launcher/deflector/constants/DeflectorVortexConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/ShooterSubsystem.javasrc/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/constants/ShooterConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/constants/ShooterTalonFXConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/io/ShooterIO.javasrc/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/io/ShooterIOTalonFX.javasrc/main/java/frc/alotobots/rebuilt/subsystems/launcher/turret/constants/TurretConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/launcher/turret/constants/TurretTalonFXSConstants.javasrc/main/java/frc/alotobots/rebuilt/subsystems/launcher/turret/io/TurretIOTalonFXS.java
💤 Files with no reviewable changes (1)
- src/main/deploy/pathplanner/autos/DepotStartNeutralClimbExtensiveGVSUTesting.auto
Comment on lines
+48
to
50
|
|
||
| public static final double SHOOTER_VELOCITY_KA = 0.0039303; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if SHOOTER_VELOCITY_KA is used anywhere in the codebase
rg -n "SHOOTER_VELOCITY_KA" --type=javaRepository: 5152Alotobots/5152_Rebuilt
Length of output: 239
🏁 Script executed:
#!/bin/bash
# Check the structure of ShooterTalonFXConstants.java around the constant
cat -n src/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/constants/ShooterTalonFXConstants.java | head -70Repository: 5152Alotobots/5152_Rebuilt
Length of output: 3239
🏁 Script executed:
#!/bin/bash
# Check ShooterIOTalonFX.java for any usage or kA configuration
rg -n "kA|SHOOTER_VELOCITY_KA|feedforward" src/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/ --type=java -iRepository: 5152Alotobots/5152_Rebuilt
Length of output: 2266
Move SHOOTER_VELOCITY_KA inside VelocityPIDConstants for organizational consistency.
This constant is placed inside PIDConstants but outside VelocityPIDConstants, breaking the pattern where all velocity-related PID constants are grouped together. Additionally, the constant is currently unused in the codebase.
♻️ Move constant inside VelocityPIDConstants
public static final class VelocityPIDConstants {
/** Position control proportional gain */
public static final double SHOOTER_VELOCITY_KP = 2.5;
/** Position control integral gain */
public static final double SHOOTER_VELOCITY_KI = 0.0;
/** Position control derivative gain */
public static final double SHOOTER_VELOCITY_KD = 0.0;
/** Gravity compensation gain */
public static final double SHOOTER_VELOCITY_KG = 0.0;
/** Static friction compensation */
public static final double SHOOTER_VELOCITY_KS = 0.0090433;
/** Velocity feedforward gain */
public static final double SHOOTER_VELOCITY_KV = 0.125;
+
+ /** Acceleration feedforward gain */
+ public static final double SHOOTER_VELOCITY_KA = 0.0039303;
}
-
- public static final double SHOOTER_VELOCITY_KA = 0.0039303;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public static final double SHOOTER_VELOCITY_KA = 0.0039303; | |
| } | |
| public static final class VelocityPIDConstants { | |
| /** Position control proportional gain */ | |
| public static final double SHOOTER_VELOCITY_KP = 2.5; | |
| /** Position control integral gain */ | |
| public static final double SHOOTER_VELOCITY_KI = 0.0; | |
| /** Position control derivative gain */ | |
| public static final double SHOOTER_VELOCITY_KD = 0.0; | |
| /** Gravity compensation gain */ | |
| public static final double SHOOTER_VELOCITY_KG = 0.0; | |
| /** Static friction compensation */ | |
| public static final double SHOOTER_VELOCITY_KS = 0.0090433; | |
| /** Velocity feedforward gain */ | |
| public static final double SHOOTER_VELOCITY_KV = 0.125; | |
| /** Acceleration feedforward gain */ | |
| public static final double SHOOTER_VELOCITY_KA = 0.0039303; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/frc/alotobots/rebuilt/subsystems/launcher/shooter/constants/ShooterTalonFXConstants.java`
around lines 48 - 50, SHOOTER_VELOCITY_KA is declared in the outer PIDConstants
scope but should be grouped with other velocity PID params; move the static
final double SHOOTER_VELOCITY_KA = 0.0039303 declaration into the inner
VelocityPIDConstants class within ShooterTalonFXConstants so all
velocity-related constants are colocated, remove the duplicate/old declaration
from PIDConstants, and keep the constant name and value unchanged so any future
references to SHOOTER_VELOCITY_KA still resolve.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Improvements