Limelight Subsystem replacement (REVIEWS ONLY, WILL NOT MERGE)

[koolpoolo]

For our switch to Limelight looking to get reviews on it very basic essentials for a vision subsystem

[jamesdooley4]

Please do the math (Excel is good at this) to compare the two methods for various tag distances & see how that affects the values in the vector below.

[markpete]

/gemini review

[gemini-code-assist]

Code Review

This pull request replaces the PhotonVision-based vision subsystem with a Limelight-based one. The changes include adding a Limelight helper library, a wrapper class for it, and updating the VisionSubsystem to use Limelight. While this is a good step, there are several critical issues in the implementation that need to be addressed. The LLCamera helper class has a fundamental flaw with a static name field that will cause incorrect behavior with multiple cameras. The VisionSubsystem has a critical bug in its update logic that will lead to incorrect pose estimation, and another bug that prevents tracking of vision data freshness. I've left detailed comments on these issues with suggestions for fixes.

[gemini-code-assist]

Code Review

This pull request replaces the PhotonVision-based vision subsystem with a new implementation using Limelight, including new helper classes and updated dependencies. The changes are a good step towards the new system, but I've identified a critical bug in the LLCamera helper class where a static field is used for the camera name, which will cause both cameras to point to the same device. There is also a high-severity logic issue in VisionSubsystem where state is being incorrectly overwritten in a loop, and several medium-severity issues related to UI layout, code duplication, and dead code. Please review the detailed comments for suggestions on how to address these issues.

[gemini-code-assist]

The name field is declared as static, which means it's shared across all instances of LLCamera. When you create two LLCamera objects (one for left_limelight and one for right_limelight), the second object's constructor will overwrite the name for both instances. This will cause both LLCamera objects to refer to the right_limelight, which is a significant bug.

To fix this, name should be an instance field.

Suggested change

	private static String name;
	public LLCamera(String name) {
	LLCamera.name = name;
	}
	private final String name;
	public LLCamera(String name) {
	this.name = name;
	}

[gemini-code-assist]

The processFiducials method updates the instance variables distance and tagAmbiguity for every valid fiducial it processes. Since this is called in a loop within update(), these variables are overwritten on each iteration. As a result, they will only reflect the values of the last valid fiducial seen by either camera, not necessarily the most relevant one (e.g., the closest).

If the goal is to track the closest target, you should compare the current fiducial's distance with the stored distance and only update if it's smaller. This would require initializing distance to a large value (e.g., Double.MAX_VALUE) at the beginning of the update() method.

  private void processFiducials(RawFiducial rf) {
    // distance to closest fiducial
    Optional<Pose3d> tagPose = fieldLayout.getTagPose(rf.id);
    if (tagPose.isPresent() && rf.distToCamera < this.distance) {
      this.distance = rf.distToCamera;
      this.tagAmbiguity = rf.ambiguity;
    }
  }

[gemini-code-assist]

The logic for processing the left camera (lines 116-122) and the right camera (lines 123-129) is identical. This code duplication should be refactored into a single helper method to improve maintainability. The helper method could take an LLCamera and a StructPublisher<Pose3d> as parameters.