Skip to content

leds for vision indication#101

Open
aishahnazar wants to merge 13 commits into
mainfrom
aishah/leds-for-vision-indication
Open

leds for vision indication#101
aishahnazar wants to merge 13 commits into
mainfrom
aishah/leds-for-vision-indication

Conversation

@aishahnazar
Copy link
Copy Markdown
Contributor

@aishahnazar aishahnazar commented Mar 10, 2026

currently has 4 checks:

  • areCamerasConnected
  • isVisionUpdating
  • isEstPoseJumpy

@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented Mar 10, 2026

🤖 Augment PR Summary

Summary: Adds a vision-health LED indicator and supporting health checks.

Changes:

  • Create VisionSubsystem.isVisionHealthy() combining camera connection, update freshness, target validity, and pose stability checks.
  • Add LocalizationCamera.isConnected() and an LEDSubsystem.runVisionStatus(...) command to render status on the strip.
  • Wire LEDSubsystem into RobotContainer with a default command that shows scrolling team colors when healthy and solid red when unhealthy.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

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.

LEDPattern base = LEDPattern.gradient(LEDPattern.GradientType.kDiscontinuous, Color.kYellow, Color.kBlue);
LEDPattern pattern = base.scrollAtRelativeSpeed(Percent.per(Second).of(100));

return run(() -> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unlike runPattern(...), this command doesn’t call ignoringDisable(true), so the default vision-status LEDs may stop updating while the robot is disabled (and can leave stale colors). Consider whether you want the vision-health indicator to keep running in disabled like the other LED commands.

Severity: medium

Fix This in Augment

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

return true;
}

Pose2d currentPose = readings.get(0).robotPose().estimatedPose.toPose2d();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

After periodic() sorts allValidReadings oldest-first, readings.get(0) selects the oldest reading (often from a different camera), which can make dt out-of-order and misclassify pose flicker. Consider basing this on the newest timestamped reading (and using CameraReading.timestampSeconds() for consistency).

Severity: medium

Fix This in Augment

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

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.

+1

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.

We also don't necessarily want to do this check with only one reading.. mmm idk I think this needs further thinking

});
}

public boolean areCamerasConnected() {
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.

I don't think we want to be sad if one camera is not connected. Maybe check for 2+ cameras disconnected instead; or at least make this a changeable constant.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1


public boolean isVisionUpdating() {
double currentTime = Timer.getFPGATimestamp();
return (currentTime - m_lastTimestamp) < 0.5;
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.

Please also make this magic number a constant.

return (currentTime - m_lastTimestamp) < 0.5;
}

public boolean hasValidTargets() {
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.

This is going to be false whenever we can't see any apriltags. I don't think this is a good indication of whether or not vision is overall healthy -- there are times where I think we won't be able to see apriltags.

return true;
}

Pose2d currentPose = readings.get(0).robotPose().estimatedPose.toPose2d();
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.

+1

return !allValidReadings.isEmpty();
}

public boolean isVisionPoseStable() {
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.

For this, you might want to copy paste the isPoseJumpy() stuff from last year. I'm also not sure if this is a good indication of vision health.

return true;
}

Pose2d currentPose = readings.get(0).robotPose().estimatedPose.toPose2d();
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.

We also don't necessarily want to do this check with only one reading.. mmm idk I think this needs further thinking

m_lastVisionPoseTimestamp = timestamp;

// if robot seems like it's jumping > 1 meter in < 0.1s → probably est pose is flickering too much
return !(distanceJump > 1.0 && dt < 0.1);
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.

In the future, please make these constants.

return runPattern(pattern).withName("LED/scrolling yellow-blue");
}

public Command runVisionStatus(java.util.function.BooleanSupplier visionHealthySupplier) {
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.

Create a trigger on isVisionHealthy() instead, and bind the trigger to the LED pattern instead

aishahnazar and others added 11 commits March 10, 2026 11:32
check to see if multiple cameras are disconnected
commit 13f0e79
Author: zoe z <121075323+amzoeee@users.noreply.github.com>
Date:   Tue Mar 10 11:18:08 2026 -0700

    test displace fuel + run column backwards in intake (#104)

    * update displace fuel timing and position

    * add intake command

commit 8d48f71
Author: zoe z <121075323+amzoeee@users.noreply.github.com>
Date:   Tue Mar 10 11:17:48 2026 -0700

    organize shoot commands (+ fix shootToHub) (#98)

    * fix whitespace + comments

    * use parallel group of sequences in shoottohub

    * add command names

    * fix shootWithoutDistance javadoc

    * remove snapToHub timeout?

commit 7bc05cf
Author: zoe z <121075323+amzoeee@users.noreply.github.com>
Date:   Tue Mar 10 09:22:22 2026 -0700

    add snapForBump command  (#97)

    * add snapForBump

    * remove unused import

    * use snap to angle for snap for bump

    * fix snap to angle logging statement

    * add names to drivetrain snap to X

    * fix comments

commit d8c257c
Author: aishahnazar <aishahnazar2@gmail.com>
Date:   Mon Mar 9 19:26:14 2026 -0700

    add front camera (#99)

    * add front camera

    * update constants for front camera

    * remove comments

    ---------

    Co-authored-by: zoe <zoe.z.zhao@gmail.com>
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