Feature/lemon hunter

[bunny2027]

No description provided.

[github-actions]

AI Code Review

Hello team!

This is a fantastic start to the lemon hunter feature! It's great to see a dedicated subsystem for this, leveraging PhotonVision and incorporating some advanced concepts like 3D pose estimation and clustering. The structure with separate constants and preferences files is also a good practice.

Positive Highlights

• Excellent use of WPILib Units: I love seeing Measure<Distance> and Measure<Angle> in LemonHunterConstants.java and their proper conversion with .in(Meters) and .in(Radians). This is a huge step towards preventing unit conversion errors!
• Clear Subsystem Structure: The LemonHunterSubsystem correctly extends SubsystemBase and has a well-defined periodic() method, which is exactly what we want for FRC code.
• Effective Logging with Epilogue: The use of @Logged annotations with Importance.CRITICAL is excellent for getting valuable telemetry from the robot and will be super helpful for debugging and analysis with AdvantageScope.
• Robust Lemon Detection Logic: The inclusion of estimateLemon3dPose, findLargestCluster, detectOverlaps, and utility methods like getNearestLemonInCluster shows a thoughtful approach to processing vision data and identifying targets.

Suggestions

Here are a few suggestions to make this feature even more robust and aligned with our team's best practices:

Code Quality & Java Standards

1. PhotonCamera Instantiation (File: src/main/java/frc/robot/subsystems/lemon_hunter/LemonHunterConstants.java)

   • Suggestion: Instead of instantiating PhotonCamera directly in LemonHunterConstants, it's generally better to instantiate it within the LemonHunterSubsystem's constructor. This ensures the camera object is managed by the subsystem and follows the singleton subsystem pattern more cleanly. Constants files should ideally only contain primitive values or Measure objects, not active hardware objects.
   • Why it matters: It helps maintain clear ownership and lifecycle management of hardware resources.

2. Encapsulate Lists (File: src/main/java/frc/robot/subsystems/lemon_hunter/LemonHunterSubsystem.java, lines 31-37)

   • Suggestion: Consider making the lists (lemonList, bestCluster, overlappingPairs, latestTargets) private and providing public getter methods if other parts of the robot need to access them.
   • Why it matters: This follows good object-oriented principles by encapsulating the internal state of the subsystem, making it easier to manage and prevent unintended modifications from outside classes.

3. Javadoc for Public Methods (File: src/main/java/frc/robot/subsystems/lemon_hunter/LemonHunterSubsystem.java)

   • Suggestion: It might be helpful to add Javadoc comments to public methods like estimateLemon3dPose, findLargestCluster, detectOverlaps, getNearestLemonInCluster, and getClusterCentroid.
   • Why it matters: This makes your code easier for others (and future you!) to understand and use, especially for complex algorithms.

4. No Newline at End of File (Files: LemonHunterPreferences.java, LemonHunterSubsystem.java)

   • Suggestion: This is a minor style point, but it's good practice to ensure all .java files end with a newline character. Spotless usually handles this, but it's worth a quick check.
   • Why it matters: Some build systems or text editors can have issues with files that don't end with a newline.

FRC/WPILib Best Practices

1. Purpose of HunterState (File: src/main/java/frc/robot/statemachines/HunterState.java)

   • Suggestion: Currently, HunterState is an empty singleton class. If its purpose is to manage states for a "Hunter" mechanism, it should contain an enum for the states and possibly methods to transition between them, or hold shared state variables for the hunter. If it's not going to hold any state, it might not be necessary to be a singleton or even exist as a class.
   • Why it matters: Clear purpose for classes helps with maintainability and understanding the robot's architecture.

2. Camera Transform3d for Pose Estimation (File: src/main/java/frc/robot/subsystems/lemon_hunter/LemonHunterSubsystem.java, estimateLemon3dPose method)

   • Suggestion: Instead of manually calculating dx_cam, dy_cam, dz_cam and applying HUNTER_OFFSET, consider defining a Transform3d for the camera's pose relative to the robot's center. WPILib and PhotonVision provide utilities to apply these transformations more robustly. For example, PhotonUtils.estimateFieldToTarget or PhotonUtils.estimateCameraToTarget can simplify the math and make it less error-prone.
   • Why it matters: Using Transform3d correctly makes the pose estimation more accurate and robust to camera mounting changes. It also directly handles the camera's position and orientation (like HUNTER_PITCH). The Rotation3d in your return statement (new Rotation3d(0, -targetPitch, targetYaw)) might need careful verification against how WPILib defines roll/pitch/yaw for camera poses.

3. LemonHunterPreferences (File: src/main/java/frc/robot/subsystems/lemon_hunter/LemonHunterPreferences.java)

   • Suggestion: Similar to HunterState, LemonHunterPreferences is currently an empty class. If it's intended to hold robot preferences (e.g., values that can be tuned via Shuffleboard or saved to Preferences), it should contain public static final fields that are initialized from Preferences or Shuffleboard entries.
   • Why it matters: This provides a centralized place for tuning parameters without recompiling code.

Questions

• What is the intended purpose of the HunterState class? Is it meant to hold the current state of a larger "hunting" operation?
• Is DriveState a custom class that provides the robot's current pose, or is it a wrapper around a standard drive subsystem?

Great work on this new feature! The groundwork for robust vision processing is clearly laid out, and these suggestions are just to refine it further. 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!