Properly check camera info equality and handle zero cameras (#1245)

- Fix CameraInfo equality check (which prevents the same camera on a new usb port from being enumerated by us)
- Fix warning prints
- Make matchCamerasOnlyByPath apply to Windows
- Add unit tests

photon-core/src/main/java/org/photonvision/vision/camera/CameraInfo.java

@@ -20,6 +20,7 @@ package org.photonvision.vision.camera;
 import edu.wpi.first.cscore.UsbCameraInfo;
 import java.util.Arrays;
 import java.util.Optional;
+import org.photonvision.common.hardware.Platform;
 
 public class CameraInfo extends UsbCameraInfo {
     public final CameraType cameraType;
@@ -80,15 +81,27 @@ public class CameraInfo extends UsbCameraInfo {
     }
 
     @Override
-    public boolean equals(Object o) {
-        if (o == this) return true;
-        if (!(o instanceof UsbCameraInfo || o instanceof CameraInfo)) return false;
-        UsbCameraInfo other = (UsbCameraInfo) o;
-        return path.equals(other.path)
-                // && a.dev == b.dev (dev is not constant in Windows)
-                && name.equals(other.name)
-                && productId == other.productId
-                && vendorId == other.vendorId;
+    public boolean equals(Object obj) {
+        if (this == obj) return true;
+        if (obj == null) return false;
+        if (getClass() != obj.getClass()) return false;
+        CameraInfo other = (CameraInfo) obj;
+
+        // Windows device number is not significant. See
+        // https://github.com/wpilibsuite/allwpilib/blob/4b94a64b06057c723d6fcafeb1a45f55a70d179a/cscore/src/main/native/windows/UsbCameraImpl.cpp#L1128
+        if (!Platform.isWindows()) {
+            if (dev != other.dev) return false;
+        }
+
+        if (!path.equals(other.path)) return false;
+        if (!name.equals(other.name)) return false;
+        if (!Arrays.asList(this.otherPaths).containsAll(Arrays.asList(other.otherPaths))) return false;
+        if (vendorId != other.vendorId) return false;
+        if (productId != other.productId) return false;
+
+        // Don't trust super.equals, as it compares references. Should PR this to allwpilib at some
+        // point
+        return true;
     }
 
     @Override

photon-core/src/main/java/org/photonvision/vision/camera/TestSource.java

@@ -0,0 +1,93 @@
+/*
+ * Copyright (C) Photon Vision.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+package org.photonvision.vision.camera;
+
+import java.util.*;
+import org.photonvision.common.configuration.CameraConfiguration;
+import org.photonvision.vision.frame.Frame;
+import org.photonvision.vision.frame.FrameProvider;
+import org.photonvision.vision.frame.FrameThresholdType;
+import org.photonvision.vision.opencv.ImageRotationMode;
+import org.photonvision.vision.pipe.impl.HSVPipe.HSVParams;
+import org.photonvision.vision.processes.VisionSource;
+import org.photonvision.vision.processes.VisionSourceSettables;
+
+/** Dummy class for unit testing the vision source manager */
+public class TestSource extends VisionSource {
+    private FrameProvider usbFrameProvider;
+
+    public TestSource(CameraConfiguration config) {
+        super(config);
+
+        if (getCameraConfiguration().cameraQuirks == null)
+            getCameraConfiguration().cameraQuirks =
+                    QuirkyCamera.getQuirkyCamera(config.usbVID, config.usbVID, config.baseName);
+    }
+
+    @Override
+    public FrameProvider getFrameProvider() {
+        return new FrameProvider() {
+            @Override
+            public Frame get() {
+                // TODO Auto-generated method stub
+                throw new UnsupportedOperationException("Unimplemented method 'get'");
+            }
+
+            @Override
+            public String getName() {
+                return cameraConfiguration.uniqueName;
+            }
+
+            @Override
+            public void requestFrameThresholdType(FrameThresholdType type) {
+                // TODO Auto-generated method stub
+                throw new UnsupportedOperationException("Unimplemented method 'requestFrameThresholdType'");
+            }
+
+            @Override
+            public void requestFrameRotation(ImageRotationMode rotationMode) {
+                // TODO Auto-generated method stub
+                throw new UnsupportedOperationException("Unimplemented method 'requestFrameRotation'");
+            }
+
+            @Override
+            public void requestFrameCopies(boolean copyInput, boolean copyOutput) {
+                // TODO Auto-generated method stub
+                throw new UnsupportedOperationException("Unimplemented method 'requestFrameCopies'");
+            }
+
+            @Override
+            public void requestHsvSettings(HSVParams params) {
+                // TODO Auto-generated method stub
+                throw new UnsupportedOperationException("Unimplemented method 'requestHsvSettings'");
+            }
+        };
+    }
+
+    @Override
+    public VisionSourceSettables getSettables() {
+        // TODO Auto-generated method stub
+        throw new UnsupportedOperationException("Unimplemented method 'getSettables'");
+    }
+
+    @Override
+    public boolean isVendorCamera() {
+        // TODO Auto-generated method stub
+        throw new UnsupportedOperationException("Unimplemented method 'isVendorCamera'");
+    }
+}

photon-core/src/main/java/org/photonvision/vision/processes/VisionSourceManager.java

@@ -39,6 +39,7 @@ import org.photonvision.vision.camera.CameraInfo;
 import org.photonvision.vision.camera.CameraQuirk;
 import org.photonvision.vision.camera.CameraType;
 import org.photonvision.vision.camera.LibcameraGpuSource;
+import org.photonvision.vision.camera.TestSource;
 import org.photonvision.vision.camera.USBCameraSource;
 
 public class VisionSourceManager {
@@ -146,8 +147,8 @@ public class VisionSourceManager {
         }
 
         // Return no new sources because there are no new sources
-        if (connectedCameras.isEmpty() && !cameraInfos.isEmpty()) {
-            if (hasWarnedNoCameras) {
+        if (connectedCameras.isEmpty()) {
+            if (!hasWarnedNoCameras) {
                 logger.warn(
                         "No cameras were detected! Check that all cameras are connected, and that the path is correct.");
                 hasWarnedNoCameras = true;
@@ -186,7 +187,7 @@ public class VisionSourceManager {
                     "Unloaded configs: "
                             + unmatchedLoadedConfigs.stream()
                                     .map(it -> it.nickname)
-                                    .collect(Collectors.joining()));
+                                    .collect(Collectors.joining(", ")));
             hasWarned = true;
         }
 
@@ -195,13 +196,8 @@ public class VisionSourceManager {
 
         if (matchedCameras.isEmpty()) return null;
 
-        // for unit tests only!
-        if (!createSources) {
-            return List.of();
-        }
-
         // Turn these camera configs into vision sources
-        var sources = loadVisionSourcesFromCamConfigs(matchedCameras);
+        var sources = loadVisionSourcesFromCamConfigs(matchedCameras, createSources);
 
         // Print info about each vision source
         for (var src : sources) {
@@ -321,7 +317,7 @@ public class VisionSourceManager {
 
         // On windows, the v4l path is actually useful and tells us the port the camera is physically
         // connected to which is neat
-        if (Platform.isWindows()) {
+        if (Platform.isWindows() && !matchCamerasOnlyByPath) {
             if (detectedCameraList.size() > 0 || unloadedConfigs.size() > 0) {
                 logger.info("Matching by windows-path & USB VID/PID only...");
                 cameraConfigurations.addAll(
@@ -456,7 +452,8 @@ public class VisionSourceManager {
             int suffix = 0;
             while (containsName(loadedConfigs, uniqueName)
                     || containsName(uniqueName)
-                    || containsName(unloadedCamConfigs, uniqueName)) {
+                    || containsName(unloadedCamConfigs, uniqueName)
+                    || containsName(ret, uniqueName)) {
                 suffix++;
                 uniqueName = String.format("%s (%d)", uniqueName, suffix);
             }
@@ -536,11 +533,17 @@ public class VisionSourceManager {
     }
 
     private static List<VisionSource> loadVisionSourcesFromCamConfigs(
-            List<CameraConfiguration> camConfigs) {
+            List<CameraConfiguration> camConfigs, boolean createSources) {
         var cameraSources = new ArrayList<VisionSource>();
         for (var configuration : camConfigs) {
             logger.debug("Creating VisionSource for " + camCfgToString(configuration));
 
+            // In unit tests, create dummy
+            if (!createSources) {
+                cameraSources.add(new TestSource(configuration));
+                continue;
+            }
+
             boolean is_pi = Platform.isRaspberryPi();
 
             if (configuration.cameraType == CameraType.ZeroCopyPicam && is_pi) {

photon-core/src/test/java/org/photonvision/vision/processes/VisionSourceManagerTest.java

@@ -21,6 +21,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.ArrayList;
+import java.util.List;
 import org.junit.jupiter.api.Test;
 import org.photonvision.common.configuration.CameraConfiguration;
 import org.photonvision.common.configuration.ConfigManager;
@@ -271,4 +272,268 @@ public class VisionSourceManagerTest {
         assertEquals(10, inst.knownCameras.size());
         assertEquals(0, inst.unmatchedLoadedConfigs.size());
     }
+
+    @Test
+    public void testDisableInhibitPathChangeIdenticalCams() {
+        Logger.setLevel(LogGroup.Camera, LogLevel.DEBUG);
+
+        var inst = new VisionSourceManager();
+        ConfigManager.getInstance().clearConfig();
+        ConfigManager.getInstance().load();
+        ConfigManager.getInstance().getConfig().getNetworkConfig().matchCamerasOnlyByPath = false;
+
+        var CAM2_OLD_PATH =
+                new String[] {"/dev/v4l/by-path/platform-fc880000.usb-usb-0:1:1.0-video-index0"};
+        var CAM2_NEW_PATH =
+                new String[] {"/dev/v4l/by-path/platform-fc880080.usb-usb-0:1:1.3-video-index0"};
+
+        var CAM1_OLD_PATHS =
+                new String[] {
+                    "/dev/v4l/by-id/usb-Arducam_Technology_Co.__Ltd._Arducam_OV2311_USB_Camera_UC621-video-index0",
+                    "/dev/v4l/by-path/platform-fc800000.usb-usb-0:1:1.0-video-index0"
+                };
+
+        var camera1_saved_config =
+                new CameraConfiguration(
+                        "Arducam OV2311 USB Camera",
+                        "Arducam OV2311 USB Camera",
+                        "fromt-left",
+                        "/dev/video0",
+                        CAM1_OLD_PATHS);
+        camera1_saved_config.usbVID = 3141;
+        camera1_saved_config.usbPID = 25446;
+        var camera2_saved_config =
+                new CameraConfiguration(
+                        "Arducam OV2311 USB Camera",
+                        "Arducam OV2311 USB Camera (1)",
+                        "fromt-left",
+                        "/dev/video2",
+                        CAM2_OLD_PATH);
+        camera2_saved_config.usbVID = 3141;
+        camera2_saved_config.usbPID = 25446;
+
+        // And load our "old" configs
+        inst.registerLoadedConfigs(camera1_saved_config, camera2_saved_config);
+
+        // Camera attached to new port, but strict matching disabled
+        {
+            CameraInfo info1 =
+                    new CameraInfo(
+                            0, "/dev/video11", "Arducam OV2311 USB Camera", CAM1_OLD_PATHS, 3141, 25446);
+            CameraInfo info2 =
+                    new CameraInfo(
+                            0, "/dev/video12", "Arducam OV2311 USB Camera", CAM2_NEW_PATH, 3141, 25446);
+
+            var cameraInfos = new ArrayList<CameraInfo>();
+            cameraInfos.add(info1);
+            cameraInfos.add(info2);
+            List<VisionSource> ret1 = inst.tryMatchCamImpl(cameraInfos);
+
+            // and check the new one got matched got matched
+            assertEquals(2, ret1.size());
+            assertEquals(
+                    1, ret1.stream().filter(it -> it.cameraConfiguration.path.equals(info1.path)).count());
+            assertEquals(
+                    1, ret1.stream().filter(it -> it.cameraConfiguration.path.equals(info2.path)).count());
+        }
+    }
+
+    @Test
+    public void testInhibitPathChangeIdenticalCams() {
+        Logger.setLevel(LogGroup.Camera, LogLevel.DEBUG);
+
+        var inst = new VisionSourceManager();
+        ConfigManager.getInstance().clearConfig();
+        ConfigManager.getInstance().load();
+        ConfigManager.getInstance().getConfig().getNetworkConfig().matchCamerasOnlyByPath = true;
+
+        var CAM2_OLD_PATH =
+                new String[] {"/dev/v4l/by-path/platform-fc880000.usb-usb-0:1:1.0-video-index0"};
+        var CAM2_NEW_PATH =
+                new String[] {"/dev/v4l/by-path/platform-fc880080.usb-usb-0:1:1.3-video-index0"};
+
+        var CAM1_OLD_PATHS =
+                new String[] {
+                    "/dev/v4l/by-id/usb-Arducam_Technology_Co.__Ltd._Arducam_OV2311_USB_Camera_UC621-video-index0",
+                    "/dev/v4l/by-path/platform-fc800000.usb-usb-0:1:1.0-video-index0"
+                };
+
+        var camera1_saved_config =
+                new CameraConfiguration(
+                        "Arducam OV2311 USB Camera",
+                        "Arducam OV2311 USB Camera (1)",
+                        "fromt-left",
+                        "/dev/video0",
+                        CAM1_OLD_PATHS);
+        camera1_saved_config.usbVID = 3141;
+        camera1_saved_config.usbPID = 25446;
+        var camera2_saved_config =
+                new CameraConfiguration(
+                        "Arducam OV2311 USB Camera",
+                        "Arducam OV2311 USB Camera (1)",
+                        "fromt-left",
+                        "/dev/video2",
+                        CAM2_OLD_PATH);
+        camera2_saved_config.usbVID = 3141;
+        camera2_saved_config.usbPID = 25446;
+
+        // And load our "old" configs
+        inst.registerLoadedConfigs(camera1_saved_config, camera2_saved_config);
+
+        // initial pass with camera in the wrong spot
+        {
+            // Give our cameras new "paths" to fake the windows logic out. this should not
+            // affect strict matching
+            CameraInfo info1 =
+                    new CameraInfo(
+                            0, "/dev/video11", "Arducam OV2311 USB Camera", CAM1_OLD_PATHS, 3141, 25446);
+            CameraInfo info2 =
+                    new CameraInfo(
+                            0, "/dev/video12", "Arducam OV2311 USB Camera", CAM2_NEW_PATH, 3141, 25446);
+
+            var cameraInfos = new ArrayList<CameraInfo>();
+            cameraInfos.add(info1);
+            cameraInfos.add(info2);
+            List<VisionSource> ret1 = inst.tryMatchCamImpl(cameraInfos);
+
+            // Our cameras should be "known"
+            assertTrue(inst.knownCameras.contains(info1));
+            assertTrue(inst.knownCameras.contains(info2));
+            assertEquals(2, inst.knownCameras.size());
+
+            // And we should have matched one camera
+            assertEquals(1, ret1.size());
+            // and only matched camera1, not 2
+            assertEquals(
+                    1, ret1.stream().filter(it -> it.cameraConfiguration.path.equals(info1.path)).count());
+            assertEquals(
+                    0, ret1.stream().filter(it -> it.cameraConfiguration.path.equals(info2.path)).count());
+        }
+
+        // Now move our camera back
+        {
+            CameraInfo info1 =
+                    new CameraInfo(
+                            0, "/dev/video11", "Arducam OV2311 USB Camera", CAM1_OLD_PATHS, 3141, 25446);
+            CameraInfo info2 =
+                    new CameraInfo(
+                            0, "/dev/video12", "Arducam OV2311 USB Camera", CAM2_OLD_PATH, 3141, 25446);
+
+            var cameraInfos = new ArrayList<CameraInfo>();
+            cameraInfos.add(info1);
+            cameraInfos.add(info2);
+            List<VisionSource> ret1 = inst.tryMatchCamImpl(cameraInfos);
+
+            // and check the new one got matched got matched
+            assertEquals(1, ret1.size());
+            assertEquals(
+                    0, ret1.stream().filter(it -> it.cameraConfiguration.path.equals(info1.path)).count());
+            assertEquals(
+                    1, ret1.stream().filter(it -> it.cameraConfiguration.path.equals(info2.path)).count());
+        }
+    }
+
+    @Test
+    public void testIdenticalCameras() {
+        Logger.setLevel(LogGroup.Camera, LogLevel.DEBUG);
+
+        // List of known cameras
+        var cameraInfos = new ArrayList<CameraInfo>();
+
+        var inst = new VisionSourceManager();
+        ConfigManager.getInstance().clearConfig();
+        ConfigManager.getInstance().load();
+        ConfigManager.getInstance().getConfig().getNetworkConfig().matchCamerasOnlyByPath = false;
+
+        // Match empty camera infos
+        inst.tryMatchCamImpl(cameraInfos);
+
+        CameraInfo info1 =
+                new CameraInfo(
+                        0,
+                        "/dev/video0",
+                        "Arducam OV2311 USB Camera",
+                        new String[] {
+                            "/dev/v4l/by-id/usb-Arducam_Technology_Co.__Ltd._Arducam_OV2311_USB_Camera_UC621-video-index0",
+                            "/dev/v4l/by-path/platform-fc800000.usb-usb-0:1:1.0-video-index0"
+                        },
+                        3141,
+                        25446);
+        CameraInfo info2 =
+                new CameraInfo(
+                        0,
+                        "/dev/video2",
+                        "Arducam OV2311 USB Camera",
+                        new String[] {
+                            "/dev/v4l/by-id/usb-Arducam_Technology_Co.__Ltd._Arducam_OV2311_USB_Camera_UC621-video-index0",
+                            "/dev/v4l/by-path/platform-fc880000.usb-usb-0:1:1.0-video-index0"
+                        },
+                        3141,
+                        25446);
+
+        cameraInfos.add(info1);
+        cameraInfos.add(info2);
+
+        // Match two "new" cameras
+        var ret1 = inst.tryMatchCamImpl(cameraInfos);
+
+        // Our cameras should be "known"
+        assertTrue(inst.knownCameras.contains(info1));
+        assertTrue(inst.knownCameras.contains(info2));
+        assertEquals(2, inst.knownCameras.size());
+        assertEquals(2, ret1.size());
+
+        // Exactly one camera should have the path we put in
+        for (int i = 0; i < cameraInfos.size(); i++) {
+            var testPath = cameraInfos.get(i).getUSBPath().get();
+            assertEquals(
+                    1,
+                    ret1.stream()
+                            .filter(it -> testPath.equals(it.cameraConfiguration.getUSBPath().get()))
+                            .count());
+        }
+
+        // and the names should be unique
+        for (int i = 0; i < ret1.size(); i++) {
+            var thisName = ret1.get(i).cameraConfiguration.uniqueName;
+            assertEquals(
+                    1,
+                    ret1.stream().filter(it -> thisName.equals(it.cameraConfiguration.uniqueName)).count());
+        }
+
+        // duplciate cameras, same info, new ref
+        var duplicateCameraInfos = new ArrayList<CameraInfo>();
+        CameraInfo info1_dup =
+                new CameraInfo(
+                        0,
+                        "/dev/video0",
+                        "Arducam OV2311 USB Camera",
+                        new String[] {
+                            "/dev/v4l/by-id/usb-Arducam_Technology_Co.__Ltd._Arducam_OV2311_USB_Camera_UC621-video-index0",
+                            "/dev/v4l/by-path/platform-fc800000.usb-usb-0:1:1.0-video-index0"
+                        },
+                        3141,
+                        25446);
+        CameraInfo info2_dup =
+                new CameraInfo(
+                        0,
+                        "/dev/video2",
+                        "Arducam OV2311 USB Camera",
+                        new String[] {
+                            "/dev/v4l/by-id/usb-Arducam_Technology_Co.__Ltd._Arducam_OV2311_USB_Camera_UC621-video-index0",
+                            "/dev/v4l/by-path/platform-fc880000.usb-usb-0:1:1.0-video-index0"
+                        },
+                        3141,
+                        25446);
+
+        duplicateCameraInfos.add(info1_dup);
+        duplicateCameraInfos.add(info2_dup);
+
+        inst.tryMatchCamImpl(duplicateCameraInfos);
+
+        // Our cameras should be "known", and we should only "know" two cameras still
+        assertTrue(inst.knownCameras.contains(info1_dup));
+        assertTrue(inst.knownCameras.contains(info2_dup));
+        assertEquals(2, inst.knownCameras.size());
+    }
 }