From 469bc0eeaeee19433c70782b84e452d1fce34d21 Mon Sep 17 00:00:00 2001 From: Programmers3539 Date: Fri, 1 Dec 2023 07:58:38 -0500 Subject: [PATCH] Fix Camera Index Assignment. (#1031) Fixes a bug where multiple cameras would receive identical stream indexes and would cause at least one camera to not function at all and the other to not display a stream. One of the reasons the that usbcamerasource equals method used to incorrectly determine equality was because the quirkycamera object didn't have a basename. When a camera that had a quirk was found it would set equal to a predefined quirkycamera without changing the name first. Add unit test that will better test the equality of two usbcamerasources. This required a few changes to allow creating a fake usbcamerasource. --- .../vision/camera/QuirkyCamera.java | 16 +++++- .../vision/camera/USBCameraSource.java | 54 ++++++++++++++++--- .../vision/processes/VisionModule.java | 2 +- .../vision/processes/VisionSourceManager.java | 2 +- .../processes/VisionSourceSettables.java | 2 +- .../processes/VisionModuleManagerTest.java | 18 ++++++- 6 files changed, 81 insertions(+), 13 deletions(-) diff --git a/photon-core/src/main/java/org/photonvision/vision/camera/QuirkyCamera.java b/photon-core/src/main/java/org/photonvision/vision/camera/QuirkyCamera.java index 53d89167ec..05737ed293 100644 --- a/photon-core/src/main/java/org/photonvision/vision/camera/QuirkyCamera.java +++ b/photon-core/src/main/java/org/photonvision/vision/camera/QuirkyCamera.java @@ -17,6 +17,8 @@ package org.photonvision.vision.camera; +import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Objects; @@ -108,8 +110,20 @@ public static QuirkyCamera getQuirkyCamera(int usbVid, int usbPid, String baseNa for (var qc : quirkyCameras) { boolean hasBaseName = !qc.baseName.isEmpty(); boolean matchesBaseName = qc.baseName.equals(baseName) || !hasBaseName; + // If we have a quirkycamera we need to copy the quirks from our predefined object and create + // a quirkycamera object with the baseName. if (qc.usbVid == usbVid && qc.usbPid == usbPid && matchesBaseName) { - return qc; + List quirks = new ArrayList(); + for (var q : CameraQuirk.values()) { + if (qc.hasQuirk(q)) quirks.add(q); + } + QuirkyCamera c = + new QuirkyCamera( + usbVid, + usbPid, + baseName, + Arrays.copyOf(quirks.toArray(), quirks.size(), CameraQuirk[].class)); + return c; } } return new QuirkyCamera(usbVid, usbPid, baseName); diff --git a/photon-core/src/main/java/org/photonvision/vision/camera/USBCameraSource.java b/photon-core/src/main/java/org/photonvision/vision/camera/USBCameraSource.java index 6717f4edd8..a2f2911431 100644 --- a/photon-core/src/main/java/org/photonvision/vision/camera/USBCameraSource.java +++ b/photon-core/src/main/java/org/photonvision/vision/camera/USBCameraSource.java @@ -28,7 +28,9 @@ import org.photonvision.common.configuration.ConfigManager; import org.photonvision.common.logging.LogGroup; import org.photonvision.common.logging.Logger; +import org.photonvision.common.util.TestUtils; import org.photonvision.vision.frame.FrameProvider; +import org.photonvision.vision.frame.provider.FileFrameProvider; import org.photonvision.vision.frame.provider.USBFrameProvider; import org.photonvision.vision.processes.VisionSource; import org.photonvision.vision.processes.VisionSourceSettables; @@ -37,10 +39,10 @@ public class USBCameraSource extends VisionSource { private final Logger logger; private final UsbCamera camera; private final USBCameraSettables usbCameraSettables; - private final USBFrameProvider usbFrameProvider; + private FrameProvider usbFrameProvider; private final CvSink cvSink; - public final QuirkyCamera cameraQuirks; + private QuirkyCamera cameraQuirks; public USBCameraSource(CameraConfiguration config) { super(config); @@ -77,6 +79,22 @@ public USBCameraSource(CameraConfiguration config) { } } + /** + * Mostly just used for unit tests to better simulate a usb camera without a camera being present. + */ + public USBCameraSource(CameraConfiguration config, int pid, int vid, boolean unitTest) { + this(config); + + cameraQuirks = QuirkyCamera.getQuirkyCamera(pid, vid, config.baseName); + + if (unitTest) + usbFrameProvider = + new FileFrameProvider( + TestUtils.getWPIImagePath( + TestUtils.WPI2019Image.kCargoStraightDark72in_HighRes, false), + TestUtils.WPI2019Image.FOV); + } + void disableAutoFocus() { if (cameraQuirks.hasQuirk(CameraQuirk.AdjustableFocus)) { try { @@ -88,6 +106,10 @@ void disableAutoFocus() { } } + public QuirkyCamera getCameraQuirks() { + return this.cameraQuirks; + } + @Override public FrameProvider getFrameProvider() { return usbFrameProvider; @@ -103,7 +125,7 @@ protected USBCameraSettables(CameraConfiguration configuration) { super(configuration); getAllVideoModes(); if (!cameraQuirks.hasQuirk(CameraQuirk.StickyFPS)) - setVideoMode(videoModes.get(0)); // fixes double FPS set + if (!videoModes.isEmpty()) setVideoMode(videoModes.get(0)); // fixes double FPS set } public void setAutoExposure(boolean cameraAutoExposure) { @@ -343,11 +365,27 @@ public boolean isVendorCamera() { } @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - USBCameraSource that = (USBCameraSource) o; - return cameraQuirks.equals(that.cameraQuirks); + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null) return false; + if (getClass() != obj.getClass()) return false; + USBCameraSource other = (USBCameraSource) obj; + if (camera == null) { + if (other.camera != null) return false; + } else if (!camera.equals(other.camera)) return false; + if (usbCameraSettables == null) { + if (other.usbCameraSettables != null) return false; + } else if (!usbCameraSettables.equals(other.usbCameraSettables)) return false; + if (usbFrameProvider == null) { + if (other.usbFrameProvider != null) return false; + } else if (!usbFrameProvider.equals(other.usbFrameProvider)) return false; + if (cvSink == null) { + if (other.cvSink != null) return false; + } else if (!cvSink.equals(other.cvSink)) return false; + if (cameraQuirks == null) { + if (other.cameraQuirks != null) return false; + } else if (!cameraQuirks.equals(other.cameraQuirks)) return false; + return true; } @Override diff --git a/photon-core/src/main/java/org/photonvision/vision/processes/VisionModule.java b/photon-core/src/main/java/org/photonvision/vision/processes/VisionModule.java index 976053396b..483f6a7c0c 100644 --- a/photon-core/src/main/java/org/photonvision/vision/processes/VisionModule.java +++ b/photon-core/src/main/java/org/photonvision/vision/processes/VisionModule.java @@ -95,7 +95,7 @@ public VisionModule(PipelineManager pipelineManager, VisionSource visionSource, // Find quirks for the current camera if (visionSource instanceof USBCameraSource) { - cameraQuirks = ((USBCameraSource) visionSource).cameraQuirks; + cameraQuirks = ((USBCameraSource) visionSource).getCameraQuirks(); } else if (visionSource instanceof LibcameraGpuSource) { cameraQuirks = QuirkyCamera.ZeroCopyPiCamera; } else { diff --git a/photon-core/src/main/java/org/photonvision/vision/processes/VisionSourceManager.java b/photon-core/src/main/java/org/photonvision/vision/processes/VisionSourceManager.java index 4b31ba0691..c16e91e025 100644 --- a/photon-core/src/main/java/org/photonvision/vision/processes/VisionSourceManager.java +++ b/photon-core/src/main/java/org/photonvision/vision/processes/VisionSourceManager.java @@ -342,7 +342,7 @@ private static List loadVisionSourcesFromCamConfigs( cameraSources.add(piCamSrc); } else { var newCam = new USBCameraSource(configuration); - if (!newCam.cameraQuirks.hasQuirk(CameraQuirk.CompletelyBroken) + if (!newCam.getCameraQuirks().hasQuirk(CameraQuirk.CompletelyBroken) && !newCam.getSettables().videoModes.isEmpty()) { cameraSources.add(newCam); } diff --git a/photon-core/src/main/java/org/photonvision/vision/processes/VisionSourceSettables.java b/photon-core/src/main/java/org/photonvision/vision/processes/VisionSourceSettables.java index cda7d08e6f..f2057a18db 100644 --- a/photon-core/src/main/java/org/photonvision/vision/processes/VisionSourceSettables.java +++ b/photon-core/src/main/java/org/photonvision/vision/processes/VisionSourceSettables.java @@ -59,7 +59,7 @@ public void setBlueGain(int blue) {} public abstract VideoMode getCurrentVideoMode(); public void setVideoModeInternal(int index) { - setVideoMode(getAllVideoModes().get(index)); + if (!getAllVideoModes().isEmpty()) setVideoMode(getAllVideoModes().get(index)); } public void setVideoMode(VideoMode mode) { diff --git a/photon-core/src/test/java/org/photonvision/vision/processes/VisionModuleManagerTest.java b/photon-core/src/test/java/org/photonvision/vision/processes/VisionModuleManagerTest.java index e0a8376159..9b4ca528b6 100644 --- a/photon-core/src/test/java/org/photonvision/vision/processes/VisionModuleManagerTest.java +++ b/photon-core/src/test/java/org/photonvision/vision/processes/VisionModuleManagerTest.java @@ -31,6 +31,7 @@ import org.photonvision.common.configuration.ConfigManager; import org.photonvision.common.dataflow.CVPipelineResultConsumer; import org.photonvision.common.util.TestUtils; +import org.photonvision.vision.camera.USBCameraSource; import org.photonvision.vision.frame.FrameProvider; import org.photonvision.vision.frame.FrameStaticProperties; import org.photonvision.vision.frame.provider.FileFrameProvider; @@ -165,7 +166,16 @@ public void testMultipleStreamIndex() { TestUtils.WPI2019Image.FOV); var testSource3 = new TestSource(ffp3, conf3); - var modules = vmm.addSources(List.of(testSource, testSource2, testSource3)); + // Arducam OV9281 UC844 raspberry pi test. + var conf4 = new CameraConfiguration("Left", "dev/video1"); + USBCameraSource usbSimulation = new USBCameraSource(conf4, 0x6366, 0x0c45, true); + + var conf5 = new CameraConfiguration("Right", "dev/video2"); + USBCameraSource usbSimulation2 = new USBCameraSource(conf5, 0x6366, 0x0c45, true); + + var modules = + vmm.addSources( + List.of(testSource, testSource2, testSource3, usbSimulation, usbSimulation2)); System.out.println( Arrays.toString( @@ -176,9 +186,15 @@ public void testMultipleStreamIndex() { modules.stream() .map(it -> it.visionSource.getCameraConfiguration().streamIndex) .collect(Collectors.toList()); + + assertTrue(usbSimulation.equals(usbSimulation)); + assertTrue(!usbSimulation.equals(usbSimulation2)); + assertTrue(idxs.contains(0)); assertTrue(idxs.contains(1)); assertTrue(idxs.contains(2)); + assertTrue(idxs.contains(3)); + assertTrue(idxs.contains(4)); } private static void printTestResults(CVPipelineResult pipelineResult) {