Skip to content

From DE#50

Merged
y0shi merged 1 commit intodevelopmentfrom
testing/march-8
Mar 9, 2026
Merged

From DE#50
y0shi merged 1 commit intodevelopmentfrom
testing/march-8

Conversation

@y0shi
Copy link
Member

@y0shi y0shi commented Mar 9, 2026

No description provided.

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

AI Code Review

Hello there! This looks like a fantastic pull request with a lot of great updates across our autonomous routines, path tuning, and vision system. I'm really impressed with the attention to detail here!

Summary

This pull request significantly enhances our robot's autonomous capabilities by introducing several new "just drive" autonomous paths, refining existing paths with updated waypoints and constraint zones, and updating crucial drivetrain parameters in our PathPlanner settings. Additionally, the vision subsystem receives valuable improvements for measurement evaluation and debugging, which will help us get more accurate pose estimates.

Positive Highlights

  • Robust Autonomous Command Composition: I really like the use of the race command with AutonShoot and wait in the Basic Comp Auto and Depot Shoot sequences. This is a smart way to ensure our shooting command runs for a sufficient duration while also providing a graceful exit with StopShot if it doesn't self-terminate, demonstrating excellent command composition!
  • Path Tuning and Constraint Zones: The adjustments to existing paths, including waypoint modifications and the addition of constraintZones in the Collect paths, show a keen eye for optimizing robot behavior during specific path segments. Using constraint zones for slower collection speeds is a great practice for resource management and reliability.
  • Accurate Robot Parameters: It's excellent to see the pathplanner/settings.json file updated with what appear to be more accurate drivetrain parameters (wheel radius, gearing, max speed, module offsets). This is a critical step for generating precise and effective trajectories, aligning perfectly with WPILib best practices for robust robot control.
  • Type-Safe Units: The VisionSubsystem correctly leverages Units.MetersPerSecond when evaluating TunerConstants.kSpeedAt12Volts for the pose distance threshold. This is a fantastic example of adhering to our team's guideline on using type-safe units, which helps prevent subtle conversion errors and makes the code safer!

Suggestions

1. FRC/WPILib Best Practices - Vision Subsystem Logging with AdvantageKit

  • File: src/main/java/frc/robot/subsystems/vision/VisionSubsystem.java
  • Lines: 20-30, 140-149, 161-167
  • Details: I noticed that the @Logged annotations for highestAmbiguityFront/Left/Right were commented out and replaced with SmartDashboard.putNumber calls. While SmartDashboard is super handy for immediate, live debugging, for critical telemetry that we might want to analyze post-match or use for AdvantageKit's deterministic replay capabilities, @Logged fields are generally preferred.
  • Recommendation: Consider re-enabling the @Logged annotations for highestAmbiguityFront, highestAmbiguityLeft, highestAmbiguityRight. You could also add @Logged annotations for the poseDistance values for each camera. This would ensure these vital metrics are consistently captured by AdvantageKit for more in-depth analysis and easier debugging of autonomous runs after the fact. If these SmartDashboard outputs are just temporary for current tuning, then they're perfectly fine as is!

2. Code Clarity - PathPlanner waitTime Values in Autos

  • Files: src/main/deploy/pathplanner/autos/*.auto
  • Details: The race commands for AutonShoot use specific wait times (e.g., 5.0, 10.0, 9.0 seconds). These are likely carefully tuned values.
  • Recommendation: It might be helpful to add a small comment within the PathPlanner UI (if the tool allows it for individual commands, or in the overall auto description) or in the pull request description explaining the rationale behind these specific waitTime durations. For example, "Wait time set to ensure the full shooting sequence completes before StopShot interrupts." This provides valuable context for anyone reviewing or modifying the auto later.

3. Code Clarity - PathPlanner constraintZone Naming

  • Files: src/main/deploy/pathplanner/paths/Collect - LEFT.path, src/main/deploy/pathplanner/paths/Collect - RIGHT.path
  • Details: The newly added constraint zones are currently named "Constraints Zone".
  • Recommendation: While functional, giving these constraintZones a more descriptive name, like "Collect Slowdown Zone" or "Intake Speed Limit Zone", could make their purpose clearer at a glance when working in PathPlanner. This helps with future path modifications and understanding intent!

4. Code Clarity - Vision Subsystem Pose Distance Threshold Multiplier

  • File: src/main/java/frc/robot/subsystems/vision/VisionSubsystem.java
  • Lines: 169-170
  • Details: The line poseDistance > TunerConstants.kSpeedAt12Volts.in(MetersPerSecond) * 0.02 uses 0.02 as a multiplier. This 0.02 is likely a time value, possibly related to latency compensation.
  • Recommendation: To enhance clarity, consider adding a comment to explain the significance of the 0.02 multiplier. Alternatively, if it's a fixed constant (like PathPlanner's default latency compensation), extracting it to a named constant in CameraConstants or TunerConstants (e.g., CameraConstants.POSE_LATENCY_COMPENSATION_SECONDS) would improve readability and maintainability.

Questions

  1. Could you briefly explain the context or reason behind the updates to the pathplanner/settings.json file, particularly the changes to driveWheelRadius, driveGearing, maxDriveSpeed, and the module X/Y coordinates? Knowing if these are due to new physical components, more accurate measurements, or something else would be really helpful for our records!

Overall, this is a fantastic set of changes that significantly refines our robot's autonomous and vision capabilities. Your attention to detail in tuning paths and implementing robust vision checks is truly commendable. Keep up the excellent work – I'm excited to see these improvements in action!


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

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

✓ Build successful and code formatting check passed!

@y0shi y0shi merged commit 6b0ccc7 into development Mar 9, 2026
2 checks passed
@y0shi y0shi deleted the testing/march-8 branch March 9, 2026 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants