Skip to content
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

Merged
merged 6 commits into from
Dec 22, 2024
Merged

Is this it? #38

merged 6 commits into from
Dec 22, 2024

Conversation

SeanErn
Copy link
Member

@SeanErn SeanErn commented Dec 22, 2024

Summary by CodeRabbit

  • New Features

    • Added new configuration files for camera calibration in the PhotonVision system.
    • Introduced scaling factors and adjustments to camera offsets for improved object detection accuracy.
    • Enhanced functionality for odometry and pathfinding in the SwerveDriveSubsystem.
  • Bug Fixes

    • Improved logging for object detection and AprilTag processing to enhance diagnostics.
  • Documentation

    • Updated .gitattributes and .gitignore to better manage file storage and exceptions.
  • Chores

    • Incremented version numbers for multiple dependencies in the PhotonLib library.
    • Refined formatting rules in the build process.

@SeanErn SeanErn linked an issue Dec 22, 2024 that may be closed by this pull request
Copy link

coderabbitai bot commented Dec 22, 2024

📝 Walkthrough
📝 Walkthrough

Walkthrough

This 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 .gitattributes and .gitignore, addition of new camera calibration JSON files, and significant improvements to the SwerveDrive, AprilTag, and Object Detection subsystems.

Changes

File Change Summary
.gitattributes Added LFS handling for .photonvision directory files
.gitignore Added exception for .zip files in photonvision subdirectories
.photonvision/config/camera/ Added new calibration JSON files for FL AprilTag, FM AprilTag, and FM Object Detection
SwerveDriveSubsystem.java Added system identification methods, enhanced logging, and new control methods
AprilTagSubsystem.java Updated logging paths for better categorization
ObjectDetectionSubsystem.java Improved object detection and tracking logic
ObjectDetectionConstants.java Adjusted camera offsets and added scaling factor
vendordeps/photonlib.json Updated PhotonLib dependencies to v2025.0.0-beta-6
build.gradle Updated Spotless configuration and added deployment commit task

Possibly related PRs


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 093f159 and baeb8e0.

📒 Files selected for processing (1)
  • build.gradle (1 hunks)
🔇 Additional comments (1)
build.gradle (1)

231-233: LGTM! Appropriate exclusion for PhotonVision files.

The exclusion of .photonvision/** from Spotless formatting is correct since these files are managed by Git LFS and likely contain binary or specifically formatted configuration data that shouldn't be modified by the formatter.

Let's verify the existence and contents of the .photonvision directory:

✅ Verification successful

Exclusion rule for PhotonVision files is correctly configured

The verification confirms:

  • .photonvision directory exists with configuration files and camera calibration data
  • Git LFS is properly configured in .gitattributes to handle these files
  • The directory structure shows multiple camera configurations and calibration JSON files that should indeed be excluded from code formatting
🏁 Scripts executed

The 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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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 or robotPose 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 parameters

Since 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

📥 Commits

Reviewing files that changed from the base of the PR and between 81d0ec8 and 093f159.

⛔ 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:

  1. Only scaling the target-to-robot translation vector
  2. Preserving the rotation component of the transform
  3. 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)

@SeanErn SeanErn merged commit 2d753f0 into master Dec 22, 2024
1 of 2 checks passed
@SeanErn SeanErn deleted the objectdetectionfixtest branch December 22, 2024 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update objects when they are still detected in the array
1 participant