From 5d55d215ec291c3f8c562a1b17c4b8a26cb0b8f7 Mon Sep 17 00:00:00 2001 From: "Cameron (3539)" Date: Mon, 4 Nov 2024 21:50:18 -0500 Subject: [PATCH] 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. --- .../configuration/CameraConfiguration.java | 44 +++++++++------ .../vision/processes/VisionSourceManager.java | 55 +++++++++---------- .../processes/VisionSourceManagerTest.java | 47 ++++++++++++++-- .../common/hardware/Platform.java | 4 +- 4 files changed, 97 insertions(+), 53 deletions(-) diff --git a/photon-core/src/main/java/org/photonvision/common/configuration/CameraConfiguration.java b/photon-core/src/main/java/org/photonvision/common/configuration/CameraConfiguration.java index 93baa446c..211f3cb8d 100644 --- 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 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=" 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 577611eba..f71bd9d92 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 @@ -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 tryMatchCamImpl(ArrayList cameraInfos) { + return tryMatchCamImpl(cameraInfos, Platform.getCurrentPlatform()); + } + /** * @param cameraInfos Used to feed camera info for unit tests. * @return New VisionSources. */ - protected List tryMatchCamImpl(ArrayList cameraInfos) { + protected List tryMatchCamImpl( + ArrayList cameraInfos, Platform platform) { boolean createSources = true; List 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 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 matches = @@ -463,7 +454,7 @@ public class VisionSourceManager { List loadedConfigs) { List ret = new ArrayList(); 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 filterAllowedDevices(List allDevices) { + private List filterAllowedDevices(List allDevices, Platform platform) { List 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 camConfigs, boolean createSources) { var cameraSources = new ArrayList(); 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; } diff --git a/photon-core/src/test/java/org/photonvision/vision/processes/VisionSourceManagerTest.java b/photon-core/src/test/java/org/photonvision/vision/processes/VisionSourceManagerTest.java index 7f4755376..17cd2ed06 100644 --- 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(); + + 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); diff --git a/photon-targeting/src/main/java/org/photonvision/common/hardware/Platform.java b/photon-targeting/src/main/java/org/photonvision/common/hardware/Platform.java index e2d015560..fdb51c279 100644 --- 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;