From 01dc7ea5ceb09994ecf6af20e2306e92a94eb203 Mon Sep 17 00:00:00 2001 From: Matt Date: Mon, 19 Feb 2024 12:34:57 -0500 Subject: [PATCH] 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 --- .../vision/camera/CameraInfo.java | 31 +- .../vision/camera/TestSource.java | 93 ++++++ .../vision/processes/VisionSourceManager.java | 27 +- .../processes/VisionSourceManagerTest.java | 265 ++++++++++++++++++ 4 files changed, 395 insertions(+), 21 deletions(-) create mode 100644 photon-core/src/main/java/org/photonvision/vision/camera/TestSource.java diff --git a/photon-core/src/main/java/org/photonvision/vision/camera/CameraInfo.java b/photon-core/src/main/java/org/photonvision/vision/camera/CameraInfo.java index 32441a308..8f5893401 100644 --- a/photon-core/src/main/java/org/photonvision/vision/camera/CameraInfo.java +++ b/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 diff --git a/photon-core/src/main/java/org/photonvision/vision/camera/TestSource.java b/photon-core/src/main/java/org/photonvision/vision/camera/TestSource.java new file mode 100644 index 000000000..51a5cef58 --- /dev/null +++ b/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 . + */ + +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'"); + } +} 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 ef06d474b..b7a8aa5b3 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 @@ -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 loadVisionSourcesFromCamConfigs( - List camConfigs) { + 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)); + continue; + } + boolean is_pi = Platform.isRaspberryPi(); if (configuration.cameraType == CameraType.ZeroCopyPicam && is_pi) { 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 3f92b0e71..f82993e05 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 @@ -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(); + cameraInfos.add(info1); + cameraInfos.add(info2); + List 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(); + cameraInfos.add(info1); + cameraInfos.add(info2); + List 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(); + cameraInfos.add(info1); + cameraInfos.add(info2); + List 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(); + + 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 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()); + } }