From 99a14d43cc47ac0be4e657a12f4ada327673a635 Mon Sep 17 00:00:00 2001 From: MrRedness <16584585+MrRedness@users.noreply.github.com> Date: Mon, 20 Nov 2023 01:30:38 -0500 Subject: [PATCH 1/5] Initial attempt at matching by path ID --- .../configuration/SqlConfigProvider.java | 15 ++-- .../vision/camera/USBCameraSource.java | 5 +- .../vision/processes/VisionSourceManager.java | 69 +++++++++++++++---- 3 files changed, 71 insertions(+), 18 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 8a0c755ad8..3f22e69522 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 @@ static class TableKeys { 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 @@ private void initDatabase() { + " 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 @@ private void saveCameras(Connection conn) { 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 @@ private void saveCameras(Connection conn) { 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 @@ private void saveCameras(Connection conn) { }) .filter(Objects::nonNull) .collect(Collectors.toList()); - statement.setString(4, JacksonUtils.serializeToString(settings)); + statement.setString(5, JacksonUtils.serializeToString(settings)); statement.executeUpdate(); } @@ -455,10 +458,11 @@ private HashMap loadCameraConfigs(Connection conn) 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 @@ private HashMap loadCameraConfigs(Connection conn) 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 @@ private HashMap loadCameraConfigs(Connection conn) 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/camera/USBCameraSource.java b/photon-core/src/main/java/org/photonvision/vision/camera/USBCameraSource.java index 6717f4edd8..d23f14aa67 100644 --- a/photon-core/src/main/java/org/photonvision/vision/camera/USBCameraSource.java +++ b/photon-core/src/main/java/org/photonvision/vision/camera/USBCameraSource.java @@ -346,8 +346,9 @@ public boolean isVendorCamera() { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - USBCameraSource that = (USBCameraSource) o; - return cameraQuirks.equals(that.cameraQuirks); + // USBCameraSource that = (USBCameraSource) o; + // return cameraQuirks.equals(that.cameraQuirks); + return false; } @Override 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 4b31ba0691..eee73ff550 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 @@ -201,6 +201,8 @@ private List matchUSBCameras( var detectedCameraList = new ArrayList<>(detectedCamInfos); ArrayList cameraConfigurations = new ArrayList<>(); + List unmatchedUsbConfigs = new CopyOnWriteArrayList<>(loadedUsbCamConfigs); + // loop over all the configs loaded from disk for (CameraConfiguration config : loadedUsbCamConfigs) { UsbCameraInfo cameraInfo; @@ -215,22 +217,34 @@ private List matchUSBCameras( detectedCameraList.stream() .filter( usbCameraInfo -> - usbCameraInfo.path.equals(config.path) + usbCameraInfo.otherPaths.length != 0 + && config.otherPaths.length != 0 + && usbCameraInfo.otherPaths[0].equals(config.otherPaths[0]) && 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"); - 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); + unmatchedUsbConfigs.remove(config); + cameraConfigurations.add(mergeInfoIntoConfig(config, cameraInfo)); } + } + + // if path based fails, attempt basename only match + for (CameraConfiguration config : unmatchedUsbConfigs) { + UsbCameraInfo cameraInfo; + + logger.debug("Failed to match by path and name, falling back to name-only match"); + 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 @@ -250,7 +264,7 @@ && cameraNameToBaseName(usbCameraInfo.name).equals(config.baseName)) 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 +297,27 @@ private CameraConfiguration mergeInfoIntoConfig(CameraConfiguration cfg, UsbCame 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; } @@ -363,4 +398,14 @@ private boolean containsName( 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 knownUsbCameras.stream().anyMatch(camera -> camera.name.equals(uniqueName)); + } } From 86cb628155708cc2db66826a4160535e824e925f Mon Sep 17 00:00:00 2001 From: MrRedness <16584585+MrRedness@users.noreply.github.com> Date: Mon, 20 Nov 2023 09:54:23 -0500 Subject: [PATCH 2/5] Added fallback to normal path matching (for windows) --- .../vision/processes/VisionSourceManager.java | 139 ++++++++++++------ 1 file changed, 94 insertions(+), 45 deletions(-) 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 eee73ff550..468d5c618e 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 @@ -201,61 +201,108 @@ private List matchUSBCameras( var detectedCameraList = new ArrayList<>(detectedCamInfos); ArrayList cameraConfigurations = new ArrayList<>(); - List unmatchedUsbConfigs = new CopyOnWriteArrayList<>(loadedUsbCamConfigs); + List unmatchedAfterByID = new ArrayList<>(loadedUsbCamConfigs); - // loop over all the configs loaded from disk + // 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.otherPaths.length != 0 - && config.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); - unmatchedUsbConfigs.remove(config); - cameraConfigurations.add(mergeInfoIntoConfig(config, cameraInfo)); + 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 -> + 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); + unmatchedAfterByID.remove(config); + cameraConfigurations.add(mergeInfoIntoConfig(config, cameraInfo)); + } } } - // if path based fails, attempt basename only match - for (CameraConfiguration config : unmatchedUsbConfigs) { - UsbCameraInfo cameraInfo; + 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)); + } + } - logger.debug("Failed to match by path and name, falling back to name-only match"); - 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 (!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. + // 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) { @@ -406,6 +453,8 @@ private boolean containsName( * @return If the list of cameras contains the unique name. */ private boolean containsName(final String uniqueName) { - return knownUsbCameras.stream().anyMatch(camera -> camera.name.equals(uniqueName)); + + return VisionModuleManager.getInstance().getModules().stream() + .anyMatch(camera -> camera.visionSource.cameraConfiguration.uniqueName.equals(uniqueName)); } } From 8460fabbdd3b4ed9b41f0a0c44a7a4d14c16ade6 Mon Sep 17 00:00:00 2001 From: MrRedness <16584585+MrRedness@users.noreply.github.com> Date: Mon, 20 Nov 2023 09:56:10 -0500 Subject: [PATCH 3/5] WPIformatting fixes --- .../org/photonvision/vision/processes/VisionSourceManager.java | 2 -- 1 file changed, 2 deletions(-) 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 468d5c618e..4ea5185790 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 @@ -211,7 +211,6 @@ private List matchUSBCameras( 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 " @@ -453,7 +452,6 @@ private boolean containsName( * @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)); } From 8ca99bf4dd97a81bce5c8ee9e91f79bdad6fab75 Mon Sep 17 00:00:00 2001 From: MrRedness <16584585+MrRedness@users.noreply.github.com> Date: Mon, 20 Nov 2023 16:05:37 -0500 Subject: [PATCH 4/5] Windows bug fix (dev is not constant) --- .../org/photonvision/vision/processes/VisionSourceManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 4ea5185790..78f5065c0f 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 @@ -390,7 +390,7 @@ private List filterAllowedDevices(List allDevices) 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; From b7bd586e6cf67c6d59d858254feeb5c0fec06532 Mon Sep 17 00:00:00 2001 From: MrRedness <16584585+MrRedness@users.noreply.github.com> Date: Mon, 20 Nov 2023 17:34:58 -0500 Subject: [PATCH 5/5] match-by-id VisionSourceManager unit testing --- .../vision/processes/VisionSourceManager.java | 2 +- .../processes/VisionSourceManagerTest.java | 81 +++++++++++++++++-- 2 files changed, 75 insertions(+), 8 deletions(-) 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 78f5065c0f..dbec0f0ed4 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,7 +196,7 @@ protected List tryMatchUSBCamImpl(boolean createSources) { * @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<>(); 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 2c64a9a622..e16213000e 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 void visionSourceTest() { 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.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)); - assertEquals(2, inst.knownUsbCameras.size()); + 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()); } }