-
Notifications
You must be signed in to change notification settings - Fork 0
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
Is this it? #38
Is this it? #38
Conversation
Does not update pose unless a matched object cant be found, updates all other stats.
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces comprehensive changes to the project's configuration and vision subsystems. The modifications primarily focus on enhancing Git Large File Storage (LFS) handling for Photonvision configuration files, updating camera calibration configurations, and improving vision-related subsystems. The changes include updates to Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)build.gradle (1)
The exclusion of Let's verify the existence and contents of the ✅ Verification successfulExclusion rule for PhotonVision files is correctly configured The verification confirms:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify the .photonvision directory structure and content types
# Check if .photonvision directory exists and list its structure
if [ -d ".photonvision" ]; then
echo "Directory structure:"
find .photonvision -type f -exec file {} \;
# Check if these files are tracked by Git LFS
echo -e "\nGit LFS tracking status:"
git lfs ls-files | grep photonvision
else
echo ".photonvision directory not found"
fi
Length of output: 475 Script: #!/bin/bash
# Let's verify if the .photonvision directory exists and check Git configuration for LFS tracking
# Check if .photonvision directory exists
echo "Directory check:"
ls -la .photonvision 2>/dev/null || echo "Directory not found"
# Check .gitattributes for LFS configuration
echo -e "\nGit LFS configuration (.gitattributes):"
cat .gitattributes 2>/dev/null | grep -i "photonvision" || echo "No PhotonVision LFS configuration found"
# Check if there are any files with .photonvision pattern
echo -e "\nFiles matching .photonvision pattern:"
find . -name ".photonvision*" -o -path "*/.photonvision/*" 2>/dev/null || echo "No matching files found"
Length of output: 1096 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Nitpick comments (4)
src/main/java/frc/alotobots/library/subsystems/vision/photonvision/objectdetection/ObjectDetectionSubsystem.java (2)
397-399
: Revisit the quadratic scaling for large distances.
While dynamically scaling tolerance is beneficial, the 0.04 coefficient and a base offset of 0.1 might become excessively large at higher distances. Consider bounding or reviewing how big this tolerance can become to avoid false positives.
183-356
: Consider concurrency aspects.
If there's a chance that sensor inputs orrobotPose
can be accessed from multiple threads, consider synchronization or concurrency-safe data structures to avoid race conditions in detectionHistories, stableObjects, and pendingObjects sets.src/main/java/frc/alotobots/library/subsystems/vision/photonvision/objectdetection/constants/ObjectDetectionConstants.java (1)
67-68
: Make SCALE_FACTOR configurable if necessary.
While 1.15 is a plausible default, real-world conditions might require quick adjustments. Consider making this value configurable if fine-tuning in the field is needed..photonvision/config/camera/FM_ObjectDetection/photon_calibration_FM_ObjectDetection_1280x720.json (1)
Line range hint
1-4
: Consider adding documentation about calibration parametersSince this is a camera calibration file, it would be helpful to add documentation (e.g., in a README.md) explaining:
- The calibration process used
- Key parameters and their expected values
- Camera model and settings
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-2: End of file expected
Use an array for a sequence of values:
[1, 2]
(parse)
[error] 2-2: End of file expected
Use an array for a sequence of values:
[1, 2]
(parse)
[error] 2-2: String values must be double quoted.
(parse)
[error] 2-3: String values must be double quoted.
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values:
[1, 2]
(parse)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.photonvision/config/12-22-24-Full.zip
is excluded by!**/*.zip
📒 Files selected for processing (11)
.gitattributes
(1 hunks).gitignore
(1 hunks).photonvision/config/camera/FL_AprilTag/photon_calibration_FL_AprilTag_1280x720.json
(1 hunks).photonvision/config/camera/FM_AprilTag/photon_calibration_FM_AprilTag_1280x720.json
(1 hunks).photonvision/config/camera/FM_ObjectDetection/config.json
(0 hunks).photonvision/config/camera/FM_ObjectDetection/photon_calibration_FM_ObjectDetection_1280x720.json
(1 hunks)src/main/java/frc/alotobots/library/subsystems/swervedrive/SwerveDriveSubsystem.java
(0 hunks)src/main/java/frc/alotobots/library/subsystems/vision/photonvision/apriltag/AprilTagSubsystem.java
(3 hunks)src/main/java/frc/alotobots/library/subsystems/vision/photonvision/objectdetection/ObjectDetectionSubsystem.java
(4 hunks)src/main/java/frc/alotobots/library/subsystems/vision/photonvision/objectdetection/constants/ObjectDetectionConstants.java
(2 hunks)vendordeps/photonlib.json
(5 hunks)
💤 Files with no reviewable changes (2)
- .photonvision/config/camera/FM_ObjectDetection/config.json
- src/main/java/frc/alotobots/library/subsystems/swervedrive/SwerveDriveSubsystem.java
✅ Files skipped from review due to trivial changes (2)
- .gitattributes
- vendordeps/photonlib.json
🧰 Additional context used
🪛 Biome (1.9.4)
.photonvision/config/camera/FL_AprilTag/photon_calibration_FL_AprilTag_1280x720.json
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
.photonvision/config/camera/FM_AprilTag/photon_calibration_FM_AprilTag_1280x720.json
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
.photonvision/config/camera/FM_ObjectDetection/photon_calibration_FM_ObjectDetection_1280x720.json
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-2: String values must be double quoted.
(parse)
[error] 2-3: String values must be double quoted.
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (11)
.photonvision/config/camera/FM_AprilTag/photon_calibration_FM_AprilTag_1280x720.json (2)
1-3
: LGTM! Git LFS pointer format is correct.
The pointer file follows the Git LFS specification with proper version, SHA-256 hash, and size declaration.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
1-3
: Verify camera configuration resolution.
Please ensure that the actual camera configuration in the binary file matches the 1280x720 resolution specified in the filename.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
src/main/java/frc/alotobots/library/subsystems/vision/photonvision/objectdetection/ObjectDetectionSubsystem.java (3)
20-20
: No issues with the new import.
The import for Transform3d aligns with its usage in the code.
183-189
: Excellent approach to logging camera inputs.
Capturing inputs in the logger at this stage provides valuable visibility for debugging.
436-443
: Verify correctness of the SCALE_FACTOR usage.
You multiply the full Transform3d translation by SCALE_FACTOR. Ensure that the real-world measurements (e.g., camera XY offsets, measurement units) remain accurate and consistent after scaling, especially since the camera offset is also changed in constants.
✅ Verification successful
The previous command had syntax issues. Let's gather information about the scaling factor and related transformations to verify the correctness of its usage. We'll look for its definition, usage patterns, and related camera constants.
Based on the search results, I can now provide a final response about the SCALE_FACTOR usage:
The SCALE_FACTOR value of 1.15 is correctly applied to distance measurements.
The scaling is applied consistently in the transformation pipeline:
- The SCALE_FACTOR is defined as a constant in ObjectDetectionConstants.java with a clear comment indicating its purpose: "Scale Factor. Tune this if the ObjectDetection is consistently under/over reporting"
- It's applied only to the translation component of the transform while preserving the rotation, which is the correct approach for adjusting distance measurements
- The camera offsets in ObjectDetectionConstants are defined separately and are not affected by this scaling, maintaining their accuracy
The implementation maintains measurement consistency by:
- Only scaling the target-to-robot translation vector
- Preserving the rotation component of the transform
- Using the scaled transform for subsequent field-space calculations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for references to `toFieldRelative` usage to confirm consistent usage or overrides.
rg -A 5 $'toFieldRelative('
Length of output: 111
Script:
#!/bin/bash
# Let's search for SCALE_FACTOR usage and its definition
rg "SCALE_FACTOR" -A 3
# Also search for Transform3d usage patterns
rg "Transform3d.*getTranslation\(\)\.times\(" -A 2
# Let's also check the constants file for camera-related constants
fd -e java constants -x cat {}
Length of output: 27219
src/main/java/frc/alotobots/library/subsystems/vision/photonvision/objectdetection/constants/ObjectDetectionConstants.java (1)
30-30
: New camera offset might need re-verification.
Changing the forward offset from 0.275 to 0.25 reduces the camera’s forward placement. Ensure that related geometry (e.g., rotational offsets or mechanical mounting) is also updated if needed.
.gitignore (1)
190-191
: LGTM! Verify the Git LFS setup for PhotonVision configs.
The exception for PhotonVision .zip files is correctly configured. This change aligns with the Git LFS setup mentioned in the AI summary.
Let's verify the Git LFS configuration is properly set up:
✅ Verification successful
Git LFS is configured but not installed in the environment
The repository has proper Git LFS configuration in .gitattributes
for PhotonVision files, but Git LFS is not installed in the current environment. This is expected in the sandbox environment and doesn't affect the correctness of the configuration:
.gitattributes
correctly tracks.photonvision/**/*
with Git LFS- The
.gitignore
exception for.photonvision/**/*.zip
aligns with this setup - A PhotonVision config zip file exists at
.photonvision/config/12-22-24-Full.zip
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Git LFS configuration for PhotonVision files
# Check if Git LFS is tracking .photonvision files
git lfs track | grep photonvision
# List any existing .zip files in .photonvision directory
fd -e zip . .photonvision/
Length of output: 188
Script:
#!/bin/bash
# Check if there's any Git LFS configuration in the repository
cat .gitattributes 2>/dev/null || echo "No .gitattributes file found"
# Check if there are any other .zip files in the repository for comparison
fd -e zip
Length of output: 87
src/main/java/frc/alotobots/library/subsystems/vision/photonvision/apriltag/AprilTagSubsystem.java (1)
104-104
: LGTM! Improved log organization for AprilTag subsystem.
The logging path updates properly categorize AprilTag-specific logs under "Vision/AprilTag/", making the logging structure more organized and easier to navigate.
Also applies to: 181-181, 184-184, 187-187, 190-190, 200-200, 202-203, 205-205, 208-208
.photonvision/config/camera/FL_AprilTag/photon_calibration_FL_AprilTag_1280x720.json (1)
1-3
: Verify the camera calibration file integrity.
The Git LFS pointer is properly formatted. Given the importance of camera calibration for AprilTag detection, let's verify the file integrity.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
.photonvision/config/camera/FM_ObjectDetection/photon_calibration_FM_ObjectDetection_1280x720.json (2)
Line range hint 1-4
: Git LFS pointer format looks good!
The pointer file correctly follows the Git LFS specification with all required fields.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-2: String values must be double quoted.
(parse)
[error] 2-3: String values must be double quoted.
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
Line range hint 1-4
: Verify the calibration file content
Let's verify that the actual LFS object contains valid PhotonVision calibration data.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-2: String values must be double quoted.
(parse)
[error] 2-3: String values must be double quoted.
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
.gitattributes
and.gitignore
to better manage file storage and exceptions.Chores