[cwkpete7383]

Flywheels subsystem. Need to tune before merging.

[qodo-code-review]

PR Summary by Qodo

Add dual-motor flywheel subsystem and driver controls

✨ Enhancement 🕐 20-40 Minutes

AI Description

• Adds a dual-TalonFX flywheel subsystem with leader-follower velocity control.
• Registers configurable flywheel hardware behind a subsystem enable flag.
• Binds the driver's right trigger to the flywheel run command.

Diagram

graph TD
  A["Driver Trigger"] -->|"while held"| B["Controls"] -->|"runs command"| C["Flywheels"]
  D["Subsystem Registry"] -->|"creates"| C
  E["Hardware IDs"] -->|"select CAN IDs"| C
  C -->|"velocity request"| F["Leader TalonFX"] -->|"opposed follow"| G["Follower TalonFX"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use imperative motor helpers inside runEnd

• ➕ Ensures each runEnd callback directly changes motor state
• ➕ Keeps start and stop behavior reusable without nested command factories
• ➕ Makes trigger release reliably stop both motors
• ➖ Requires separating actuator operations from public command construction
• ➖ Slightly changes the current command API

2. Compose start, hold, and stop commands

• ➕ Preserves command-only subsystem APIs
• ➕ Makes lifecycle sequencing explicit through WPILib composition
• ➖ Adds unnecessary command composition for a simple hold-to-run interaction
• ➖ Can make interruption behavior harder to inspect

Recommendation: Use private imperative methods to apply velocity and stop both motors, then invoke those methods directly from runEnd and wrap them in runOnce where standalone commands are needed. The current runEnd callbacks only create start/stop commands without scheduling them, so the trigger-bound command does not actually issue those motor actions.

Files changed (4) +118 / -0

Enhancement (3) +115 / -0

Controls.javaBind the right trigger to flywheel operation +5/-0

Bind the right trigger to flywheel operation

• Adds flywheel binding setup during controls initialization. Holding the driver's right trigger schedules the flywheel run command.

src/main/java/frc/robot/Controls.java

Flywheels.javaImplement the dual-TalonFX flywheel subsystem +102/-0

Implement the dual-TalonFX flywheel subsystem

• Introduces leader-follower TalonFX control, current limits, neutral and inversion settings, placeholder PID/feedforward gains, and Motion Magic velocity acceleration. Provides run, one-shot start, and one-shot stop command factories; configuration values still require tuning.

src/main/java/frc/robot/Flywheels.java

Subsystems.javaRegister and gate the flywheel subsystem +8/-0

Register and gate the flywheel subsystem

• Adds a flywheel enable flag and conditionally constructs the subsystem alongside the drivetrain.

src/main/java/frc/robot/Subsystems.java

Other (1) +3 / -0

Hardware.javaDefine flywheel CAN device identifiers +3/-0

Define flywheel CAN device identifiers

• Adds placeholder CAN IDs for the two flywheel TalonFX controllers, pending validation against the physical robot.

src/main/java/frc/robot/Hardware.java

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Flywheels receive no motor commands ✓ Resolved 🐞 Bug ≡ Correctness

Description

runFlywheels calls the startFlywheels and stopFlywheels command factories inside callbacks but
discards the returned commands without scheduling them. Holding or releasing the right trigger
therefore never reaches setControl or stopMotor, so neither flywheel responds.

Code

src/main/java/frc/robot/Flywheels.java[R76-79]

+              startFlywheels(speed);
+            },
+            () -> {
+              stopFlywheels();

Evidence

The bound outer command only invokes methods returning new commands, while the actual hardware calls
are inside those returned commands and are never scheduled.

src/main/java/frc/robot/Controls.java[84-86]
src/main/java/frc/robot/Flywheels.java[73-100]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`runFlywheels` creates nested commands inside `runEnd` callbacks and discards them, so the motor actions never execute.

## Fix Focus Areas
- src/main/java/frc/robot/Flywheels.java[73-100]
- src/main/java/frc/robot/Controls.java[84-86]

## Recommended Fix
Put the `setControl` calls directly in the `runEnd` start callback and the `stopMotor` calls directly in its end callback. Alternatively, compose the existing commands using WPILib command-composition APIs rather than invoking command factories as ordinary methods.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Second motor misses safety limits ✗ Dismissed 🐞 Bug ☼ Reliability

Description

configureMotors obtains and applies a configurator only for FlywheelOne, leaving FlywheelTwo
without the declared supply and stator current-limit configuration. When both motors run, the
follower retains its device configuration instead of sharing the intended electrical protections.

Code

src/main/java/frc/robot/Flywheels.java[49]

+    TalonFXConfigurator flConfigurator = FlywheelOne.getConfigurator();

Evidence

Both motors are constructed, but only the first motor's configurator is obtained and receives the
configuration containing the enabled current limits.

src/main/java/frc/robot/Flywheels.java[40-49]
src/main/java/frc/robot/Flywheels.java[51-70]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The current-limit configuration is applied only to the leader, leaving the follower without the subsystem's intended electrical protections.

## Fix Focus Areas
- src/main/java/frc/robot/Flywheels.java[47-71]

## Recommended Fix
Obtain the configurator for each TalonFX and apply an appropriate configuration to both devices. Preserve any motor-specific inversion or follower settings while ensuring both motors receive the intended current limits and neutral mode.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Flywheels cannot reach commanded speed ✗ Dismissed 🐞 Bug ≡ Correctness

Description

configureMotors applies zero values for every feedback and feedforward gain while startFlywheels
exclusively uses closed-loop Motion Magic velocity control. Once command execution is corrected, the
controller has no configured output term to drive the motors toward the requested speed.

Code

src/main/java/frc/robot/Flywheels.java[R21-24]

+  double kP = 0.0;
+  double kI = 0.0;
+  double kD = 0.0;
+  double kA = 0.0;

Evidence

Every configured control gain is initialized to zero and applied to slot zero, which is the slot
implicitly used by the new velocity request.

src/main/java/frc/robot/Flywheels.java[18-35]
src/main/java/frc/robot/Flywheels.java[60-68]
src/main/java/frc/robot/Flywheels.java[84-90]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
All flywheel feedback and feedforward gains are zero, so the Motion Magic velocity request cannot produce useful closed-loop output.

## Fix Focus Areas
- src/main/java/frc/robot/Flywheels.java[18-35]
- src/main/java/frc/robot/Flywheels.java[60-68]
- src/main/java/frc/robot/Flywheels.java[84-90]

## Recommended Fix
Characterize the flywheel mechanism and configure appropriate nonzero feedforward and feedback gains before enabling the binding. Store the tuned target speed and gains as clearly named constants, then validate that the requested rotations per second are reached safely.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Disabling flywheels crashes startup ✓ Resolved 🐞 Bug ☼ Reliability

Description

configureFlywheelBindings unconditionally dereferences s.Flywheels, although Subsystems
assigns that field to null when FLYWHEELS_ENABLED is false. Turning off the newly added feature
flag therefore throws during Controls construction instead of starting the robot without flywheel
controls.

Code

src/main/java/frc/robot/Controls.java[R84-85]

+  public void configureFlywheelBindings() {
+    driverController.rightTrigger().whileTrue(s.Flywheels.runFlywheels());

Evidence

The controls constructor always installs the new binding, while the corresponding feature-flag
branch explicitly makes the referenced subsystem null.

src/main/java/frc/robot/Controls.java[42-45]
src/main/java/frc/robot/Controls.java[84-86]
src/main/java/frc/robot/Subsystems.java[39-43]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The flywheel binding dereferences a nullable subsystem, so using the subsystem's disable flag causes robot startup to fail.

## Fix Focus Areas
- src/main/java/frc/robot/Controls.java[42-45]
- src/main/java/frc/robot/Controls.java[84-86]
- src/main/java/frc/robot/Subsystems.java[39-43]

## Recommended Fix
Only configure the flywheel trigger when the subsystem is enabled and non-null. Prefer centralizing this check with subsystem construction so future bindings cannot accidentally dereference disabled subsystems.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

Review mode: ⚖️ Balanced: This adds a new motor-control subsystem with hardware configuration, command bindings, and runtime behavior, creating meaningful integration and safety risks that warrant a complete review.

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗