Skip to content

add vision pose ambiguity scalar#130

Open
tortillini-tokki wants to merge 5 commits into
mainfrom
tori/vision-speed-check
Open

add vision pose ambiguity scalar#130
tortillini-tokki wants to merge 5 commits into
mainfrom
tori/vision-speed-check

Conversation

@tortillini-tokki

Copy link
Copy Markdown
Contributor

add vision pose ambiguity scalar for front cameras when we're moving fast

- change robot RotationSupplier to SwerveDriveState for access to heading + speed
- add isFront boolean, sets true for front cameras
- multiply stdDev matrices by dynamic matrixScalar dependent on robot speed :)
@augmentcode

augmentcode Bot commented Mar 29, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR adjusts vision measurement confidence based on robot speed to reduce trust in front-camera pose estimates while driving quickly.

Changes:

  • Added new tuning constants for a max vision velocity threshold and a std-dev scaling factor.
  • Updated VisionSubsystem/LocalizationCamera APIs to accept a SwerveDriveState supplier (instead of heading-only), enabling access to chassis speeds.
  • Applied an additional std-dev scalar for “front” cameras when linear speed exceeds the configured threshold.
  • Updated unit tests to construct a mock SwerveDriveState for the new camera constructor signature.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

m_robotSwerveStateSupplier = robotSwerveStateSupplier;

// boolean that stores camera location
if (m_cameraName.indexOf("front") == -1) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LocalizationCamera.java:64 — Determining m_isFront by substring-matching the camera name ("front") is brittle and can silently disable this behavior for robots whose front camera names don’t follow that convention (e.g., CAMERA*_NAME values like "camera3"). Consider making the “front vs back” classification explicit via constants/config rather than inferred from the name.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldnt be a problem bc we have a naming convention!

}

// if camera is in front and robot is above max velocity, update matrixScalar
double linearRobotSpeed = Math.hypot(m_robotSwerveStateSupplier.get().Speeds.vxMetersPerSecond,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LocalizationCamera.java:304 — This assumes m_robotSwerveStateSupplier.get() always returns a fully-populated SwerveDriveState with non-null Pose/Speeds; if either is null (or changes between calls) this can throw or compute speed/heading inconsistently. It’d be safer to grab one snapshot of the state per update and ensure the fields you read are valid before using them.

Severity: medium

Other Locations
  • src/main/java/frc/robot/subsystems/vision/LocalizationCamera.java:244

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

@amzoeee amzoeee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@amzoeee amzoeee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments in slack re: accel and vel checks

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.

3 participants