up to data version of Justin's code

[Orcasphynx]

No description provided.

[github-actions]

AI Code Review

Hello team!

This looks like a great pull request, bringing some useful updates and a new "lemon hunting" feature. It's always good to see the codebase evolving with new capabilities!

Positive Highlights

• IDE Performance Improvement: The increase in Xmx for the Java Language Server in .vscode/settings.json is a smart move. For larger FRC projects, this can significantly improve development experience and IDE responsiveness – great proactive tuning!
• Excellent Use of Type-Safe Units: The getDriveToLemonRequest() method in RobotContainer.java demonstrates fantastic use of WPILib's Units system, especially with RadiansPerSecond.of() and .in(RadiansPerSecond). This directly follows our team's guidelines and helps prevent those tricky unit conversion errors. Well done!
• Clear Architecture for New Feature: The introduction of LEMON_HUNTING_REQUEST and HUNT_SPEED in DriveConstants.java, along with the getDriveToLemonRequest() method, shows good architectural planning for a new robot behavior.

Suggestions

src/main/java/frc/robot/RobotContainer.java

1. Clarify getDriveAndLaunchRequest() Deadband:
   • Observation: In getDriveAndLaunchRequest(), the explicit .withDeadband(DriveConstants.MAX_DRIVE_SPEED * 0.1); was removed.
   • Context: DriveConstants.DEFAULT_DRIVE_REQUEST (which this method builds upon) already includes deadband configurations.
   • Suggestion: This is likely a good cleanup to avoid redundant deadband application. Just to be sure, could you confirm that the intent was to rely solely on the deadband defined in DEFAULT_DRIVE_REQUEST for this command, and that no specialized deadband behavior is needed here?
2. Review getDriveToLemonRequest() Rotational Control Logic:
   • Observation: The calculation for targetAngularVelocity and its subsequent use in rotationalRate for the getDriveToLemonRequest() method appears to have a slight conceptual mismatch for a typical PD controller.
   • Explanation:
     • targetAngle is the current bearing to the lemon cluster.
     • targetAngle.minus(driveState.getPreviousDriveStats().Pose.getRotation()).getRadians() calculates the angular error relative to the robot's previous rotation. While this is a delta angle, it's not a true angular velocity (rate of change of angle over time).
     • Using this "previous angular error" as a feedforward term (the first term in rotationalRate) and within the kD term might lead to less predictable or stable aiming behavior than intended.
     • A standard PD controller for aiming typically uses kP * currentAngularError + kD * (d(currentAngularError)/dt) or kP * currentAngularError - kD * currentRobotAngularVelocity for damping.
   • Suggestion: Consider re-evaluating the targetAngularVelocity calculation.
     • If targetAngularVelocity is intended as a feedforward (desired angular velocity), it should represent the rate at which the targetAngle itself is changing, or a desired constant angular velocity for aiming.
     • For the kD term, it's usually kD * (desired_angular_velocity - actual_angular_velocity) or kD * (current_error - previous_error) / dt.
     • One common approach for PD aiming is:

       double currentAngularError = targetAngle.minus(driveState.getCurrentDriveStats().Pose.getRotation()).getRadians();
       // Calculate a desired angular velocity based on error (P term)
       double desiredRotationalVelocity = currentAngularError * DrivePreferences.autoAim_kP.getValue();
       // Add a D term for damping based on the difference between desired and actual robot angular velocity
       double rotationalRate = desiredRotationalVelocity
           + DrivePreferences.autoAim_kD.getValue() * (desiredRotationalVelocity - driveState.getFieldVelocity().omegaRadiansPerSecond);

     • This ensures the kD term correctly dampens the robot's current angular velocity to match the desired angular velocity.

src/main/java/frc/robot/subsystems/drive/DriveConstants.java

1. Clarify Removal of AUTO_DRIVE_REQUEST:
   • Observation: The AUTO_DRIVE_REQUEST (which used closed-loop velocity control and ForwardPerspectiveValue.BlueAlliance) was removed.
   • Context: This request was previously used for autonomous driving.
   • Suggestion: If AutoBuilder is now fully managing the drive requests for autonomous paths (which often use velocity control), this removal might be perfectly fine. Could you confirm that the functionality previously provided by AUTO_DRIVE_REQUEST is either no longer needed or is now handled appropriately by AutoBuilder's defaults or other specialized auto commands?

Questions

• Regarding the getDriveAndLaunchRequest() deadband removal, was it specifically intended to use the DEFAULT_DRIVE_REQUEST's deadband, or was there another reason?
• Could you elaborate on the mathematical intent behind the targetAngularVelocity calculation in getDriveToLemonRequest()? Understanding the goal will help ensure the PD controller behaves as expected for aiming.
• Is the removal of AUTO_DRIVE_REQUEST in DriveConstants.java intentional, relying on AutoBuilder or other autonomous commands to handle specific driving requests?

---

Great work on these updates! The continuous refinement of our codebase, especially with new features and attention to detail like type-safe units, is what makes our robot perform so well. Keep up the excellent work!

---

This review was automatically generated by AI. Please use your judgment and feel free to discuss any suggestions!

[github-actions]

✓ Build successful and code formatting check passed!

[github-actions]

✓ Build successful and code formatting check passed!