From 6db5bc5e0c439a2e794fb1b36e0840fc67642fc6 Mon Sep 17 00:00:00 2001 From: Joseph Farkas <16584585+MrRedness@users.noreply.github.com> Date: Mon, 4 Dec 2023 16:55:16 +0000 Subject: [PATCH] Matching cameras by path ID (#1015) Allows multiple cameras of the same model to be used while ensuring they stay tied to the physical camera and not the port. Matching occurs when first connecting cameras so swapping the ports while PV is running will swap the virtual cameras until a restart. Currently only tested on Linux. --- .../configuration/SqlConfigProvider.java | 15 +- .../vision/processes/VisionSourceManager.java | 154 ++++++++++++++---- .../processes/VisionSourceManagerTest.java | 81 ++++++++- 3 files changed, 208 insertions(+), 42 deletions(-) diff --git a/photon-core/src/main/java/org/photonvision/common/configuration/SqlConfigProvider.java b/photon-core/src/main/java/org/photonvision/common/configuration/SqlConfigProvider.java index 8a0c755ad..3f22e6952 100644 --- a/photon-core/src/main/java/org/photonvision/common/configuration/SqlConfigProvider.java +++ b/photon-core/src/main/java/org/photonvision/common/configuration/SqlConfigProvider.java @@ -51,6 +51,7 @@ public class SqlConfigProvider extends ConfigProvider { static final String CAM_UNIQUE_NAME = "unique_name"; static final String CONFIG_JSON = "config_json"; static final String DRIVERMODE_JSON = "drivermode_json"; + static final String OTHERPATHS_JSON = "otherpaths_json"; static final String PIPELINE_JSONS = "pipeline_jsons"; static final String NETWORK_CONFIG = "networkConfig"; @@ -147,6 +148,7 @@ public class SqlConfigProvider extends ConfigProvider { + " unique_name TINYTEXT PRIMARY KEY,\n" + " config_json text NOT NULL,\n" + " drivermode_json text NOT NULL,\n" + + " otherpaths_json text NOT NULL,\n" + " pipeline_jsons mediumtext NOT NULL\n" + ");"; createCameraTableStatement.execute(sql); @@ -295,8 +297,8 @@ public class SqlConfigProvider extends ConfigProvider { try { // Replace this camera's row with the new settings var sqlString = - "REPLACE INTO cameras (unique_name, config_json, drivermode_json, pipeline_jsons) VALUES " - + "(?,?,?,?);"; + "REPLACE INTO cameras (unique_name, config_json, drivermode_json, otherpaths_json, pipeline_jsons) VALUES " + + "(?,?,?,?,?);"; for (var c : config.getCameraConfigurations().entrySet()) { PreparedStatement statement = conn.prepareStatement(sqlString); @@ -305,6 +307,7 @@ public class SqlConfigProvider extends ConfigProvider { statement.setString(1, c.getKey()); statement.setString(2, JacksonUtils.serializeToString(config)); statement.setString(3, JacksonUtils.serializeToString(config.driveModeSettings)); + statement.setString(4, JacksonUtils.serializeToString(config.otherPaths)); // Serializing a list of abstract classes sucks. Instead, make it into an array // of strings, which we can later unpack back into individual settings @@ -321,7 +324,7 @@ public class SqlConfigProvider extends ConfigProvider { }) .filter(Objects::nonNull) .collect(Collectors.toList()); - statement.setString(4, JacksonUtils.serializeToString(settings)); + statement.setString(5, JacksonUtils.serializeToString(settings)); statement.executeUpdate(); } @@ -455,10 +458,11 @@ public class SqlConfigProvider extends ConfigProvider { query = conn.prepareStatement( String.format( - "SELECT %s, %s, %s, %s FROM cameras", + "SELECT %s, %s, %s, %s, %s FROM cameras", TableKeys.CAM_UNIQUE_NAME, TableKeys.CONFIG_JSON, TableKeys.DRIVERMODE_JSON, + TableKeys.OTHERPATHS_JSON, TableKeys.PIPELINE_JSONS)); var result = query.executeQuery(); @@ -474,6 +478,8 @@ public class SqlConfigProvider extends ConfigProvider { var driverMode = JacksonUtils.deserialize( result.getString(TableKeys.DRIVERMODE_JSON), DriverModePipelineSettings.class); + var otherPaths = + JacksonUtils.deserialize(result.getString(TableKeys.OTHERPATHS_JSON), String[].class); List pipelineSettings = JacksonUtils.deserialize( result.getString(TableKeys.PIPELINE_JSONS), dummyList.getClass()); @@ -487,6 +493,7 @@ public class SqlConfigProvider extends ConfigProvider { config.pipelineSettings = loadedSettings; config.driveModeSettings = driverMode; + config.otherPaths = otherPaths; loadedConfigurations.put(uniqueName, config); } } catch (SQLException | IOException e) { 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 c16e91e02..2e6ada0aa 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 @@ -196,52 +196,112 @@ public class VisionSourceManager { * @param loadedUsbCamConfigs The USB {@link CameraConfiguration}s loaded from disk. * @return the matched configurations. */ - private List matchUSBCameras( + protected List matchUSBCameras( List detectedCamInfos, List loadedUsbCamConfigs) { var detectedCameraList = new ArrayList<>(detectedCamInfos); ArrayList cameraConfigurations = new ArrayList<>(); - // loop over all the configs loaded from disk + List unmatchedAfterByID = new ArrayList<>(loadedUsbCamConfigs); + + // loop over all the configs loaded from disk, attempting to match each camera + // to a config by path-by-id on linux for (CameraConfiguration config : loadedUsbCamConfigs) { UsbCameraInfo cameraInfo; - // attempt matching by path and basename - logger.debug( - "Trying to find a match for loaded camera " - + config.baseName - + " with path " - + config.path); - cameraInfo = - detectedCameraList.stream() - .filter( - usbCameraInfo -> - usbCameraInfo.path.equals(config.path) - && cameraNameToBaseName(usbCameraInfo.name).equals(config.baseName)) - .findFirst() - .orElse(null); - - // if path based fails, attempt basename only match - if (cameraInfo == null) { - logger.debug("Failed to match by path and name, falling back to name-only match"); + if (config.otherPaths.length == 0) { + logger.debug("No valid path-by-id found for config with name " + config.baseName); + } else { + // attempt matching by path and basename + logger.debug( + "Trying to find a match for loaded camera " + + config.baseName + + " with path-by-id " + + config.otherPaths[0]); cameraInfo = detectedCameraList.stream() .filter( usbCameraInfo -> - cameraNameToBaseName(usbCameraInfo.name).equals(config.baseName)) + usbCameraInfo.otherPaths.length != 0 + && usbCameraInfo.otherPaths[0].equals(config.otherPaths[0]) + && cameraNameToBaseName(usbCameraInfo.name).equals(config.baseName)) .findFirst() .orElse(null); - } - // If we actually matched a camera to a config, remove that camera from the list and add it to - // the output - if (cameraInfo != null) { - logger.debug("Matched the config for " + config.baseName + " to a physical camera!"); - detectedCameraList.remove(cameraInfo); - cameraConfigurations.add(mergeInfoIntoConfig(config, cameraInfo)); + // If we actually matched a camera to a config, remove that camera from the list + // and add it to the output + if (cameraInfo != null) { + logger.debug("Matched the config for " + config.baseName + " to a physical camera!"); + detectedCameraList.remove(cameraInfo); + unmatchedAfterByID.remove(config); + cameraConfigurations.add(mergeInfoIntoConfig(config, cameraInfo)); + } } } - // If we have any unmatched cameras left, create a new CameraConfiguration for them here. + if (!unmatchedAfterByID.isEmpty() && !detectedCameraList.isEmpty()) { + logger.debug("Match by path-by-id failed, falling back to path-only matching"); + + List unmatchedAfterByPath = new ArrayList<>(loadedUsbCamConfigs); + + // now attempt to match the cameras and configs remaining by normal path + for (CameraConfiguration config : unmatchedAfterByID) { + UsbCameraInfo cameraInfo; + + // attempt matching by path and basename + logger.debug( + "Trying to find a match for loaded camera " + + config.baseName + + " with path " + + config.path); + cameraInfo = + detectedCameraList.stream() + .filter( + usbCameraInfo -> + usbCameraInfo.path.equals(config.path) + && cameraNameToBaseName(usbCameraInfo.name).equals(config.baseName)) + .findFirst() + .orElse(null); + + // If we actually matched a camera to a config, remove that camera from the list + // and add it to the output + if (cameraInfo != null) { + logger.debug("Matched the config for " + config.baseName + " to a physical camera!"); + detectedCameraList.remove(cameraInfo); + unmatchedAfterByPath.remove(config); + cameraConfigurations.add(mergeInfoIntoConfig(config, cameraInfo)); + } + } + + if (!unmatchedAfterByPath.isEmpty() && !detectedCameraList.isEmpty()) { + logger.debug("Match by ID and path failed, falling back to name-only matching"); + + // if both path and ID based matching fails, attempt basename only match + for (CameraConfiguration config : unmatchedAfterByPath) { + UsbCameraInfo cameraInfo; + + logger.debug("Trying to find a match for loaded camera with name " + config.baseName); + + cameraInfo = + detectedCameraList.stream() + .filter( + usbCameraInfo -> + cameraNameToBaseName(usbCameraInfo.name).equals(config.baseName)) + .findFirst() + .orElse(null); + + // If we actually matched a camera to a config, remove that camera from the list + // and add it to the output + if (cameraInfo != null) { + logger.debug("Matched the config for " + config.baseName + " to a physical camera!"); + detectedCameraList.remove(cameraInfo); + cameraConfigurations.add(mergeInfoIntoConfig(config, cameraInfo)); + } + } + } + } + + // If we have any unmatched cameras left, create a new CameraConfiguration for + // them here. logger.debug( "After matching loaded configs " + detectedCameraList.size() + " cameras were unmatched."); for (UsbCameraInfo info : detectedCameraList) { @@ -250,7 +310,7 @@ public class VisionSourceManager { String uniqueName = baseNameToUniqueName(baseName); int suffix = 0; - while (containsName(cameraConfigurations, uniqueName)) { + while (containsName(cameraConfigurations, uniqueName) || containsName(uniqueName)) { suffix++; uniqueName = String.format("%s (%d)", uniqueName, suffix); } @@ -283,6 +343,27 @@ public class VisionSourceManager { cfg.path = info.path; } + if (cfg.otherPaths.length != info.otherPaths.length) { + logger.debug( + "Updating otherPath config from " + + Arrays.toString(cfg.otherPaths) + + " to " + + Arrays.toString(info.otherPaths)); + cfg.otherPaths = info.otherPaths.clone(); + } else { + for (int i = 0; i < info.otherPaths.length; i++) { + if (!cfg.otherPaths[i].equals(info.otherPaths[i])) { + logger.debug( + "Updating otherPath config from " + + Arrays.toString(cfg.otherPaths) + + " to " + + Arrays.toString(info.otherPaths)); + cfg.otherPaths = info.otherPaths.clone(); + break; + } + } + } + return cfg; } @@ -309,7 +390,7 @@ public class VisionSourceManager { private boolean usbCamEquals(UsbCameraInfo a, UsbCameraInfo b) { return a.path.equals(b.path) - && a.dev == b.dev + // && a.dev == b.dev (dev is not constant in Windows) && a.name.equals(b.name) && a.productId == b.productId && a.vendorId == b.vendorId; @@ -363,4 +444,15 @@ public class VisionSourceManager { return configList.stream() .anyMatch(configuration -> configuration.uniqueName.equals(uniqueName)); } + + /** + * Check if the current list of known cameras contains the given unique name. + * + * @param uniqueName The unique name. + * @return If the list of cameras contains the unique name. + */ + private boolean containsName(final String uniqueName) { + return VisionModuleManager.getInstance().getModules().stream() + .anyMatch(camera -> camera.visionSource.cameraConfiguration.uniqueName.equals(uniqueName)); + } } 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 2c64a9a62..e16213000 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 @@ -35,23 +35,90 @@ public class VisionSourceManagerTest { ConfigManager.getInstance().load(); inst.tryMatchUSBCamImpl(); - var config = new CameraConfiguration("secondTestVideo", "dev/video1"); + var config3 = + new CameraConfiguration( + "secondTestVideo", + "secondTestVideo1", + "secondTestVideo1", + "dev/video1", + new String[] {"by-id/123"}); + var config4 = + new CameraConfiguration( + "secondTestVideo", + "secondTestVideo2", + "secondTestVideo2", + "dev/video2", + new String[] {"by-id/321"}); + UsbCameraInfo info1 = new UsbCameraInfo(0, "dev/video0", "testVideo", new String[0], 1, 2); + infoList.add(info1); - inst.registerLoadedConfigs(config); - var sources = inst.tryMatchUSBCamImpl(false); + inst.registerLoadedConfigs(config3, config4); + + inst.tryMatchUSBCamImpl(false); assertTrue(inst.knownUsbCameras.contains(info1)); - assertEquals(1, inst.unmatchedLoadedConfigs.size()); + assertEquals(2, inst.unmatchedLoadedConfigs.size()); + + UsbCameraInfo info2 = new UsbCameraInfo(0, "dev/video1", "testVideo", new String[0], 1, 2); - UsbCameraInfo info2 = - new UsbCameraInfo(0, "dev/video1", "secondTestVideo", new String[0], 2, 1); infoList.add(info2); + + var cams = inst.matchUSBCameras(infoList, inst.unmatchedLoadedConfigs); + + // assertEquals("testVideo (1)", cams.get(0).uniqueName); // Proper suffixing + inst.tryMatchUSBCamImpl(false); assertTrue(inst.knownUsbCameras.contains(info2)); - assertEquals(2, inst.knownUsbCameras.size()); + assertEquals(2, inst.unmatchedLoadedConfigs.size()); + + UsbCameraInfo info3 = + new UsbCameraInfo(0, "dev/video2", "secondTestVideo", new String[] {"by-id/123"}, 2, 1); + + UsbCameraInfo info4 = + new UsbCameraInfo(0, "dev/video3", "secondTestVideo", new String[] {"by-id/321"}, 3, 1); + + infoList.add(info4); + + cams = inst.matchUSBCameras(infoList, inst.unmatchedLoadedConfigs); + + var cam4 = + cams.stream() + .filter( + cam -> cam.otherPaths.length > 0 && cam.otherPaths[0].equals(config4.otherPaths[0])) + .findFirst() + .orElse(null); + // If this is null, cam4 got matched to config3 instead of config4 + + assertEquals(cam4.nickname, config4.nickname); + + infoList.add(info3); + + cams = inst.matchUSBCameras(infoList, inst.unmatchedLoadedConfigs); + + inst.tryMatchUSBCamImpl(false); + + assertTrue(inst.knownUsbCameras.contains(info2)); + assertTrue(inst.knownUsbCameras.contains(info3)); + + var cam3 = + cams.stream() + .filter( + cam -> cam.otherPaths.length > 0 && cam.otherPaths[0].equals(config3.otherPaths[0])) + .findFirst() + .orElse(null); + cam4 = + cams.stream() + .filter( + cam -> cam.otherPaths.length > 0 && cam.otherPaths[0].equals(config4.otherPaths[0])) + .findFirst() + .orElse(null); + + assertEquals(cam3.nickname, config3.nickname); + assertEquals(cam4.nickname, config4.nickname); + assertEquals(4, inst.knownUsbCameras.size()); assertEquals(0, inst.unmatchedLoadedConfigs.size()); } }