Another config matching bug (#1518)

This is quite an odd issue/fix. So this is what happened... Photonvision booted with the camera connected and the camera was working... After a short time the camera stopped working (for some reason maybe static, maybe temp, maybe wiring, idk). During this time pv showed Jul 04 06:25:18 BackLeft java[643]: [2024-07-04 06:25:18] [CSCore - PvCSCoreLogger] [ERROR] CS: ERROR 40: ioctl VIDIOC_QBUF failed at UsbCameraImpl.cpp:723: Invalid argument (UsbUtil.cpp:156) Jul 04 06:25:18 BackLeft java[643]: [2024-07-04 06:25:18] [CSCore - PvCSCoreLogger] [WARN] CS: WARNING 30: BackLeft: could not queue buffer 0 (UsbCameraImpl.cpp:724) I went over and played with the wire. The camera fully disconnected but it ended up "reconnecting" When the camera was "reconnected" photonvision detected a "new camera" except this time with no otherpaths (aka no usb path, or by id path). That resulted in pv creating a new camera configuration for a camera with no otherpaths Cscore then started to report errors that look like it attempted to connect to the same camera twice This fixes it by filtering out USB cameras that have no otherpath on linux.
2026-06-23 01:21:40 +00:00 · 2024-11-04 21:50:18 -05:00
parent 625dacb020
commit 5d55d215ec
4 changed files with 97 additions and 53 deletions
--- a/photon-core/src/main/java/org/photonvision/common/configuration/CameraConfiguration.java
+++ b/photon-core/src/main/java/org/photonvision/common/configuration/CameraConfiguration.java
@@ -84,15 +84,7 @@ public class CameraConfiguration {
        this.calibrations = new ArrayList<>();
        this.otherPaths = alternates;

-        logger.debug(
-                "Creating USB camera configuration for "
-                        + cameraType
-                        + " "
-                        + baseName
-                        + " (AKA "
-                        + nickname
-                        + ") at "
-                        + path);
+        logger.debug("Creating USB camera configuration for " + this.toShortString());
    }

    @JsonCreator
@@ -120,15 +112,7 @@ public class CameraConfiguration {
        this.usbPID = usbPID;
        this.usbVID = usbVID;

-        logger.debug(
-                "Creating camera configuration for "
-                        + cameraType
-                        + " "
-                        + baseName
-                        + " (AKA "
-                        + nickname
-                        + ") at "
-                        + path);
+        logger.debug("Loaded camera configuration for " + toShortString());
    }

    public void addPipelineSettings(List<CVPipelineSettings> settings) {
@@ -189,6 +173,30 @@ public class CameraConfiguration {
        return Arrays.stream(otherPaths).filter(path -> path.contains("/by-path/")).findFirst();
    }

+    public String toShortString() {
+        return "CameraConfiguration [baseName="
+                + baseName
+                + ", uniqueName="
+                + uniqueName
+                + ", nickname="
+                + nickname
+                + ", path="
+                + path
+                + ", otherPaths="
+                + Arrays.toString(otherPaths)
+                + ", cameraType="
+                + cameraType
+                + ", cameraQuirks="
+                + cameraQuirks
+                + ", FOV="
+                + FOV
+                + "]"
+                + ", PID="
+                + usbPID
+                + ", VID="
+                + usbVID;
+    }
+
    @Override
    public String toString() {
        return "CameraConfiguration [baseName="
--- a/photon-core/src/main/java/org/photonvision/vision/processes/VisionSourceManager.java
+++ b/photon-core/src/main/java/org/photonvision/vision/processes/VisionSourceManager.java
@@ -30,6 +30,7 @@ import org.photonvision.common.configuration.ConfigManager;
 import org.photonvision.common.dataflow.DataChangeService;
 import org.photonvision.common.dataflow.events.OutgoingUIEvent;
 import org.photonvision.common.hardware.Platform;
+import org.photonvision.common.hardware.Platform.OSType;
 import org.photonvision.common.logging.LogGroup;
 import org.photonvision.common.logging.Logger;
 import org.photonvision.common.util.TimedTaskManager;
@@ -128,20 +129,26 @@ public class VisionSourceManager {
        return tryMatchCamImpl(null);
    }

+    protected List<VisionSource> tryMatchCamImpl(ArrayList<CameraInfo> cameraInfos) {
+        return tryMatchCamImpl(cameraInfos, Platform.getCurrentPlatform());
+    }
+
    /**
     * @param cameraInfos Used to feed camera info for unit tests.
     * @return New VisionSources.
     */
-    protected List<VisionSource> tryMatchCamImpl(ArrayList<CameraInfo> cameraInfos) {
+    protected List<VisionSource> tryMatchCamImpl(
+            ArrayList<CameraInfo> cameraInfos, Platform platform) {
        boolean createSources = true;
        List<CameraInfo> connectedCameras;
        if (cameraInfos == null) {
            // Detect USB cameras using CSCore
-            connectedCameras = new ArrayList<>(filterAllowedDevices(getConnectedUSBCameras()));
+            connectedCameras = new ArrayList<>(filterAllowedDevices(getConnectedUSBCameras(), platform));
            // Detect CSI cameras using libcamera
-            connectedCameras.addAll(new ArrayList<>(filterAllowedDevices(getConnectedCSICameras())));
+            connectedCameras.addAll(
+                    new ArrayList<>(filterAllowedDevices(getConnectedCSICameras(), platform)));
        } else {
-            connectedCameras = new ArrayList<>(filterAllowedDevices(cameraInfos));
+            connectedCameras = new ArrayList<>(filterAllowedDevices(cameraInfos, platform));
            createSources =
                    false; // Dont create sources if we are using supplied camerainfo for unit tests.
        }
@@ -162,7 +169,7 @@ public class VisionSourceManager {
        // All cameras are already loaded return no new sources.
        if (connectedCameras.isEmpty()) return null;

-        logger.debug("Matching " + connectedCameras.size() + " new cameras!");
+        logger.debug("Matching " + connectedCameras.size() + " new camera(s)!");

        // Debug prints
        for (var info : connectedCameras) {
@@ -170,7 +177,7 @@ public class VisionSourceManager {
        }

        if (!unmatchedLoadedConfigs.isEmpty())
-            logger.debug("Trying to match " + unmatchedLoadedConfigs.size() + " unmatched configs...");
+            logger.debug("Trying to match " + unmatchedLoadedConfigs.size() + " unmatched config(s)...");

        // Match camera configs to physical cameras
        List<CameraConfiguration> matchedCameras =
@@ -182,7 +189,7 @@ public class VisionSourceManager {
                    () ->
                            "After matching, "
                                    + unmatchedLoadedConfigs.size()
-                                    + " configs remained unmatched. Is your camera disconnected?");
+                                    + " config(s) remained unmatched. Is your camera disconnected?");
            logger.warn(
                    "Unloaded configs: "
                            + unmatchedLoadedConfigs.stream()
@@ -233,7 +240,7 @@ public class VisionSourceManager {
        if (checkUSBPath && savedConfig.getUSBPath().isEmpty()) {
            logger.debug(
                    "WARN: Camera has empty USB path, but asked to match by name: "
-                            + camCfgToString(savedConfig));
+                            + savedConfig.toShortString());
        }

        return (CameraInfo physicalCamera) -> {
@@ -277,22 +284,6 @@ public class VisionSourceManager {
                ConfigManager.getInstance().getConfig().getNetworkConfig().matchCamerasOnlyByPath);
    }

-    private static final String camCfgToString(CameraConfiguration c) {
-        return new StringBuilder()
-                .append("[baseName=")
-                .append(c.baseName)
-                .append(", uniqueName=")
-                .append(c.uniqueName)
-                .append(", otherPaths=")
-                .append(Arrays.toString(c.otherPaths))
-                .append(", vid=")
-                .append(c.usbVID)
-                .append(", pid=")
-                .append(c.usbPID)
-                .append("]")
-                .toString();
-    }
-
    /**
     * Create {@link CameraConfiguration}s based on a list of detected USB cameras and the configs on
     * disk.
@@ -423,7 +414,7 @@ public class VisionSourceManager {
                logger.debug(
                        String.format(
                                "Trying to find a match for loaded camera %s (%s) with camera config: %s",
-                                config.baseName, config.uniqueName, camCfgToString(config)));
+                                config.baseName, config.uniqueName, config.toShortString()));

                // Get matcher and filter against it, picking out the first match
                Predicate<CameraInfo> matches =
@@ -463,7 +454,7 @@ public class VisionSourceManager {
            List<CameraConfiguration> loadedConfigs) {
        List<CameraConfiguration> ret = new ArrayList<CameraConfiguration>();
        logger.debug(
-                "After matching loaded configs, these configs remained unmatched: "
+                "After matching loaded configs, these cameras remained unmatched: "
                        + detectedCameraList.stream()
                                .map(n -> String.valueOf(n))
                                .collect(Collectors.joining("-", "{", "}")));
@@ -537,7 +528,7 @@ public class VisionSourceManager {
     * @param allDevices
     * @return list of devices with blacklisted or ignore devices removed.
     */
-    private List<CameraInfo> filterAllowedDevices(List<CameraInfo> allDevices) {
+    private List<CameraInfo> filterAllowedDevices(List<CameraInfo> allDevices, Platform platform) {
        List<CameraInfo> filteredDevices = new ArrayList<>();
        for (var device : allDevices) {
            if (deviceBlacklist.contains(device.name)) {
@@ -546,6 +537,13 @@ public class VisionSourceManager {
            } else if (device.name.matches(ignoredCamerasRegex)) {
                logger.trace("Skipping ignored device: \"" + device.name + "\" at \"" + device.path);
            } else if (device.getIsV4lCsiCamera()) {
+            } else if (device.otherPaths.length == 0
+                    && platform.osType == OSType.LINUX
+                    && device.cameraType == CameraType.UsbCamera) {
+                logger.trace(
+                        "Skipping device with no other paths: \"" + device.name + "\" at \"" + device.path);
+                // If cscore hasnt passed this other paths aka a path by id or a path as in usb port then we
+                // cant guarantee it is a valid camera.
            } else {
                filteredDevices.add(device);
                logger.trace(
@@ -559,8 +557,6 @@ public class VisionSourceManager {
            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));
@@ -580,6 +576,7 @@ public class VisionSourceManager {
                    cameraSources.add(newCam);
                }
            }
+            logger.debug("Creating VisionSource for " + configuration.toShortString());
        }
        return cameraSources;
    }
--- a/photon-core/src/test/java/org/photonvision/vision/processes/VisionSourceManagerTest.java
+++ b/photon-core/src/test/java/org/photonvision/vision/processes/VisionSourceManagerTest.java
@@ -17,14 +17,14 @@

 package org.photonvision.vision.processes;

-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;

 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;
+import org.photonvision.common.hardware.Platform;
 import org.photonvision.common.logging.LogGroup;
 import org.photonvision.common.logging.LogLevel;
 import org.photonvision.common.logging.Logger;
@@ -62,7 +62,8 @@ public class VisionSourceManagerTest {
        config4.usbVID = 5;
        config4.usbPID = 6;

-        CameraInfo info1 = new CameraInfo(0, "dev/video0", "testVideo", new String[0], 1, 2);
+        CameraInfo info1 =
+                new CameraInfo(0, "dev/video0", "testVideo", new String[] {"/usb/path/0"}, 1, 2);

        cameraInfos.add(info1);

@@ -73,7 +74,8 @@ public class VisionSourceManagerTest {
        assertTrue(inst.knownCameras.contains(info1));
        assertEquals(2, inst.unmatchedLoadedConfigs.size());

-        CameraInfo info2 = new CameraInfo(0, "dev/video1", "secondTestVideo", new String[0], 2, 3);
+        CameraInfo info2 =
+                new CameraInfo(0, "dev/video1", "secondTestVideo", new String[] {"/usb/path/1"}, 2, 3);

        cameraInfos.add(info2);

@@ -500,6 +502,43 @@ public class VisionSourceManagerTest {
        }
    }

+    @Test
+    public void testNoOtherPaths() {
+        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[] {}, 3141, 25446);
+
+        cameraInfos.add(info1);
+
+        // Match two "new" cameras
+        var ret1 = inst.tryMatchCamImpl(cameraInfos, Platform.LINUX_64);
+
+        // Our cameras should be "known"
+        assertFalse(inst.knownCameras.contains(info1));
+        assertEquals(0, inst.knownCameras.size());
+        assertEquals(null, ret1);
+
+        // Match two "new" cameras
+        var ret2 = inst.tryMatchCamImpl(cameraInfos, Platform.WINDOWS_64);
+
+        // Our cameras should be "known"
+        assertTrue(inst.knownCameras.contains(info1));
+        assertEquals(1, inst.knownCameras.size());
+        assertEquals(1, ret2.size());
+    }
+
    @Test
    public void testIdenticalCameras() {
        Logger.setLevel(LogGroup.Camera, LogLevel.DEBUG);
--- a/photon-targeting/src/main/java/org/photonvision/common/hardware/Platform.java
+++ b/photon-targeting/src/main/java/org/photonvision/common/hardware/Platform.java
@@ -54,7 +54,7 @@ public enum Platform {
    LINUX_ARM32("Linux ARM32", "linuxarm32", false, OSType.LINUX, false), // ODROID XU4, C1+
    UNKNOWN("Unsupported Platform", "", false, OSType.UNKNOWN, false);

-    private enum OSType {
+    public enum OSType {
        WINDOWS,
        LINUX,
        MACOS,
@@ -123,7 +123,7 @@ public enum Platform {
    private static final String UnknownPlatformString =
            String.format("Unknown Platform. OS: %s, Architecture: %s", OS_NAME, OS_ARCH);

-    private static Platform getCurrentPlatform() {
+    public static Platform getCurrentPlatform() {
        if (RuntimeDetector.isWindows()) {
            if (RuntimeDetector.is32BitIntel()) {
                return WINDOWS_32;