No more vision warnings

[Orcasphynx]

No description provided.

[github-actions]

AI Code Review

Hey team!

This is a really great pull request that brings some significant improvements to our vision system. The refactoring and enhanced logging are going to make a big difference in understanding and debugging our robot's pose estimation!

Positive Highlights

• Excellent Vision Refactoring: Moving the detailed pose estimation logic from DrivetrainSubsystem into VisionSubsystem is a fantastic architectural improvement. It makes VisionSubsystem a dedicated and more maintainable unit for all vision processing.
• Comprehensive Vision Logging: The new @Logged fields for accepted and rejected vision results, categorized by camera and reason (ambiguity, jumping), are incredibly valuable. This aligns perfectly with our AdvantageKit best practices and will be instrumental for debugging and performance analysis.
• Dynamic Trust Values: The implementation of dynamically calculating standard deviations (xyStdDev, thetaStdDev) based on the distance to tags and the number of tags used is a significant step forward for robust and intelligent pose estimation. This is a very smart way to improve our robot's confidence in vision measurements!

Suggestions

• CameraConstants.java - Naming Conventions (Constants)

  • Observation: Constants like photonCameraName_Front, photonCamera_Front, and FIELD_LAYOUT are not currently in UPPER_SNAKE_CASE.
  • Recommendation: "Consider updating the naming convention for public static final constants in CameraConstants.java to UPPER_SNAKE_CASE (e.g., PHOTON_CAMERA_NAME_FRONT, PHOTON_CAMERA_FRONT, FIELD_LAYOUT). This helps maintain consistency with Java and FRC team standards for constants, making the code easier to read and understand at a glance."

• VisionSubsystem.java - Javadoc for VisionMeasurement

  • Observation: The VisionMeasurement inner class and its constructor/getter methods could benefit from Javadoc comments.
  • Recommendation: "It might be helpful to add Javadoc comments to the VisionMeasurement inner class, its constructor, and getter methods. Explaining what each field represents (e.g., estimatedPose is the robot's pose, timestamp is when the measurement was taken, trustValues are the standard deviations) would greatly improve code clarity for anyone using this class."

• VisionSubsystem.java - Code Duplication in evaluateEstimation

  • Observation: The "jumping" check (if (!RobotModeTriggers.disabled().getAsBoolean() && estimateDistance > TunerConstants.kSpeedAt12Volts.in(MetersPerSecond) * 0.02)) is duplicated in both evaluateEstimation overloads.
  • Recommendation: "One approach to reduce this code duplication could be to extract the 'jumping' check logic into a private helper method. Alternatively, you might be able to combine the two evaluateEstimation methods if feasible by passing a List<Short> that might contain one element for single-tag cases. This would make the code more DRY (Don't Repeat Yourself) and easier to maintain if the jumping threshold ever needs adjustment."

Questions

• VisionSubsystem.java - setPipeline method: The setPipeline method was removed from VisionSubsystem. Is there another way to control the PhotonVision pipeline index (e.g., for different game pieces or lighting conditions), or is it intended for the pipeline to be fixed now?

Overall, this is a fantastic set of changes that significantly improves our vision system's robustness and debuggability. Keep up the great 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!

[github-actions]

✓ Build successful and code formatting check passed!