-
Notifications
You must be signed in to change notification settings - Fork 1
Clean Object Detection #45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…nk you claude for the suggestion)
instead of comparing the x and y of two translations i compared the translations directly using wpilibs .minus function
switched out for loop with stream
This reverts commit dbb4869.
Nummun14
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
streammm
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCamera.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCameraIO.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.java
Outdated
Show resolved
Hide resolved
...n/java/frc/trigon/robot/misc/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java
Outdated
Show resolved
Hide resolved
...n/java/frc/trigon/robot/misc/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java
Outdated
Show resolved
Hide resolved
...n/java/frc/trigon/robot/misc/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Show resolved
Hide resolved
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
WalkthroughPose-estimation types were renamed and relocated: 🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
src/main/java/frc/trigon/robot/RobotContainer.javasrc/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.javasrc/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCameraConstants.javasrc/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/misc/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/misc/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly and completely, but don't over-explain
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.javasrc/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/RobotContainer.javasrc/main/java/frc/trigon/robot/misc/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCameraConstants.javasrc/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/misc/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCamera.java
🧬 Code graph analysis (5)
src/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.java (1)
src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java (1)
ObjectPoseEstimator(19-214)
src/main/java/frc/trigon/robot/RobotContainer.java (1)
src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java (1)
ObjectPoseEstimator(19-214)
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCameraIO.java (1)
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
ObjectDetectionCameraConstants(5-8)
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java (2)
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
ObjectDetectionCameraConstants(5-8)src/main/java/frc/trigon/robot/misc/simulatedfield/SimulationFieldHandler.java (1)
SimulationFieldHandler(16-161)
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCamera.java (1)
src/main/java/frc/trigon/robot/RobotContainer.java (1)
RobotContainer(26-87)
🔇 Additional comments (12)
src/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.java (1)
10-10: Import path update looks good.The package restructuring aligns with moving ObjectPoseEstimator to the pose estimation module.
src/main/java/frc/trigon/robot/RobotContainer.java (1)
18-18: Import path update is consistent.Matches the package restructuring seen across the PR.
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCameraIO.java (1)
16-18: Terminology update from "target" to "object" is consistent.Field rename and Javadoc update align with the broader API changes across the PR.
src/main/java/frc/trigon/robot/misc/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.java (1)
46-61: Field rename propagated correctly.All references to
hasTargetupdated tohasObjectconsistently throughout the input processing logic.src/main/java/frc/trigon/robot/misc/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java (3)
38-39: Terminology updates are consistent.Field rename from
hasTargettohasObjectaligns with the broader API changes.
54-54: Method rename improves clarity.
calculateVisibleGamePiecesPlacementis more descriptive than the previous name.
90-90: Method rename is more specific.
isObjectWithinFOVbetter conveys that it's checking object visibility.src/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
7-7: Visibility change is appropriate.Making
TRACKED_OBJECT_TOLERANCE_METERSpublic allows ObjectPoseEstimator to reference it for multi-camera tracking logic.src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java (1)
129-140: Naming update improves clarity.Renaming from
bestObjectTranslationtoclosestObjectTranslationbetter reflects the actual selection criteria.src/main/java/frc/trigon/robot/misc/objectdetectioncamera/ObjectDetectionCamera.java (3)
33-56: Method and variable naming improvements.The rename from "best" to "closest" better describes the selection criteria. Variable names updated consistently throughout the method.
58-60: API rename is consistent.
hasObjectaligns with the terminology changes across all object detection files.
73-75: New public method exposes rotations.
getObjectsRotationsprovides access to raw rotation data, which is now used by the refactored object position calculation logic.
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.java (1)
20-20: Unused fieldtargetGamePieceType.This field is assigned in the constructor but never used. Either remove it or complete the intended implementation where it would filter by game piece type.
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.java (2)
40-53: Minor: Javadoc references old class name.Line 41 says "Constructs a new PoseEstimator" but the class is now
RobotPoseEstimator.🔎 Suggested fix
/** - * Constructs a new PoseEstimator and sets the relativeRobotPoseSource. + * Constructs a new RobotPoseEstimator and sets the relativeRobotPoseSource.
55-67: Minor: Javadoc references old class name.Line 56 says "Constructs a new PoseEstimator" but the class is now
RobotPoseEstimator.src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java (2)
82-95: Critical:objectIDparameter is unused - all game piece types reported for every index.The method receives an
objectIDparameter but never filters by it. This causes all visible game pieces to be added to every game piece type index in the array, making type-specific detection impossible.Add filtering by game piece type:
private ArrayList<Pair<SimulatedGamePiece, Rotation3d>> calculateVisibleGamePiecesPlacement(Pose3d cameraPose, int objectID) { final ArrayList<SimulatedGamePiece> gamePiecesOnField = SimulationFieldHandler.getSimulatedGamePieces(); final ArrayList<Pair<SimulatedGamePiece, Rotation3d>> visibleTargetObjects = new ArrayList<>(); for (SimulatedGamePiece currentObject : gamePiecesOnField) { + if (currentObject.getGamePieceType().getID() != objectID) + continue; if (currentObject.isScored()) continue; final Rotation3d cameraAngleToObject = calculateCameraAngleToObject(currentObject.getPose(), cameraPose);
76-82: Method name and javadoc mention "placements" but method returns rotations."Placements" typically refers to positions/poses, but this method returns
Rotation3dvalues (angles relative to the camera). Consider renaming tocalculateVisibleGamePiecesWithRotationsor updating the documentation to clarify that "placement" here means rotation, not position.As per coding guidelines, names must be descriptive and intention-revealing.
♻️ Duplicate comments (1)
src/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.java (1)
152-159: Remove unused method.This method was flagged in a previous review as dead code and still has no callers.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
src/main/java/frc/trigon/robot/RobotContainer.javasrc/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.javasrc/main/java/frc/trigon/robot/constants/CameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/apriltagcamera/AprilTagCamera.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/relativerobotposesource/RelativeRobotPoseSourceConstants.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/StandardDeviations.javasrc/main/java/frc/trigon/robot/subsystems/swerve/Swerve.javasrc/main/java/frc/trigon/robot/subsystems/swerve/SwerveConstants.javasrc/main/java/frc/trigon/robot/subsystems/swerve/swervemodule/SwerveModule.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.javasrc/main/java/frc/trigon/robot/subsystems/swerve/Swerve.javasrc/main/java/frc/trigon/robot/poseestimation/apriltagcamera/AprilTagCamera.javasrc/main/java/frc/trigon/robot/poseestimation/relativerobotposesource/RelativeRobotPoseSourceConstants.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/StandardDeviations.javasrc/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/subsystems/swerve/swervemodule/SwerveModule.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraConstants.javasrc/main/java/frc/trigon/robot/subsystems/swerve/SwerveConstants.javasrc/main/java/frc/trigon/robot/constants/CameraConstants.javasrc/main/java/frc/trigon/robot/RobotContainer.java
🧠 Learnings (2)
📚 Learning: 2025-12-30T00:12:14.902Z
Learnt from: levyishai
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java:161-187
Timestamp: 2025-12-30T00:12:14.902Z
Learning: In the Programming-TRIGON/RobotTemplate Java codebase, follow Uncle Bob's Clean Code principles: prefer self-documenting code through descriptive naming and small, focused methods (method extraction) over adding explanatory comments. Ensure classes, methods, and variables convey intent (naming should be expressive and consistent). Use comments only to explain non-obvious decisions or workarounds, not as a substitute for readable code.
Applied to files:
src/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.javasrc/main/java/frc/trigon/robot/subsystems/swerve/Swerve.javasrc/main/java/frc/trigon/robot/poseestimation/apriltagcamera/AprilTagCamera.javasrc/main/java/frc/trigon/robot/poseestimation/relativerobotposesource/RelativeRobotPoseSourceConstants.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/StandardDeviations.javasrc/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/subsystems/swerve/swervemodule/SwerveModule.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraConstants.javasrc/main/java/frc/trigon/robot/subsystems/swerve/SwerveConstants.javasrc/main/java/frc/trigon/robot/constants/CameraConstants.javasrc/main/java/frc/trigon/robot/RobotContainer.java
📚 Learning: 2025-12-30T20:34:50.231Z
Learnt from: Strflightmight09
Repo: Programming-TRIGON/RobotTemplate PR: 48
File: src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java:180-183
Timestamp: 2025-12-30T20:34:50.231Z
Learning: In template/ scaffolding repositories (e.g., RobotTemplate), allow and preserve optional code patterns that are commented out for users to enable, rather than removing or fully implementing them. During reviews, treat commented-out blocks as intentional placeholders to be potentially enabled by users; avoid enforcing removal of such code and ensure commented sections are clearly documented as optional templates.
Applied to files:
src/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.javasrc/main/java/frc/trigon/robot/subsystems/swerve/Swerve.javasrc/main/java/frc/trigon/robot/poseestimation/apriltagcamera/AprilTagCamera.javasrc/main/java/frc/trigon/robot/poseestimation/relativerobotposesource/RelativeRobotPoseSourceConstants.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/StandardDeviations.javasrc/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/subsystems/swerve/swervemodule/SwerveModule.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraConstants.javasrc/main/java/frc/trigon/robot/subsystems/swerve/SwerveConstants.javasrc/main/java/frc/trigon/robot/constants/CameraConstants.javasrc/main/java/frc/trigon/robot/RobotContainer.java
🧬 Code graph analysis (12)
src/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.java (1)
src/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.java (1)
ObjectPoseEstimator(19-214)
src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java (1)
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.java (1)
RobotPoseEstimatorConstants(3-10)
src/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.java (4)
src/main/java/frc/trigon/robot/RobotContainer.java (1)
RobotContainer(26-87)src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.java (1)
ObjectDetectionCamera(15-108)src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
ObjectDetectionCameraConstants(5-8)src/main/java/frc/trigon/robot/misc/simulatedfield/SimulatedGamePieceConstants.java (1)
SimulatedGamePieceConstants(10-47)
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java (2)
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
ObjectDetectionCameraConstants(5-8)src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraIO.java (1)
ObjectDetectionCameraIO(6-27)
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraIO.java (1)
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
ObjectDetectionCameraConstants(5-8)
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.java (1)
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.java (1)
RobotPoseEstimatorConstants(3-10)
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.java (2)
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
ObjectDetectionCameraConstants(5-8)src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraIO.java (1)
ObjectDetectionCameraIO(6-27)
src/main/java/frc/trigon/robot/subsystems/swerve/swervemodule/SwerveModule.java (1)
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.java (1)
RobotPoseEstimatorConstants(3-10)
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
src/main/java/frc/trigon/robot/misc/simulatedfield/SimulatedGamePieceConstants.java (1)
SimulatedGamePieceConstants(10-47)
src/main/java/frc/trigon/robot/subsystems/swerve/SwerveConstants.java (1)
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.java (1)
RobotPoseEstimatorConstants(3-10)
src/main/java/frc/trigon/robot/constants/CameraConstants.java (1)
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.java (1)
ObjectDetectionCamera(15-108)
src/main/java/frc/trigon/robot/RobotContainer.java (2)
src/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.java (1)
ObjectPoseEstimator(19-214)src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.java (1)
RobotPoseEstimator(32-287)
🔇 Additional comments (20)
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.java (1)
1-1: LGTM!The package relocation and terminology changes from "target" to "object" and "best" to "closest" are consistent and improve clarity. Method and variable names now accurately describe their behavior.
Also applies to: 6-7, 34-35, 39-41, 58-59, 63-63, 73-73, 78-78
src/main/java/frc/trigon/robot/constants/CameraConstants.java (1)
7-7: LGTM!Import path correctly updated to reflect the new package location.
src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java (1)
15-15: LGTM!Import and usage correctly updated to reference the renamed
RobotPoseEstimatorConstantsclass.Also applies to: 37-37
src/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.java (1)
10-10: LGTM!Import path correctly updated to the new package location.
src/main/java/frc/trigon/robot/poseestimation/apriltagcamera/AprilTagCamera.java (1)
7-7: LGTM!Import path correctly updated to reflect the relocated
StandardDeviationsclass.src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
1-8: LGTM!Constants are well-structured and properly centralized. Using
values().lengthkeepsNUMBER_OF_GAME_PIECE_TYPESin sync with enum changes automatically.src/main/java/frc/trigon/robot/subsystems/swerve/SwerveConstants.java (1)
13-13: LGTM!Import and usage correctly updated to reference the renamed
RobotPoseEstimatorConstantsclass, consistent with changes inSwerve.java.Also applies to: 110-110
src/main/java/frc/trigon/robot/poseestimation/relativerobotposesource/RelativeRobotPoseSourceConstants.java (1)
3-3: LGTM!Import path correctly updated to reflect the relocated
StandardDeviationsclass.src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/StandardDeviations.java (1)
1-1: LGTM!Package relocation aligns with the broader reorganization of pose estimation components.
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.java (1)
1-10: LGTM!Class and package rename is consistent with the broader refactoring. Constants are preserved correctly.
src/main/java/frc/trigon/robot/subsystems/swerve/swervemodule/SwerveModule.java (1)
12-12: LGTM!Import and constant references updated consistently with the
RobotPoseEstimatorConstantsrename.Also applies to: 203-203, 212-212
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCameraIO.java (1)
1-1: LGTM!Package relocation and terminology change from
hasTargettohasObjectare consistent with the broader refactoring.Also applies to: 16-18
src/main/java/frc/trigon/robot/RobotContainer.java (1)
18-18: LGTM!Import and type updates correctly reference the renamed
ObjectPoseEstimatorandRobotPoseEstimatorclasses.Also applies to: 20-20, 27-27
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.java (2)
1-7: LGTM!Package and import updates align with the object detection camera reorganization.
45-67: LGTM!Field references consistently updated from
hasTargettohasObject. Logic remains correct.src/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.java (3)
1-1: LGTM!Package relocation to
poseestimationis appropriate given this class is part of the pose estimation system.
22-42: LGTM!Multi-camera support added cleanly via varargs. Constructor and field updates are correct.
129-140: LGTM!Variable rename from
bestObjectTranslationtoclosestObjectTranslationimproves clarity.src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.java (2)
1-1: LGTM!Package and class rename to
RobotPoseEstimatoris consistent with the broader refactoring.Also applies to: 32-32
210-221: LGTM!Constant references correctly updated to
RobotPoseEstimatorConstants. The commented-outsortCamerasByLatestResultTimestampis appropriate for a template repository. Based on learnings, this is an intentional optional pattern.Also applies to: 232-233, 283-283
...n/robot/misc/objectDetection/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java
Outdated
Show resolved
Hide resolved
...n/java/frc/trigon/robot/misc/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.java (1)
40-46: Update javadoc to reflect class rename.The javadoc still references "PoseEstimator" but the class has been renamed to "RobotPoseEstimator".
🔎 Proposed fix
/** - * Constructs a new PoseEstimator and sets the relativeRobotPoseSource. + * Constructs a new RobotPoseEstimator and sets the relativeRobotPoseSource. * This constructor enables usage of a relative robot pose source and disables the use of april tags for pose estimation, and instead uses them to reset the relative robot pose source's offset. * * @param relativeRobotPoseSource the relative robot pose source that should be used to update the pose estimator * @param aprilTagCameras the apriltag cameras that should be used to update the relative robot pose source */
♻️ Duplicate comments (1)
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.java (1)
46-55: Recalculating distance on every iteration is inefficient.Line 50 recalculates
closestObjectDistanceToRobotevery loop iteration, but this value only needs updating when you find a closer object. Calculate it once before the loop and update it inside the if block.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
src/main/java/frc/trigon/robot/RobotContainer.javasrc/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.javasrc/main/java/frc/trigon/robot/subsystems/swerve/Swerve.javasrc/main/java/frc/trigon/robot/subsystems/swerve/SwerveConstants.javasrc/main/java/frc/trigon/robot/subsystems/swerve/swervemodule/SwerveModule.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.javasrc/main/java/frc/trigon/robot/subsystems/swerve/SwerveConstants.javasrc/main/java/frc/trigon/robot/subsystems/swerve/swervemodule/SwerveModule.javasrc/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.javasrc/main/java/frc/trigon/robot/RobotContainer.java
🧠 Learnings (2)
📚 Learning: 2025-12-30T00:12:14.902Z
Learnt from: levyishai
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java:161-187
Timestamp: 2025-12-30T00:12:14.902Z
Learning: In the Programming-TRIGON/RobotTemplate Java codebase, follow Uncle Bob's Clean Code principles: prefer self-documenting code through descriptive naming and small, focused methods (method extraction) over adding explanatory comments. Ensure classes, methods, and variables convey intent (naming should be expressive and consistent). Use comments only to explain non-obvious decisions or workarounds, not as a substitute for readable code.
Applied to files:
src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.javasrc/main/java/frc/trigon/robot/subsystems/swerve/SwerveConstants.javasrc/main/java/frc/trigon/robot/subsystems/swerve/swervemodule/SwerveModule.javasrc/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.javasrc/main/java/frc/trigon/robot/RobotContainer.java
📚 Learning: 2025-12-30T20:34:50.231Z
Learnt from: Strflightmight09
Repo: Programming-TRIGON/RobotTemplate PR: 48
File: src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java:180-183
Timestamp: 2025-12-30T20:34:50.231Z
Learning: In template/ scaffolding repositories (e.g., RobotTemplate), allow and preserve optional code patterns that are commented out for users to enable, rather than removing or fully implementing them. During reviews, treat commented-out blocks as intentional placeholders to be potentially enabled by users; avoid enforcing removal of such code and ensure commented sections are clearly documented as optional templates.
Applied to files:
src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.javasrc/main/java/frc/trigon/robot/subsystems/swerve/SwerveConstants.javasrc/main/java/frc/trigon/robot/subsystems/swerve/swervemodule/SwerveModule.javasrc/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.javasrc/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.javasrc/main/java/frc/trigon/robot/RobotContainer.java
🧬 Code graph analysis (7)
src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java (1)
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.java (1)
RobotPoseEstimatorConstants(3-10)
src/main/java/frc/trigon/robot/subsystems/swerve/SwerveConstants.java (1)
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.java (1)
RobotPoseEstimatorConstants(3-10)
src/main/java/frc/trigon/robot/subsystems/swerve/swervemodule/SwerveModule.java (1)
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.java (1)
RobotPoseEstimatorConstants(3-10)
src/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.java (1)
src/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.java (1)
ObjectPoseEstimator(19-214)
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.java (2)
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.java (1)
PhotonObjectDetectionCameraIO(16-85)src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java (1)
SimulationObjectDetectionCameraIO(17-159)
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.java (1)
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.java (1)
RobotPoseEstimatorConstants(3-10)
src/main/java/frc/trigon/robot/RobotContainer.java (1)
src/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.java (1)
ObjectPoseEstimator(19-214)
🔇 Additional comments (8)
src/main/java/frc/trigon/robot/commands/commandclasses/GamePieceAutoDriveCommand.java (1)
10-10: LGTM - Import path correctly updated.The import properly reflects ObjectPoseEstimator's new package location under
poseestimation.src/main/java/frc/trigon/robot/subsystems/swerve/SwerveConstants.java (1)
13-13: LGTM - Constants reference correctly updated.Both the import and usage of
ODOMETRY_FREQUENCY_HERTZproperly reference the newRobotPoseEstimatorConstantsclass.Also applies to: 110-110
src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java (1)
14-14: LGTM - Odometry frequency reference properly updated.The import and constant usage correctly reference
RobotPoseEstimatorConstants.ODOMETRY_FREQUENCY_HERTZ.Also applies to: 37-37
src/main/java/frc/trigon/robot/subsystems/swerve/swervemodule/SwerveModule.java (1)
12-12: LGTM - All odometry frequency references updated.Import and both usages of
ODOMETRY_FREQUENCY_HERTZcorrectly reference the renamedRobotPoseEstimatorConstants.Also applies to: 203-203, 212-212
src/main/java/frc/trigon/robot/RobotContainer.java (1)
19-21: LGTM - Pose estimator references correctly updated.All imports and the
ROBOT_POSE_ESTIMATORdeclaration properly reflect the renamed classes and new package structure:
ObjectPoseEstimatormoved toposeestimationpackagePoseEstimatorrenamed toRobotPoseEstimatorin newrobotposeestimatorpackageAlso applies to: 27-27
src/main/java/frc/trigon/robot/poseestimation/objectdetectioncamera/ObjectDetectionCamera.java (3)
1-1: Package reorganization improves logical structure.Moving object detection cameras under
poseestimationbetter reflects their role in the system.Also applies to: 6-7
34-41: Better naming: "closest" is more precise than "best".The rename clearly communicates the selection criterion without requiring explanation.
58-60: API rename applied consistently.The "target" → "object" terminology update is clear and consistent across method names and field accesses.
Also applies to: 63-63, 73-75
.../java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagLimelightIO.java (1)
41-58: Critical: Array index bug in getVisibleTagIDsThe method has two logic errors:
When the best target is at
visibleTags[0], the loop setsidAddition = 0on the first iteration, then line 55 writes tovisibleTagIDs[0], overwriting the best target ID that was just set.When the best target appears later in the array (e.g., at index 5), line 55 attempts to write to
visibleTagIDs[i + idAddition]which can exceed array bounds (e.g.,visibleTagIDs[6]when length is 6).Suggested fix
private int[] getVisibleTagIDs(LimelightHelpers.LimelightResults results) { final LimelightHelpers.LimelightTarget_Fiducial[] visibleTags = results.targets_Fiducials; final int[] visibleTagIDs = new int[visibleTags.length]; - visibleTagIDs[0] = (int) getBestTarget(results).fiducialID; - int idAddition = 1; + final int bestTargetID = (int) getBestTarget(results).fiducialID; + visibleTagIDs[0] = bestTargetID; + int nextIndex = 1; for (int i = 0; i < visibleTags.length; i++) { final int currentID = (int) visibleTags[i].fiducialID; - if (currentID == visibleTagIDs[0]) { - idAddition = 0; + if (currentID == bestTargetID) { continue; } - visibleTagIDs[i + idAddition] = currentID; + visibleTagIDs[nextIndex++] = currentID; } return visibleTagIDs; }src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.java (1)
40-42: Update constructor comments to match class rename.The JavaDoc comments still refer to "PoseEstimator" but should say "RobotPoseEstimator" to match the renamed class.
🔎 Suggested fix
- * Constructs a new PoseEstimator and sets the relativeRobotPoseSource. + * Constructs a new RobotPoseEstimator and sets the relativeRobotPoseSource.- * Constructs a new PoseEstimator. + * Constructs a new RobotPoseEstimator.Also applies to: 55-57
src/main/java/frc/trigon/robot/constants/CameraConstants.java (1)
10-13: Clarify if zero-transform is a placeholder.The all-zero transform (camera at exact robot center, no rotation) appears to be a template placeholder. Consider making this explicit so users know to configure their actual camera position, perhaps with a TODO comment or a more obviously placeholder value.
♻️ Duplicate comments (6)
src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java (2)
36-40: RenamehasAnyTargettohasAnyObjectfor consistency.The variable still uses "target" terminology while the rest of the code now uses "object" (e.g.,
inputs.hasObject,isObjectWithinFOV).
82-91: RenamevisibleTargetObjectstovisibleObjects.Line 84: The variable name still includes "target" while the method and other identifiers have been updated to use "object" terminology.
src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCamera.java (1)
41-55: RecalculatingclosestObjectDistanceToRoboton every iteration is inefficient.Move the distance calculation outside the loop and update it only when
closestObjectTranslationchanges.Proposed fix
Translation2d closestObjectTranslation = targetObjectsTranslation[0]; + double closestObjectDistanceToRobot = currentRobotTranslation.getDistance(closestObjectTranslation); for (int i = 1; i < targetObjectsTranslation.length; i++) { final Translation2d currentObjectTranslation = targetObjectsTranslation[i]; - final double closestObjectDistanceToRobot = currentRobotTranslation.getDistance(closestObjectTranslation); final double currentObjectDistanceToRobot = currentRobotTranslation.getDistance(currentObjectTranslation); - if (currentObjectDistanceToRobot < closestObjectDistanceToRobot) + if (currentObjectDistanceToRobot < closestObjectDistanceToRobot) { + closestObjectDistanceToRobot = currentObjectDistanceToRobot; closestObjectTranslation = currentObjectTranslation; + } }src/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.java (3)
152-159: Remove unused methodcalculateObjectDistanceRatingFromPose.This method is never called anywhere in the codebase.
179-182: Bug: ComparingDoublewrapper objects with!=.
knownObjectPositions.get(closestObjectToVisibleObject)returns aDoubleobject. Using!=compares object references, not values. TwoDoubleinstances with the same value will incorrectly evaluate totrue.Use
.equals()instead:Proposed fix
if (closestObjectToVisibleObject != null && closestObjectToVisibleObjectDistanceMeters < ObjectDetectionCameraConstants.TRACKED_OBJECT_TOLERANCE_METERS - && knownObjectPositions.get(closestObjectToVisibleObject) != currentTimestamp) + && !knownObjectPositions.get(closestObjectToVisibleObject).equals(currentTimestamp)) removeObject(closestObjectToVisibleObject);
161-187: Multi-camera tracking logic would benefit from method extraction.Per the team's preference for self-documenting code through small, focused methods (based on learnings), consider extracting:
- Finding the closest unprocessed known object
- The condition check for replacing a known object
This would make the algorithm more readable without needing comments.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
src/main/java/frc/trigon/robot/constants/CameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCamera.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagLimelightIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagPhotonCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagSimulationCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagLimelightIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/constants/CameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagSimulationCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCamera.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagPhotonCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCameraIO.java
🧠 Learnings (2)
📚 Learning: 2025-12-30T00:12:14.902Z
Learnt from: levyishai
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java:161-187
Timestamp: 2025-12-30T00:12:14.902Z
Learning: In the Programming-TRIGON/RobotTemplate Java codebase, follow Uncle Bob's Clean Code principles: prefer self-documenting code through descriptive naming and small, focused methods (method extraction) over adding explanatory comments. Ensure classes, methods, and variables convey intent (naming should be expressive and consistent). Use comments only to explain non-obvious decisions or workarounds, not as a substitute for readable code.
Applied to files:
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagLimelightIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/constants/CameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagSimulationCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCamera.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagPhotonCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCameraIO.java
📚 Learning: 2025-12-30T20:34:50.231Z
Learnt from: Strflightmight09
Repo: Programming-TRIGON/RobotTemplate PR: 48
File: src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java:180-183
Timestamp: 2025-12-30T20:34:50.231Z
Learning: In template/ scaffolding repositories (e.g., RobotTemplate), allow and preserve optional code patterns that are commented out for users to enable, rather than removing or fully implementing them. During reviews, treat commented-out blocks as intentional placeholders to be potentially enabled by users; avoid enforcing removal of such code and ensure commented sections are clearly documented as optional templates.
Applied to files:
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagLimelightIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/constants/CameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagSimulationCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCamera.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCameraConstants.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagPhotonCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCameraIO.java
🧬 Code graph analysis (10)
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagLimelightIO.java (1)
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCameraIO.java (1)
AprilTagCameraIO(8-33)
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagSimulationCameraIO.java (1)
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCameraConstants.java (1)
AprilTagCameraConstants(17-66)
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.java (2)
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCamera.java (1)
AprilTagCamera(15-184)src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimatorConstants.java (1)
RobotPoseEstimatorConstants(3-10)
src/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.java (4)
src/main/java/frc/trigon/robot/RobotContainer.java (1)
RobotContainer(26-87)src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCamera.java (1)
ObjectDetectionCamera(15-108)src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
ObjectDetectionCameraConstants(5-8)src/main/java/frc/trigon/robot/misc/simulatedfield/SimulatedGamePieceConstants.java (1)
SimulatedGamePieceConstants(10-47)
src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
src/main/java/frc/trigon/robot/misc/simulatedfield/SimulatedGamePieceConstants.java (1)
SimulatedGamePieceConstants(10-47)
src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.java (2)
src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
ObjectDetectionCameraConstants(5-8)src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraIO.java (1)
ObjectDetectionCameraIO(6-27)
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCamera.java (2)
src/main/java/frc/trigon/robot/RobotContainer.java (1)
RobotContainer(26-87)src/main/java/frc/trigon/robot/constants/FieldConstants.java (1)
FieldConstants(16-47)
src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraIO.java (1)
src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
ObjectDetectionCameraConstants(5-8)
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagPhotonCameraIO.java (1)
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCameraConstants.java (1)
AprilTagCameraConstants(17-66)
src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java (3)
src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
ObjectDetectionCameraConstants(5-8)src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraIO.java (1)
ObjectDetectionCameraIO(6-27)src/main/java/frc/trigon/robot/misc/simulatedfield/SimulationFieldHandler.java (1)
SimulationFieldHandler(16-161)
🔇 Additional comments (21)
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagLimelightIO.java (1)
1-1: LGTM - namespace refactoringPackage and import updates correctly reflect the new directory structure.
Also applies to: 5-6
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagPhotonCameraIO.java (1)
1-1: LGTM - namespace refactoringPackage and import paths correctly updated to the new camera structure.
Also applies to: 13-15
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCameraIO.java (1)
1-1: LGTM - namespace refactoringPackage declaration correctly updated to new cameras subdirectory.
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCameraConstants.java (1)
1-1: LGTM - namespace refactoringPackage and imports correctly updated to align with new cameras directory structure.
Also applies to: 7-9
src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/AprilTagCamera.java (1)
1-7: Package and import path updates look correct.The package relocation to
cameras.apriltagcameraand the import update forStandardDeviationsalign with the broader refactoring in this PR.src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraConstants.java (1)
5-8: Clean constants class.Well-structured with descriptive names. The dynamic computation of
NUMBER_OF_GAME_PIECE_TYPESfrom the enum ensures it stays in sync automatically.src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCameraIO.java (1)
14-18: Terminology update is consistent.The rename from
hasTargettohasObjectaligns with the standardized terminology across the codebase.src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java (1)
1-9: Package and import updates are correct.The reorganization to
cameras.objectdetectioncamera.ioand updated import paths are consistent with the broader refactoring.src/main/java/frc/trigon/robot/poseestimation/cameras/apriltagcamera/io/AprilTagSimulationCameraIO.java (1)
1-15: Package update and simulation setup look good.The reorganization is consistent with the rest of the PR. The simulation camera initialization properly uses the shared
SIMULATION_CAMERA_PROPERTIESand registers with the vision simulation.src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/ObjectDetectionCamera.java (2)
1-10: Package and import updates are consistent.The relocation to
cameras.objectdetectioncameraaligns with the broader package reorganization.
58-60: Terminology update tohasObjectis correct.Consistent with the terminology shift throughout the codebase.
src/main/java/frc/trigon/robot/poseestimation/ObjectPoseEstimator.java (2)
1-1: Package placement is now correct.The
ObjectPoseEstimatoris atfrc.trigon.robot.poseestimation, which addresses the previous feedback about not belonging underobjectdetectioncamera.
22-42: Multi-camera support via varargs is clean.The constructor signature with
ObjectDetectionCamera... camerasenables flexible multi-camera configurations while the internal array storage is straightforward.src/main/java/frc/trigon/robot/poseestimation/cameras/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.java (3)
1-1: LGTM - Package reorganization is consistent.The package path and imports correctly reflect the new structure under
poseestimation.cameras.Also applies to: 5-7
46-46: LGTM - Terminology update is consistent.The field name change from
hasTargettohasObjectaligns with the object detection camera terminology.
54-54: LGTM - Field usage consistently updated.All references to the field correctly use
hasObjectinstead ofhasTarget.Also applies to: 61-61
src/main/java/frc/trigon/robot/poseestimation/robotposeestimator/RobotPoseEstimator.java (4)
1-1: LGTM - Package and imports updated correctly.The package structure and AprilTagCamera import path reflect the new camera organization under
poseestimation.cameras.Also applies to: 17-17
32-32: LGTM - Class rename is appropriate.Renaming to
RobotPoseEstimatorbetter distinguishes this from other pose estimators (likeObjectPoseEstimator).
47-47: LGTM - Constructor signatures updated correctly.Both constructor signatures properly use the new
RobotPoseEstimatorclass name.Also applies to: 61-61
232-233: LGTM - Constants references updated consistently.All references correctly use
RobotPoseEstimatorConstantsinstead of the oldPoseEstimatorConstants.Also applies to: 283-283
src/main/java/frc/trigon/robot/constants/CameraConstants.java (1)
7-7: Import path update looks good.The updated import aligns with the package reorganization mentioned in the PR objectives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧠 Learnings (5)
📓 Common learnings
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:07.671Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
📚 Learning: 2026-01-12T16:16:07.671Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:07.671Z
Learning: In ObjectPoseEstimator.updateObjectsPositions() (src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java), keep the call to excludedKnownObjects.clear() inside the loop so that each visible camera object uses a fresh exclusion list when updateObjectPosition() is called. Moving the clear() call outside the loop can cause stale exclusions and mis-handle discarded object updates. This pattern is specific to this file and should be preserved to ensure correct per-object exclusion state. Verify with tests that object updates are processed correctly per object and that there are no unintended cross-object exclusions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2025-12-30T00:12:14.902Z
Learnt from: levyishai
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java:161-187
Timestamp: 2025-12-30T00:12:14.902Z
Learning: In the Programming-TRIGON/RobotTemplate Java codebase, follow Uncle Bob's Clean Code principles: prefer self-documenting code through descriptive naming and small, focused methods (method extraction) over adding explanatory comments. Ensure classes, methods, and variables convey intent (naming should be expressive and consistent). Use comments only to explain non-obvious decisions or workarounds, not as a substitute for readable code.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2026-01-11T12:11:54.447Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:30-37
Timestamp: 2026-01-11T12:11:54.447Z
Learning: In the RobotTemplate codebase (Java/FRC), prefer silent failure (no-op) over throwing exceptions for certain validation scenarios. Specifically, for constructors or methods dealing with varargs (e.g., cameras), do not throw on invalid input such as zero elements; instead, perform a no-op or safe default if zero cameras are provided. This pattern should be applied to Java files under the frc/trigon/robot area (e.g., src/main/java/frc/trigon/robot/**/*.java). Document the rationale for silent failures where used, and ensure this behavior is clearly tested and reviewed to avoid hidden issues or regressions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2025-12-30T20:34:50.231Z
Learnt from: Strflightmight09
Repo: Programming-TRIGON/RobotTemplate PR: 48
File: src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java:180-183
Timestamp: 2025-12-30T20:34:50.231Z
Learning: In template/ scaffolding repositories (e.g., RobotTemplate), allow and preserve optional code patterns that are commented out for users to enable, rather than removing or fully implementing them. During reviews, treat commented-out blocks as intentional placeholders to be potentially enabled by users; avoid enforcing removal of such code and ensure commented sections are clearly documented as optional templates.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧬 Code graph analysis (1)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java (4)
src/main/java/frc/trigon/robot/RobotContainer.java (1)
RobotContainer(26-86)src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java (1)
ObjectDetectionCamera(15-108)src/main/java/frc/trigon/robot/misc/simulatedfield/SimulatedGamePieceConstants.java (1)
SimulatedGamePieceConstants(10-47)src/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java (1)
ObjectDetectionConstants(5-8)
🔇 Additional comments (4)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java (4)
47-52: LGTM!Clean periodic update cycle with appropriate logging.
59-132: LGTM!Public API is well-structured: defensive copies in
getObjectsOnField(), consistent early-return null handling, and clear verb-based naming.
160-187: LGTM!The conflict resolution logic correctly handles reassigning discarded updates. The fresh
HashSetper conflict (line 162) aligns with the intended pattern for handling discarded object updates. Based on learnings, this is the correct approach.
189-232: LGTM!Helper methods are concise, well-named, and follow single responsibility. The linear search in
getClosestObjectFromSetToPosition()is appropriate for the expected small number of tracked objects in FRC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧠 Learnings (5)
📓 Common learnings
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:07.671Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
📚 Learning: 2026-01-12T16:16:07.671Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:07.671Z
Learning: In ObjectPoseEstimator.updateObjectsPositions() (src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java), keep the call to excludedKnownObjects.clear() inside the loop so that each visible camera object uses a fresh exclusion list when updateObjectPosition() is called. Moving the clear() call outside the loop can cause stale exclusions and mis-handle discarded object updates. This pattern is specific to this file and should be preserved to ensure correct per-object exclusion state. Verify with tests that object updates are processed correctly per object and that there are no unintended cross-object exclusions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2025-12-30T00:12:14.902Z
Learnt from: levyishai
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java:161-187
Timestamp: 2025-12-30T00:12:14.902Z
Learning: In the Programming-TRIGON/RobotTemplate Java codebase, follow Uncle Bob's Clean Code principles: prefer self-documenting code through descriptive naming and small, focused methods (method extraction) over adding explanatory comments. Ensure classes, methods, and variables convey intent (naming should be expressive and consistent). Use comments only to explain non-obvious decisions or workarounds, not as a substitute for readable code.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2026-01-11T12:11:54.447Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:30-37
Timestamp: 2026-01-11T12:11:54.447Z
Learning: In the RobotTemplate codebase (Java/FRC), prefer silent failure (no-op) over throwing exceptions for certain validation scenarios. Specifically, for constructors or methods dealing with varargs (e.g., cameras), do not throw on invalid input such as zero elements; instead, perform a no-op or safe default if zero cameras are provided. This pattern should be applied to Java files under the frc/trigon/robot area (e.g., src/main/java/frc/trigon/robot/**/*.java). Document the rationale for silent failures where used, and ensure this behavior is clearly tested and reviewed to avoid hidden issues or regressions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2025-12-30T20:34:50.231Z
Learnt from: Strflightmight09
Repo: Programming-TRIGON/RobotTemplate PR: 48
File: src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java:180-183
Timestamp: 2025-12-30T20:34:50.231Z
Learning: In template/ scaffolding repositories (e.g., RobotTemplate), allow and preserve optional code patterns that are commented out for users to enable, rather than removing or fully implementing them. During reviews, treat commented-out blocks as intentional placeholders to be potentially enabled by users; avoid enforcing removal of such code and ensure commented sections are clearly documented as optional templates.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🔇 Additional comments (6)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java (6)
22-25: Potential issue: UsingTranslation2das HashMap key.
Translation2drelies on floating-point equality. Tiny precision differences could cause failed lookups. This works here because you always use the exact stored object for removal, but be aware if you ever construct a newTranslation2dwith "equal" values for lookup—it may not find the entry.
47-52: LGTM.Clean periodic cycle: update, prune, log. The flow is easy to follow.
59-132: LGTM.Public API is well-structured: defensive copies, consistent null handling, clear naming. The static
RobotContainer.ROBOT_POSE_ESTIMATORreferences create coupling, but this pattern appears standard in your codebase.
134-143: LGTM.Clean separation between detection phase and commit phase. The exclusion set is now created per-collision (line 162) rather than cleared in a loop—correct handling per learnings.
160-192: Acceptable complexity for the problem domain.The collision resolution logic (multiple detections mapping to one tracked object) is inherently complex. The recursive call in
reassignDiscardedObjectUpdateis bounded by the number of tracked objects, which is limited in FRC. Method names likeaddClosestUpdatedPositionToHashMapare long but self-documenting per Clean Code principles.
200-232: LGTM.Utility methods are small, focused, and follow single responsibility. The
removeIfusage inremoveOldObjectsis efficient and idiomatic.
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧠 Learnings (5)
📓 Common learnings
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:07.671Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
📚 Learning: 2026-01-12T16:16:07.671Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:07.671Z
Learning: In ObjectPoseEstimator.updateObjectsPositions() (src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java), keep the call to excludedKnownObjects.clear() inside the loop so that each visible camera object uses a fresh exclusion list when updateObjectPosition() is called. Moving the clear() call outside the loop can cause stale exclusions and mis-handle discarded object updates. This pattern is specific to this file and should be preserved to ensure correct per-object exclusion state. Verify with tests that object updates are processed correctly per object and that there are no unintended cross-object exclusions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2025-12-30T00:12:14.902Z
Learnt from: levyishai
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java:161-187
Timestamp: 2025-12-30T00:12:14.902Z
Learning: In the Programming-TRIGON/RobotTemplate Java codebase, follow Uncle Bob's Clean Code principles: prefer self-documenting code through descriptive naming and small, focused methods (method extraction) over adding explanatory comments. Ensure classes, methods, and variables convey intent (naming should be expressive and consistent). Use comments only to explain non-obvious decisions or workarounds, not as a substitute for readable code.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2026-01-11T12:11:54.447Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:30-37
Timestamp: 2026-01-11T12:11:54.447Z
Learning: In the RobotTemplate codebase (Java/FRC), prefer silent failure (no-op) over throwing exceptions for certain validation scenarios. Specifically, for constructors or methods dealing with varargs (e.g., cameras), do not throw on invalid input such as zero elements; instead, perform a no-op or safe default if zero cameras are provided. This pattern should be applied to Java files under the frc/trigon/robot area (e.g., src/main/java/frc/trigon/robot/**/*.java). Document the rationale for silent failures where used, and ensure this behavior is clearly tested and reviewed to avoid hidden issues or regressions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2025-12-30T20:34:50.231Z
Learnt from: Strflightmight09
Repo: Programming-TRIGON/RobotTemplate PR: 48
File: src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java:180-183
Timestamp: 2025-12-30T20:34:50.231Z
Learning: In template/ scaffolding repositories (e.g., RobotTemplate), allow and preserve optional code patterns that are commented out for users to enable, rather than removing or fully implementing them. During reviews, treat commented-out blocks as intentional placeholders to be potentially enabled by users; avoid enforcing removal of such code and ensure commented sections are clearly documented as optional templates.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧬 Code graph analysis (1)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java (4)
src/main/java/frc/trigon/robot/RobotContainer.java (1)
RobotContainer(26-86)src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java (1)
ObjectDetectionCamera(15-108)src/main/java/frc/trigon/robot/misc/simulatedfield/SimulatedGamePieceConstants.java (1)
SimulatedGamePieceConstants(10-47)src/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java (1)
ObjectDetectionConstants(5-8)
🔇 Additional comments (8)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java (8)
47-52: LGTM!The periodic loop correctly updates, prunes, and logs in the right order.
59-114: LGTM!All public remove methods have proper null guards and the defensive copy in
getObjectsOnField()prevents external modification of internal state.
121-132: LGTM!Simple, clear implementations with appropriate delegation.
134-150: LGTM!The update and commit flow correctly separates building the update map from applying changes, preventing concurrent modification issues.
152-166: LGTM!The update logic correctly handles the case where multiple visible objects might map to the same tracked object by creating fresh exclusion sets per conflict resolution.
168-187: LGTM!The conflict resolution algorithm correctly handles the case where multiple visible objects compete for the same tracked object, with appropriate fallback to treating discarded updates as new objects.
189-208: LGTM!Helper methods are clear and use appropriate defensive copies.
210-232: LGTM!Standard distance-finding and age-based cleanup patterns implemented correctly.
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Show resolved
Hide resolved
|
|
||
| /** | ||
| * Calculates the position of the best object on the field from the 3D rotation of the object relative to the camera. | ||
| * Calculates the position of the closest object on the field from its 3D rotation relative to the camera. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on Their positions (plural)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
.../java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
Outdated
Show resolved
Hide resolved
.../java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
Outdated
Show resolved
Hide resolved
.../java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
Outdated
Show resolved
Hide resolved
...ava/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCameraIO.java
Outdated
Show resolved
Hide resolved
...ava/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCameraIO.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java (1)
92-97: Potential division by zero when pitch is zero.If
objectPitchSinis 0 (object at camera height),xTransformbecomesInfinity, which propagates to the returned position. Consider adding a guard.Suggested fix
final double cameraZ = cameraPose.getTranslation().getZ(); final double objectPitchSin = Math.sin(objectRotationStart.getRotation().getY()); + if (Math.abs(objectPitchSin) < 1e-9) + return new Translation2d(); final double xTransform = cameraZ / objectPitchSin;
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCameraIO.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java
🧠 Learnings (6)
📓 Common learnings
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:12.465Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
📚 Learning: 2026-01-12T16:16:07.671Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:07.671Z
Learning: In ObjectPoseEstimator.updateObjectsPositions() (src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java), keep the call to excludedKnownObjects.clear() inside the loop so that each visible camera object uses a fresh exclusion list when updateObjectPosition() is called. Moving the clear() call outside the loop can cause stale exclusions and mis-handle discarded object updates. This pattern is specific to this file and should be preserved to ensure correct per-object exclusion state. Verify with tests that object updates are processed correctly per object and that there are no unintended cross-object exclusions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2025-12-30T00:12:14.902Z
Learnt from: levyishai
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java:161-187
Timestamp: 2025-12-30T00:12:14.902Z
Learning: In the Programming-TRIGON/RobotTemplate Java codebase, follow Uncle Bob's Clean Code principles: prefer self-documenting code through descriptive naming and small, focused methods (method extraction) over adding explanatory comments. Ensure classes, methods, and variables convey intent (naming should be expressive and consistent). Use comments only to explain non-obvious decisions or workarounds, not as a substitute for readable code.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java
📚 Learning: 2026-01-11T12:11:54.447Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:30-37
Timestamp: 2026-01-11T12:11:54.447Z
Learning: In the RobotTemplate codebase (Java/FRC), prefer silent failure (no-op) over throwing exceptions for certain validation scenarios. Specifically, for constructors or methods dealing with varargs (e.g., cameras), do not throw on invalid input such as zero elements; instead, perform a no-op or safe default if zero cameras are provided. This pattern should be applied to Java files under the frc/trigon/robot area (e.g., src/main/java/frc/trigon/robot/**/*.java). Document the rationale for silent failures where used, and ensure this behavior is clearly tested and reviewed to avoid hidden issues or regressions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java
📚 Learning: 2025-12-30T20:34:50.231Z
Learnt from: Strflightmight09
Repo: Programming-TRIGON/RobotTemplate PR: 48
File: src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java:180-183
Timestamp: 2025-12-30T20:34:50.231Z
Learning: In template/ scaffolding repositories (e.g., RobotTemplate), allow and preserve optional code patterns that are commented out for users to enable, rather than removing or fully implementing them. During reviews, treat commented-out blocks as intentional placeholders to be potentially enabled by users; avoid enforcing removal of such code and ensure commented sections are clearly documented as optional templates.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCameraIO.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java
📚 Learning: 2026-01-12T16:16:12.465Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:12.465Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java
🧬 Code graph analysis (4)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java (4)
src/main/java/frc/trigon/robot/RobotContainer.java (1)
RobotContainer(26-86)src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java (1)
ObjectDetectionCamera(15-107)src/main/java/frc/trigon/robot/misc/simulatedfield/SimulatedGamePieceConstants.java (1)
SimulatedGamePieceConstants(10-47)src/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java (1)
ObjectDetectionConstants(5-8)
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java (1)
src/main/java/frc/trigon/robot/RobotContainer.java (1)
RobotContainer(26-86)
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCameraIO.java (1)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java (1)
ObjectDetectionConstants(5-8)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java (1)
src/main/java/frc/trigon/robot/misc/simulatedfield/SimulatedGamePieceConstants.java (1)
SimulatedGamePieceConstants(10-47)
🔇 Additional comments (13)
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCameraIO.java (1)
20-27: LGTM!The
visibleObjectRotationsfield is well-documented and properly initialized with the constants-driven array size.src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java (3)
57-59: LGTM!Clean accessor using the game piece type's ID for indexing.
61-74: LGTM!The renamed methods (
getObjectPositionsOnField,getObjectsRotations) are clear and follow the new "object" terminology consistently.
100-106: LGTM!Clean factory method for selecting the appropriate IO implementation based on robot hardware state.
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java (9)
47-52: LGTM!The
periodic()method follows a clear sequence: update tracked positions, prune stale entries, then log. Clean and readable.
54-133: LGTM!Public API is well-designed with consistent null-safety checks, clear Javadocs, and good delegation to reduce duplication.
135-144: LGTM!The update flow is well-structured: collect visible objects, compute position mappings, then apply updates atomically.
146-151: LGTM!The two-step update (remove old keys, add new positions) ensures atomic updates without ConcurrentModificationException risks.
161-167: LGTM!Conflict resolution creates a fresh exclusion set per conflict, correctly handling the case per the established pattern. Based on learnings, this design is intentional.
179-188: LGTM!The reassignment logic correctly handles edge cases: adds to exclusion set, checks both null and tolerance, and treats orphaned updates as new objects.
201-225: LGTM!Helper methods are well-factored with single responsibilities.
getClosestObjectFromSetToPositionproperly handles empty sets.
227-233: LGTM!Clean use of
removeIffor pruning stale entries. The expiration check is straightforward and readable.
25-41: Translation2d is properly implemented for HashMap usage—no changes needed.WPILib's
Translation2dclass correctly overridesequals()andhashCode(). Theequals()method uses value-based comparison (x and y coordinates with 1e-9 epsilon), andhashCode()usesObjects.hash(). HashMap operations likeremove()on line 114 will work correctly. The tolerance-based matching logic in the code is intentional design, not a workaround for missing equality semantics.
.../java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
Show resolved
Hide resolved
...ava/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCameraIO.java
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java (1)
87-100: Return null instead of origin when pose is unavailable, and filter invalid results in the caller.When
robotPoseAtResultTimestampis null, returningnew Translation2d()(origin) creates a false detection at field position (0,0). This gets tracked as a real object inObjectPoseEstimator, introducing spurious data into the tracking system. Instead, return null and filter out invalid positions ingetObjectPositionsOnFieldbefore returning the array, or skip adding null results to avoid logging phantom detections.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java
🧠 Learnings (6)
📓 Common learnings
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:12.465Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
📚 Learning: 2026-01-12T16:16:07.671Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:07.671Z
Learning: In ObjectPoseEstimator.updateObjectsPositions() (src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java), keep the call to excludedKnownObjects.clear() inside the loop so that each visible camera object uses a fresh exclusion list when updateObjectPosition() is called. Moving the clear() call outside the loop can cause stale exclusions and mis-handle discarded object updates. This pattern is specific to this file and should be preserved to ensure correct per-object exclusion state. Verify with tests that object updates are processed correctly per object and that there are no unintended cross-object exclusions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2025-12-30T00:12:14.902Z
Learnt from: levyishai
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java:161-187
Timestamp: 2025-12-30T00:12:14.902Z
Learning: In the Programming-TRIGON/RobotTemplate Java codebase, follow Uncle Bob's Clean Code principles: prefer self-documenting code through descriptive naming and small, focused methods (method extraction) over adding explanatory comments. Ensure classes, methods, and variables convey intent (naming should be expressive and consistent). Use comments only to explain non-obvious decisions or workarounds, not as a substitute for readable code.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java
📚 Learning: 2026-01-11T12:11:54.447Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:30-37
Timestamp: 2026-01-11T12:11:54.447Z
Learning: In the RobotTemplate codebase (Java/FRC), prefer silent failure (no-op) over throwing exceptions for certain validation scenarios. Specifically, for constructors or methods dealing with varargs (e.g., cameras), do not throw on invalid input such as zero elements; instead, perform a no-op or safe default if zero cameras are provided. This pattern should be applied to Java files under the frc/trigon/robot area (e.g., src/main/java/frc/trigon/robot/**/*.java). Document the rationale for silent failures where used, and ensure this behavior is clearly tested and reviewed to avoid hidden issues or regressions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java
📚 Learning: 2025-12-30T20:34:50.231Z
Learnt from: Strflightmight09
Repo: Programming-TRIGON/RobotTemplate PR: 48
File: src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java:180-183
Timestamp: 2025-12-30T20:34:50.231Z
Learning: In template/ scaffolding repositories (e.g., RobotTemplate), allow and preserve optional code patterns that are commented out for users to enable, rather than removing or fully implementing them. During reviews, treat commented-out blocks as intentional placeholders to be potentially enabled by users; avoid enforcing removal of such code and ensure commented sections are clearly documented as optional templates.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.javasrc/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java
📚 Learning: 2026-01-12T16:16:12.465Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:12.465Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java
🧬 Code graph analysis (2)
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java (2)
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.java (1)
PhotonObjectDetectionCameraIO(16-85)src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java (1)
SimulationObjectDetectionCameraIO(17-159)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java (1)
src/main/java/frc/trigon/robot/misc/simulatedfield/SimulatedGamePieceConstants.java (1)
SimulatedGamePieceConstants(10-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java (1)
1-8: LGTM!Clean constants class with appropriate visibility levels. Package restructuring is consistent with the broader refactor.
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java (5)
161-167: LGTM - exclusion set handling is correct.Creating a fresh
HashSetat line 163 ensures each conflicting update gets independent exclusion tracking, which aligns with the documented design intent for handling discarded object updates.
211-225: LGTM!Clean implementation of closest-object search with proper empty-set handling.
66-71: LGTM!Null-safe removal pattern is applied consistently across all removal methods.
113-115: All callers correctly use tracked positions; no changes needed.
removeObject()requires an exact position match since it uses a HashMap key. However, all three internal callers (removeClosestObjectToRobot,removeClosestObjectToIntake,removeClosestObjectToPosition) properly retrieve the position viagetClosestTrackedObjectToPosition(), which returns keys from the tracked objects HashMap. Translation2d uses coordinate-based equality (not reference equality), so these will match perfectly. The Javadoc already documents this requirement clearly.
34-41: Document non-null requirement for camera parameter.Line 136 will throw NPE if
camerais null. Whilecamerais never null in practice (only instantiated once in RobotContainer with a constant), the constructor documentation should explicitly state thatcameramust not be null.
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧠 Learnings (5)
📓 Common learnings
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:12.465Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
📚 Learning: 2026-01-12T16:16:07.671Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:07.671Z
Learning: In ObjectPoseEstimator.updateObjectsPositions() (src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java), keep the call to excludedKnownObjects.clear() inside the loop so that each visible camera object uses a fresh exclusion list when updateObjectPosition() is called. Moving the clear() call outside the loop can cause stale exclusions and mis-handle discarded object updates. This pattern is specific to this file and should be preserved to ensure correct per-object exclusion state. Verify with tests that object updates are processed correctly per object and that there are no unintended cross-object exclusions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2025-12-30T00:12:14.902Z
Learnt from: levyishai
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java:161-187
Timestamp: 2025-12-30T00:12:14.902Z
Learning: In the Programming-TRIGON/RobotTemplate Java codebase, follow Uncle Bob's Clean Code principles: prefer self-documenting code through descriptive naming and small, focused methods (method extraction) over adding explanatory comments. Ensure classes, methods, and variables convey intent (naming should be expressive and consistent). Use comments only to explain non-obvious decisions or workarounds, not as a substitute for readable code.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2026-01-11T12:11:54.447Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:30-37
Timestamp: 2026-01-11T12:11:54.447Z
Learning: In the RobotTemplate codebase (Java/FRC), prefer silent failure (no-op) over throwing exceptions for certain validation scenarios. Specifically, for constructors or methods dealing with varargs (e.g., cameras), do not throw on invalid input such as zero elements; instead, perform a no-op or safe default if zero cameras are provided. This pattern should be applied to Java files under the frc/trigon/robot area (e.g., src/main/java/frc/trigon/robot/**/*.java). Document the rationale for silent failures where used, and ensure this behavior is clearly tested and reviewed to avoid hidden issues or regressions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2025-12-30T20:34:50.231Z
Learnt from: Strflightmight09
Repo: Programming-TRIGON/RobotTemplate PR: 48
File: src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java:180-183
Timestamp: 2025-12-30T20:34:50.231Z
Learning: In template/ scaffolding repositories (e.g., RobotTemplate), allow and preserve optional code patterns that are commented out for users to enable, rather than removing or fully implementing them. During reviews, treat commented-out blocks as intentional placeholders to be potentially enabled by users; avoid enforcing removal of such code and ensure commented sections are clearly documented as optional templates.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🔇 Additional comments (7)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java (7)
18-41: LGTM - Clean class structure and constructor.Fields are appropriately scoped as final, and the constructor properly initializes state. The use of
Translation2das a HashMap key is valid since WPILib implements properequals/hashCodefor this class.
47-52: LGTM - Clean periodic implementation.Good separation between updating, pruning, and logging. The method name clearly describes what each step does.
59-61: Good defensive copy pattern.Returning
new ArrayList<>(keySet())prevents external code from modifying internal state.
66-71: LGTM - Early return pattern.Clean null handling with early return before attempting removal.
152-161: Verify recursion depth is acceptable for your object counts.This recursive approach handles displaced updates elegantly, but recursion depth equals the number of updates that "cascade" when a visible object claims a tracked position. For typical FRC game piece counts (< 20), this is fine. If object counts grow significantly, consider iterative refactoring.
196-210: LGTM - Clean closest-object search implementation.The
closestObjectDistanceis properly initialized before the loop (matching the commit message optimization), and the null check for empty set is good defensive programming.
212-218: LGTM - Correct expiration logic.Clean use of
removeIfand the expiration check is straightforward.
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java (2)
94-97: Potential division by zero when pitch is near zero.If
objectRotationStart.getRotation().getY()is 0 (object at horizon level),objectPitchSinbecomes 0, causingxTransformto be Infinity/NaN. While objects on the ground typically have negative pitch, sensor noise or edge cases could produce near-zero values.Consider adding a guard:
if (Math.abs(objectPitchSin) < 1e-6) { return new Translation2d(); // or handle as invalid detection }
88-90: Returning (0,0) on null pose may mask errors.When
robotPoseAtResultTimestampis null, returningnew Translation2d()(origin) could be indistinguishable from a valid detection at field origin. Callers ingetObjectPositionsOnFieldwould include this spurious position in the array.Consider returning
nullhere and filtering nulls in the caller, or logging a warning. Based on learnings, if silent failure is preferred, document this behavior.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
🧠 Learnings (6)
📓 Common learnings
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:47-52
Timestamp: 2026-01-13T18:53:18.558Z
Learning: In `ObjectPoseEstimator` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, only a single instance is created in the codebase (not multiple instances for different game piece types), so hardcoded log keys like `"ObjectPoseEstimator/knownObjectPositions"` do not cause collisions.
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:12.465Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
📚 Learning: 2026-01-12T16:16:12.465Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:12.465Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
📚 Learning: 2026-01-13T18:53:18.558Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:47-52
Timestamp: 2026-01-13T18:53:18.558Z
Learning: In `ObjectPoseEstimator` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, only a single instance is created in the codebase (not multiple instances for different game piece types), so hardcoded log keys like `"ObjectPoseEstimator/knownObjectPositions"` do not cause collisions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
📚 Learning: 2025-12-30T00:12:14.902Z
Learnt from: levyishai
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java:161-187
Timestamp: 2025-12-30T00:12:14.902Z
Learning: In the Programming-TRIGON/RobotTemplate Java codebase, follow Uncle Bob's Clean Code principles: prefer self-documenting code through descriptive naming and small, focused methods (method extraction) over adding explanatory comments. Ensure classes, methods, and variables convey intent (naming should be expressive and consistent). Use comments only to explain non-obvious decisions or workarounds, not as a substitute for readable code.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
📚 Learning: 2025-12-30T20:34:50.231Z
Learnt from: Strflightmight09
Repo: Programming-TRIGON/RobotTemplate PR: 48
File: src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java:180-183
Timestamp: 2025-12-30T20:34:50.231Z
Learning: In template/ scaffolding repositories (e.g., RobotTemplate), allow and preserve optional code patterns that are commented out for users to enable, rather than removing or fully implementing them. During reviews, treat commented-out blocks as intentional placeholders to be potentially enabled by users; avoid enforcing removal of such code and ensure commented sections are clearly documented as optional templates.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
📚 Learning: 2026-01-11T12:11:54.447Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:30-37
Timestamp: 2026-01-11T12:11:54.447Z
Learning: In the RobotTemplate codebase (Java/FRC), prefer silent failure (no-op) over throwing exceptions for certain validation scenarios. Specifically, for constructors or methods dealing with varargs (e.g., cameras), do not throw on invalid input such as zero elements; instead, perform a no-op or safe default if zero cameras are provided. This pattern should be applied to Java files under the frc/trigon/robot area (e.g., src/main/java/frc/trigon/robot/**/*.java). Document the rationale for silent failures where used, and ensure this behavior is clearly tested and reviewed to avoid hidden issues or regressions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
🧬 Code graph analysis (1)
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java (3)
src/main/java/frc/trigon/robot/RobotContainer.java (1)
RobotContainer(26-86)src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/io/PhotonObjectDetectionCameraIO.java (1)
PhotonObjectDetectionCameraIO(16-85)src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/io/SimulationObjectDetectionCameraIO.java (1)
SimulationObjectDetectionCameraIO(17-159)
🔇 Additional comments (2)
src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java (2)
59-76: LGTM!Clean, focused accessor methods with clear naming. The rename from
getTargetObjectsRotationstogetObjectsRotationsaligns well with the API simplification.
102-108: LGTM!Clean factory pattern for IO selection based on robot state. The ordering (replay → simulation → real) is logical.
.../java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧠 Learnings (6)
📓 Common learnings
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:47-52
Timestamp: 2026-01-13T18:53:18.558Z
Learning: In `ObjectPoseEstimator` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, only a single instance is created in the codebase (not multiple instances for different game piece types), so hardcoded log keys like `"ObjectPoseEstimator/knownObjectPositions"` do not cause collisions.
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:12.465Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
📚 Learning: 2026-01-13T18:53:18.558Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:47-52
Timestamp: 2026-01-13T18:53:18.558Z
Learning: In ObjectPoseEstimator.java, only a single instance is instantiated in the codebase, so hardcoded log keys such as "ObjectPoseEstimator/knownObjectPositions" are unlikely to collide across multiple instances or game piece types. During reviews, verify that there really is a single instance; if future changes introduce multiple instances, switch to instance-scoped or contextual log keys to avoid collisions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2026-01-12T16:16:07.671Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:07.671Z
Learning: In ObjectPoseEstimator.updateObjectsPositions() (src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java), keep the call to excludedKnownObjects.clear() inside the loop so that each visible camera object uses a fresh exclusion list when updateObjectPosition() is called. Moving the clear() call outside the loop can cause stale exclusions and mis-handle discarded object updates. This pattern is specific to this file and should be preserved to ensure correct per-object exclusion state. Verify with tests that object updates are processed correctly per object and that there are no unintended cross-object exclusions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2025-12-30T00:12:14.902Z
Learnt from: levyishai
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java:161-187
Timestamp: 2025-12-30T00:12:14.902Z
Learning: In the Programming-TRIGON/RobotTemplate Java codebase, follow Uncle Bob's Clean Code principles: prefer self-documenting code through descriptive naming and small, focused methods (method extraction) over adding explanatory comments. Ensure classes, methods, and variables convey intent (naming should be expressive and consistent). Use comments only to explain non-obvious decisions or workarounds, not as a substitute for readable code.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2026-01-11T12:11:54.447Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:30-37
Timestamp: 2026-01-11T12:11:54.447Z
Learning: In the RobotTemplate codebase (Java/FRC), prefer silent failure (no-op) over throwing exceptions for certain validation scenarios. Specifically, for constructors or methods dealing with varargs (e.g., cameras), do not throw on invalid input such as zero elements; instead, perform a no-op or safe default if zero cameras are provided. This pattern should be applied to Java files under the frc/trigon/robot area (e.g., src/main/java/frc/trigon/robot/**/*.java). Document the rationale for silent failures where used, and ensure this behavior is clearly tested and reviewed to avoid hidden issues or regressions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2025-12-30T20:34:50.231Z
Learnt from: Strflightmight09
Repo: Programming-TRIGON/RobotTemplate PR: 48
File: src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java:180-183
Timestamp: 2025-12-30T20:34:50.231Z
Learning: In template/ scaffolding repositories (e.g., RobotTemplate), allow and preserve optional code patterns that are commented out for users to enable, rather than removing or fully implementing them. During reviews, treat commented-out blocks as intentional placeholders to be potentially enabled by users; avoid enforcing removal of such code and ensure commented sections are clearly documented as optional templates.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🔇 Additional comments (6)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java (6)
18-25: LGTM!Class structure and fields are well-organized. The
HashMap<Translation2d, Double>appropriately maps object positions to their detection timestamps.
34-41: LGTM!Clean constructor with appropriate field initialization. Documentation is clear.
47-52: LGTM!The periodic method is well-structured with clear responsibilities. Based on learnings, the hardcoded log key is acceptable since only a single instance is created.
59-133: LGTM!Public API is clean with consistent null-safety checks. Methods are small and focused per clean code principles.
getObjectsOnField()correctly returns a defensive copy.
135-150: LGTM!The update coordination logic is sound. The map correctly distinguishes between updates to existing objects (key ≠ value) and new objects (key = value).
164-221: LGTM!Helper methods are well-factored. The reference equality check on line 183 is correct since
getClosestUpdateToObjectreturns one of its input references. Edge cases are properly handled with null returns.
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/main/java/frc/trigon/robot/Robot.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/robot/Robot.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧠 Learnings (6)
📓 Common learnings
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:47-52
Timestamp: 2026-01-13T18:53:18.558Z
Learning: In `ObjectPoseEstimator` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, only a single instance is created in the codebase (not multiple instances for different game piece types), so hardcoded log keys like `"ObjectPoseEstimator/knownObjectPositions"` do not cause collisions.
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:12.465Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
📚 Learning: 2025-12-30T00:12:14.902Z
Learnt from: levyishai
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/poseestimation/poseestimator/ObjectPoseEstimator.java:161-187
Timestamp: 2025-12-30T00:12:14.902Z
Learning: In the Programming-TRIGON/RobotTemplate Java codebase, follow Uncle Bob's Clean Code principles: prefer self-documenting code through descriptive naming and small, focused methods (method extraction) over adding explanatory comments. Ensure classes, methods, and variables convey intent (naming should be expressive and consistent). Use comments only to explain non-obvious decisions or workarounds, not as a substitute for readable code.
Applied to files:
src/main/java/frc/trigon/robot/Robot.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2025-12-30T20:34:50.231Z
Learnt from: Strflightmight09
Repo: Programming-TRIGON/RobotTemplate PR: 48
File: src/main/java/frc/trigon/robot/subsystems/swerve/Swerve.java:180-183
Timestamp: 2025-12-30T20:34:50.231Z
Learning: In template/ scaffolding repositories (e.g., RobotTemplate), allow and preserve optional code patterns that are commented out for users to enable, rather than removing or fully implementing them. During reviews, treat commented-out blocks as intentional placeholders to be potentially enabled by users; avoid enforcing removal of such code and ensure commented sections are clearly documented as optional templates.
Applied to files:
src/main/java/frc/trigon/robot/Robot.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2026-01-11T12:11:54.447Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:30-37
Timestamp: 2026-01-11T12:11:54.447Z
Learning: In the RobotTemplate codebase (Java/FRC), prefer silent failure (no-op) over throwing exceptions for certain validation scenarios. Specifically, for constructors or methods dealing with varargs (e.g., cameras), do not throw on invalid input such as zero elements; instead, perform a no-op or safe default if zero cameras are provided. This pattern should be applied to Java files under the frc/trigon/robot area (e.g., src/main/java/frc/trigon/robot/**/*.java). Document the rationale for silent failures where used, and ensure this behavior is clearly tested and reviewed to avoid hidden issues or regressions.
Applied to files:
src/main/java/frc/trigon/robot/Robot.javasrc/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2026-01-13T18:53:18.558Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:47-52
Timestamp: 2026-01-13T18:53:18.558Z
Learning: In ObjectPoseEstimator.java, only a single instance is instantiated in the codebase, so hardcoded log keys such as "ObjectPoseEstimator/knownObjectPositions" are unlikely to collide across multiple instances or game piece types. During reviews, verify that there really is a single instance; if future changes introduce multiple instances, switch to instance-scoped or contextual log keys to avoid collisions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
📚 Learning: 2026-01-12T16:16:07.671Z
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:07.671Z
Learning: In ObjectPoseEstimator.updateObjectsPositions() (src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java), keep the call to excludedKnownObjects.clear() inside the loop so that each visible camera object uses a fresh exclusion list when updateObjectPosition() is called. Moving the clear() call outside the loop can cause stale exclusions and mis-handle discarded object updates. This pattern is specific to this file and should be preserved to ensure correct per-object exclusion state. Verify with tests that object updates are processed correctly per object and that there are no unintended cross-object exclusions.
Applied to files:
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
🧬 Code graph analysis (2)
src/main/java/frc/trigon/robot/Robot.java (1)
src/main/java/frc/trigon/robot/misc/simulatedfield/SimulationFieldHandler.java (1)
SimulationFieldHandler(15-159)
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java (4)
src/main/java/frc/trigon/robot/RobotContainer.java (1)
RobotContainer(26-86)src/main/java/frc/trigon/robot/misc/objectDetection/objectdetectioncamera/ObjectDetectionCamera.java (1)
ObjectDetectionCamera(15-109)src/main/java/frc/trigon/robot/misc/simulatedfield/SimulatedGamePieceConstants.java (1)
SimulatedGamePieceConstants(10-47)src/main/java/frc/trigon/robot/misc/objectDetection/ObjectDetectionConstants.java (1)
ObjectDetectionConstants(5-8)
🔇 Additional comments (8)
src/main/java/frc/trigon/robot/Robot.java (1)
14-14: LGTM!Clean integration.
simulationPeriodic()is the correct lifecycle hook for simulation-specific updates, andSimulationFieldHandler.update()appropriately handles game piece state management and logging during simulation.Also applies to: 62-64
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java (7)
18-25: LGTM!Class structure and field declarations are clean. Using
Translation2das HashMap keys works correctly here since object references are maintained through the update cycle. The Javadoc clearly explains the map's purpose.
34-41: LGTM!Constructor is clean and properly initializes all fields. Per the codebase conventions, no exception is thrown for potentially null inputs.
47-61: LGTM!The periodic flow is clean: update → prune → log.
getObjectsOnField()correctly returns a defensive copy. Based on learnings, the hardcoded log key is acceptable since only a single instance exists.
63-115: LGTM!All removal methods properly handle null cases before calling
removeObject(). The Javadoc onremoveObject()correctly warns callers about the precision requirement for the position parameter.
117-133: LGTM!Simple, self-documenting methods. Clean delegation pattern.
135-150: LGTM!Clean batch update pattern: collect all updates first, then apply atomically. Using a single timestamp for all updates in the same cycle is correct.
193-219: LGTM!Helper methods are well-structured.
getClosestObjectFromSetToPositionhandles the empty set case correctly.removeOldObjectsuses idiomaticremoveIffor safe iteration-with-removal.
src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/main/java/frc/trigon/lib
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/lib
🧠 Learnings (1)
📓 Common learnings
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:47-52
Timestamp: 2026-01-13T18:53:18.558Z
Learning: In `ObjectPoseEstimator` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, only a single instance is created in the codebase (not multiple instances for different game piece types), so hardcoded log keys like `"ObjectPoseEstimator/knownObjectPositions"` do not cause collisions.
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:12.465Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint Code Base
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/main/java/frc/trigon/lib
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: ## Review Style
- Explain issues clearly, but don't over-explain. Don't write anything unnecessary or repetetive
- Suggest a solution when helpful
- No code blocks unless they genuinely add clarity - GitHub already shows the diff
- Don't describe what the code does - focus on what could be improved
Naming & Readability
- Variable, function, and class names must be descriptive and intention-revealing
- No abbreviations unless universally understood (e.g., id, url)
- Boolean names should read as questions (isValid, hasPermission, canExecute)
- Functions should be verbs, classes should be nouns
- Magic numbers and strings should be named constants
- Code should be self-documenting; comments should explain "why", not "what"
Clean Code Principles
- Functions should do one thing and be small
- Too many parameters? Suggest grouping into an object
- Flag deep nesting - suggest early returns or extraction
- DRY: flag duplicated logic
- Functions with non-obvious side effects
- Single responsibility for classes and modules
- Proper abstraction levels - no mixing high and low level logic
Logic & Correctness
- Off-by-one errors, null/undefined handling, race conditions
- Unhandled edge cases
- Incorrect boolean logic or conditions
- Resource leaks, missing cleanup
- Error handling gaps
- Performance issues when significant
Code Structure
- File and folder organization
- Proper separation of concerns
- Dependency management and coupling
Files:
src/main/java/frc/trigon/lib
🧠 Learnings (1)
📓 Common learnings
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:47-52
Timestamp: 2026-01-13T18:53:18.558Z
Learning: In `ObjectPoseEstimator` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, only a single instance is created in the codebase (not multiple instances for different game piece types), so hardcoded log keys like `"ObjectPoseEstimator/knownObjectPositions"` do not cause collisions.
Learnt from: ShmayaR
Repo: Programming-TRIGON/RobotTemplate PR: 45
File: src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java:138-142
Timestamp: 2026-01-12T16:16:12.465Z
Learning: In `ObjectPoseEstimator.updateObjectsPositions()` in `src/main/java/frc/trigon/robot/misc/objectDetection/ObjectPoseEstimator.java`, the `excludedKnownObjects.clear()` call is intentionally placed inside the loop so that each visible object from the camera gets a fresh exclusion list when calling `updateObjectPosition()`. This is required for handling discarded object updates correctly, and should not be moved outside the loop.
| @@ -1 +1 @@ | |||
| Subproject commit 7129d95439838a599875a4f6c53159408a5fdb57 | |||
| Subproject commit 9403200d5d1161a2c75be560251618af01230b5a | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, check the git status and submodule configuration
echo "=== Checking git configuration ==="
git config --file .gitmodules --get-regexp path
git ls-files -s | grep -E "src/main/java/frc/trigon/lib"
echo -e "\n=== Checking submodule status ==="
git submodule status
echo -e "\n=== Checking if submodule directory exists ==="
ls -la src/main/java/frc/trigon/lib 2>/dev/null | head -20 || echo "Submodule directory not initialized"Repository: Programming-TRIGON/RobotTemplate
Length of output: 523
🏁 Script executed:
# Check the PR/commit that contains this change
echo "=== Checking recent commits affecting the submodule ==="
git log -p --follow -n 5 -- src/main/java/frc/trigon/lib 2>/dev/null | head -100 || echo "No git history available"
echo -e "\n=== Checking git diff for the current change ==="
git diff HEAD~1 HEAD -- src/main/java/frc/trigon/lib 2>/dev/null || echo "Cannot get diff"Repository: Programming-TRIGON/RobotTemplate
Length of output: 197
🏁 Script executed:
# Search the main codebase for references to classes mentioned in the summary
echo "=== Searching for PoseEstimator usage ==="
rg -l "PoseEstimator|RobotPoseEstimator|ObjectPoseEstimator" --type java 2>/dev/null | head -20
echo -e "\n=== Searching for imports from the lib submodule ==="
rg "import.*frc\.trigon\.lib" --type java 2>/dev/null | head -20Repository: Programming-TRIGON/RobotTemplate
Length of output: 3345
Add commit message context explaining the submodule update.
The commit message "Update lib" doesn't explain what was changed or why. Given that the main codebase depends on this library across hardware abstractions (Phoenix6, Pigeon2), utilities (Flippable types, QuickSortHandler), and pose estimation, use a more descriptive message: list the key changes (e.g., refactoring names, new features, dependency updates) and any migration steps required.
No description provided.