Refactor MAC address detection (#1991)

Co-authored-by: Sam Freund <[email protected]>

Author: Gold856

photon-core/src/main/java/org/photonvision/common/dataflow/networktables/NetworkTablesManager.java

@@ -43,7 +43,7 @@
 import org.photonvision.common.logging.LogGroup;
 import org.photonvision.common.logging.LogLevel;
 import org.photonvision.common.logging.Logger;
-import org.photonvision.common.networking.NetworkManager;
+import org.photonvision.common.networking.NetworkUtils;
 import org.photonvision.common.scripting.ScriptEventType;
 import org.photonvision.common.scripting.ScriptManager;
 import org.photonvision.common.util.TimedTaskManager;
@@ -58,6 +58,7 @@ public class NetworkTablesManager {
     public final String kCoprocTableName = "coprocessors";
     private final String kFieldLayoutName = "apriltag_field_layout";
     public final NetworkTable kRootTable = ntInstance.getTable(kRootTableName);
+    public final NetworkTable kCoprocTable = kRootTable.getSubTable(kCoprocTableName);
 
     // This is used to subscribe to all coprocessor tables, so we can detect conflicts
     @SuppressWarnings("unused")
@@ -69,6 +70,7 @@ public class NetworkTablesManager {
 
     public boolean conflictingHostname = false;
     public String conflictingCameras = "";
+    private String currentMacAddress;
 
     private boolean m_isRetryingConnection = false;
 
@@ -235,8 +237,12 @@ private void broadcastVersion() {
      * this table.
      */
     private void checkHostnameAndCameraNames() {
-        String MAC = NetworkManager.getInstance().getMACAddress();
-        if (MAC == null || MAC.isEmpty()) {
+        String mac = NetworkUtils.getMacAddress();
+        if (!mac.equals(currentMacAddress)) {
+            logger.debug("MAC address changed! New MAC address is " + mac + ", was " + currentMacAddress);
+            currentMacAddress = mac;
+        }
+        if (mac.isEmpty()) {
             logger.error("Cannot check hostname and camera names, MAC address is not set!");
             return;
         }
@@ -254,62 +260,51 @@ private void checkHostnameAndCameraNames() {
                         .map(entry -> entry.getValue().nickname)
                         .toArray(String[]::new);
 
-        // Create a subtable under the photonvision root table
-        NetworkTable coprocTable = kRootTable.getSubTable(kCoprocTableName);
-
         // Create a subtable for this coprocessor using its MAC address
-        NetworkTable macTable = coprocTable.getSubTable(MAC);
+        NetworkTable macTable = kCoprocTable.getSubTable(mac);
 
         // Publish the hostname and camera names
         macTable.getEntry("hostname").setString(hostname);
         macTable.getEntry("cameraNames").setStringArray(cameraNames);
-        logger.debug("Published hostname and camera names to NT under MAC: " + MAC);
 
         boolean conflictingHostname = false;
         StringBuilder conflictingCameras = new StringBuilder();
 
         // Check for conflicts with other coprocessors
-        for (String key : coprocTable.getSubTables()) {
+        for (String key : kCoprocTable.getSubTables()) {
             // Check that key is formatted like a MAC address
             if (!key.matches("([0-9A-F]{2}-){5}[0-9A-F]{2}")) {
                 logger.warn("Skipping non-MAC key in conflict detection: " + key);
                 continue;
             }
+            if (key.equals(mac)) { // Skip our own entry
+                continue;
+            }
+            NetworkTable otherCoprocTable = kCoprocTable.getSubTable(key);
+            String otherHostname = otherCoprocTable.getEntry("hostname").getString("");
+            String[] otherCameraNames =
+                    otherCoprocTable.getEntry("cameraNames").getStringArray(new String[0]);
+            // Check for hostname conflicts
+            if (otherHostname.equals(hostname)) {
+                logger.warn("Hostname conflict detected with coprocessor " + key + ": " + hostname);
+                conflictingHostname = true;
+            }
 
-            if (!key.equals(MAC)) { // Skip our own entry
-                NetworkTable otherCoprocTable = coprocTable.getSubTable(key);
-                String otherHostname = otherCoprocTable.getEntry("hostname").getString("");
-                String[] otherCameraNames =
-                        otherCoprocTable.getEntry("cameraNames").getStringArray(new String[0]);
-                // Check for hostname conflicts
-                if (otherHostname.equals(hostname)) {
-                    logger.warn("Hostname conflict detected with coprocessor " + key + ": " + hostname);
-                    conflictingHostname = true;
-                }
-
-                // Check for camera name conflicts
-                for (String cameraName : cameraNames) {
-                    if (Arrays.stream(otherCameraNames).anyMatch(otherName -> otherName.equals(cameraName))) {
-                        logger.warn("Camera name conflict detected: " + cameraName);
-                        conflictingCameras.append(
-                                conflictingCameras.isEmpty() ? cameraName : ", " + cameraName);
-                    }
+            // Check for camera name conflicts
+            for (String cameraName : cameraNames) {
+                if (Arrays.stream(otherCameraNames).anyMatch(otherName -> otherName.equals(cameraName))) {
+                    logger.warn("Camera name conflict detected: " + cameraName);
+                    conflictingCameras.append(conflictingCameras.isEmpty() ? cameraName : ", " + cameraName);
                 }
             }
         }
 
-        boolean hasChanged =
-                this.conflictingHostname != conflictingHostname
-                        || !this.conflictingCameras.equals(conflictingCameras.toString());
-
         // Publish the conflict status
-        if (hasChanged) {
-            DataChangeService.getInstance()
-                    .publishEvent(
-                            new OutgoingUIEvent<>(
-                                    "fullsettings",
-                                    UIPhotonConfiguration.programStateToUi(ConfigManager.getInstance().getConfig())));
-        }
+        DataChangeService.getInstance()
+                .publishEvent(
+                        new OutgoingUIEvent<>(
+                                "fullsettings",
+                                UIPhotonConfiguration.programStateToUi(ConfigManager.getInstance().getConfig())));
 
         conflictAlert.setText(
                 conflictingHostname

photon-core/src/main/java/org/photonvision/common/hardware/metrics/MetricsManager.java

@@ -45,8 +45,8 @@ public class MetricsManager {
 
     ProtobufPublisher<DeviceMetrics> metricPublisher =
             NetworkTablesManager.getInstance()
-                    .kRootTable
-                    .getSubTable("/" + NetworkTablesManager.getInstance().kCoprocTableName + "/metrics")
+                    .kCoprocTable
+                    .getSubTable("/metrics")
                     .getProtobufTopic(CameraServerJNI.getHostname(), DeviceMetrics.proto)
                     .publish();
 

photon-core/src/main/java/org/photonvision/common/networking/NetworkManager.java

@@ -179,35 +179,6 @@ private void setHostname(String hostname) {
         }
     }
 
-    public String getMACAddress() {
-        var config = ConfigManager.getInstance().getConfig().getNetworkConfig();
-        if (config.networkManagerIface == null || config.networkManagerIface.isBlank()) {
-            logger.error("No network interface configured, cannot get MAC address!");
-            return "";
-        }
-        try {
-            NetworkInterface iFace = NetworkInterface.getByName(config.networkManagerIface);
-            if (iFace == null) {
-                logger.error("Network interface " + config.networkManagerIface + " not found!");
-                return "";
-            }
-            byte[] mac = iFace.getHardwareAddress();
-            if (mac == null) {
-                logger.error("No MAC address found for " + config.networkManagerIface);
-                return "";
-            }
-            StringBuilder sb = new StringBuilder(17);
-            for (byte b : mac) {
-                sb.append(String.format("%02X-", b));
-            }
-            sb.setLength(sb.length() - 1);
-            return sb.toString();
-        } catch (Exception e) {
-            logger.error("Error getting MAC address for " + config.networkManagerIface, e);
-            return "";
-        }
-    }
-
     private void setConnectionDHCP(NetworkConfig config) {
         String connName = "dhcp-" + config.networkManagerIface;
 

photon-core/src/main/java/org/photonvision/common/networking/NetworkUtils.java

@@ -23,6 +23,7 @@
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import org.photonvision.common.configuration.ConfigManager;
 import org.photonvision.common.hardware.Platform;
 import org.photonvision.common.logging.LogGroup;
 import org.photonvision.common.logging.Logger;
@@ -203,4 +204,49 @@ public static String getIPAddresses(String iFaceName) {
         }
         return String.join(", ", addresses);
     }
+
+    public static String getMacAddress() {
+        var config = ConfigManager.getInstance().getConfig().getNetworkConfig();
+        if (config.networkManagerIface == null || config.networkManagerIface.isBlank()) {
+            // This is a silly heuristic to find a network interface that PV might be using. It looks like
+            // it works pretty well, but Hyper-V adapters still show up in the list. But we're using MAC
+            // address as a semi-unique identifier, not as a source of truth, so this should be fine.
+            // Hyper-V adapters seem to show up near the end of the list anyways, so it's super likely
+            // we'll find the right adapter anyways
+            try {
+                for (var iface : NetworkInterface.networkInterfaces().toList()) {
+                    if (iface.isUp() && !iface.isVirtual() && !iface.isLoopback()) {
+                        byte[] mac = iface.getHardwareAddress();
+                        if (mac == null) {
+                            logger.error("No MAC address found for " + iface.getDisplayName());
+                        }
+                        return formatMacAddress(mac);
+                    }
+                }
+            } catch (Exception e) {
+                logger.error("Error getting MAC address:", e);
+            }
+            return "";
+        }
+        try {
+            byte[] mac = NetworkInterface.getByName(config.networkManagerIface).getHardwareAddress();
+            if (mac == null) {
+                logger.error("No MAC address found for " + config.networkManagerIface);
+                return "";
+            }
+            return formatMacAddress(mac);
+        } catch (Exception e) {
+            logger.error("Error getting MAC address for " + config.networkManagerIface, e);
+            return "";
+        }
+    }
+
+    private static String formatMacAddress(byte[] mac) {
+        StringBuilder sb = new StringBuilder(17);
+        sb.append(String.format("%02X", mac[0]));
+        for (int i = 1; i < mac.length; i++) {
+            sb.append(String.format("-%02X", mac[i]));
+        }
+        return sb.toString();
+    }
 }