Improve network management (#1478)

This PR changes the way that photonvision interacts with nmcli to control networking on the coprocessor. Instead of modifying an existing connection, Photonvision adds new connections for DHCP and Static IP configurations. It then activiates the proper one at startup and any time that the network configuration is changed.


It also now uses the interface name and not the connection name and checks that the interface is available before making any changes. If the saved interface is not found, it updates the stored interface name and applies the network settings to the current interface. This should minimize the failure to control the network if the network interface wasn't available when PhotonVision first booted.

One other benefit of not altering the default configuration is that, if PhotonVision fails to run for any reason, the device can be accessed using the original networking configuration.

The code has been tested on an OrangePi5 and and a Raspberry Pi 4.

Addresses: #1261

photon-client/src/stores/settings/GeneralSettingsStore.ts

@@ -78,7 +78,7 @@ export const useSettingsStore = defineStore("settings", {
       return this.general.gpuAcceleration !== undefined;
     },
     networkInterfaceNames(): string[] {
-      return this.network.networkInterfaceNames.map((i) => i.connName);
+      return this.network.networkInterfaceNames.map((i) => i.devName);
     }
   },
   actions: {

photon-core/src/main/java/org/photonvision/common/configuration/NetworkConfig.java

@@ -26,7 +26,6 @@ import java.util.HashMap;
 import java.util.Map;
 import org.photonvision.common.hardware.Platform;
 import org.photonvision.common.networking.NetworkMode;
-import org.photonvision.common.networking.NetworkUtils;
 import org.photonvision.common.util.file.JacksonUtils;
 
 public class NetworkConfig {
@@ -50,23 +49,14 @@ public class NetworkConfig {
     @JsonIgnore public static final String NM_IFACE_STRING = "${interface}";
     @JsonIgnore public static final String NM_IP_STRING = "${ipaddr}";
 
-    public String networkManagerIface;
+    public String networkManagerIface = "";
+    // TODO: remove these strings if no longer needed
     public String setStaticCommand =
             "nmcli con mod ${interface} ipv4.addresses ${ipaddr}/8 ipv4.method \"manual\" ipv6.method \"disabled\"";
     public String setDHCPcommand =
             "nmcli con mod ${interface} ipv4.method \"auto\" ipv6.method \"disabled\"";
 
     public NetworkConfig() {
-        if (Platform.isLinux()) {
-            // Default to the name of the first Ethernet connection. Otherwise, "Wired connection 1" is a
-            // reasonable guess
-            this.networkManagerIface =
-                    NetworkUtils.getAllWiredInterfaces().stream()
-                            .map(it -> it.connName)
-                            .findFirst()
-                            .orElse("Wired connection 1");
-        }
-
         // We can (usually) manage networking on Linux devices, and if we can, we should try to. Command
         // line inhibitions happen at a level above this class
         setShouldManage(deviceCanManageNetwork());
@@ -112,7 +102,7 @@ public class NetworkConfig {
 
     @JsonIgnore
     public String getPhysicalInterfaceName() {
-        return NetworkUtils.getNMinfoForConnName(this.networkManagerIface).devName;
+        return this.networkManagerIface;
     }
 
     @JsonIgnore

photon-core/src/main/java/org/photonvision/common/configuration/PhotonConfiguration.java

@@ -120,7 +120,7 @@ public class PhotonConfiguration {
 
         // Hack active interfaces into networkSettings
         var netConfigMap = networkConfig.toHashMap();
-        netConfigMap.put("networkInterfaceNames", NetworkUtils.getAllWiredInterfaces());
+        netConfigMap.put("networkInterfaceNames", NetworkUtils.getAllActiveWiredInterfaces());
         netConfigMap.put("networkingDisabled", NetworkManager.getInstance().networkingIsDisabled);
 
         settingsSubmap.put("networkSettings", netConfigMap);

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

@@ -17,6 +17,10 @@
 
 package org.photonvision.common.networking;
 
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.NoSuchElementException;
+
 import org.photonvision.common.configuration.ConfigManager;
 import org.photonvision.common.configuration.NetworkConfig;
 import org.photonvision.common.dataflow.DataChangeDestination;
@@ -27,7 +31,9 @@ import org.photonvision.common.hardware.Platform;
 import org.photonvision.common.hardware.PlatformUtils;
 import org.photonvision.common.logging.LogGroup;
 import org.photonvision.common.logging.Logger;
+import org.photonvision.common.networking.NetworkUtils.NMDeviceInfo;
 import org.photonvision.common.util.ShellExec;
+import org.photonvision.common.util.TimedTaskManager;
 
 public class NetworkManager {
     private static final Logger logger = new Logger(NetworkManager.class, LogGroup.General);
@@ -52,102 +58,60 @@ public class NetworkManager {
             return;
         }
 
+        if (!Platform.isLinux()) {
+            logger.info("Not managing network on non-Linux platforms.");
+            return;
+        }
+
+        if (!PlatformUtils.isRoot()) {
+            logger.error("Cannot manage network without root!");
+            return;
+        }
+
+        // Start tasks to monitor the network interface(s)
+        var ethernetDevices = NetworkUtils.getAllWiredInterfaces();
+        for (NMDeviceInfo deviceInfo : ethernetDevices) {
+            var task = "deviceStatus-" + deviceInfo.devName;
+            if (!TimedTaskManager.getInstance().taskActive(task)) {
+                TimedTaskManager.getInstance().addTask(task, deviceStatus(deviceInfo.devName), 5000);
+            }
+        }
+
+        var physicalDevices = NetworkUtils.getAllActiveWiredInterfaces();
         var config = ConfigManager.getInstance().getConfig().getNetworkConfig();
-        logger.info("Setting " + config.connectionType + " with team " + config.ntServerAddress);
-        if (Platform.isLinux()) {
-            if (!PlatformUtils.isRoot()) {
-                logger.error("Cannot manage hostname without root!");
-            }
-
-            // always set hostname
-            if (!config.hostname.isEmpty()) {
-                try {
-                    var shell = new ShellExec(true, false);
-                    shell.executeBashCommand("cat /etc/hostname | tr -d \" \\t\\n\\r\"");
-                    var oldHostname = shell.getOutput().replace("\n", "");
-
-                    var setHostnameRetCode =
-                            shell.executeBashCommand(
-                                    "echo $NEW_HOSTNAME > /etc/hostname".replace("$NEW_HOSTNAME", config.hostname));
-                    setHostnameRetCode =
-                            shell.executeBashCommand("hostnamectl set-hostname " + config.hostname);
-
-                    // Add to /etc/hosts
-                    var addHostRetCode =
-                            shell.executeBashCommand(
-                                    String.format(
-                                            "sed -i \"s/127.0.1.1.*%s/127.0.1.1\\t%s/g\" /etc/hosts",
-                                            oldHostname, config.hostname));
-
-                    shell.executeBashCommand("sudo service avahi-daemon restart");
-
-                    var success = setHostnameRetCode == 0 && addHostRetCode == 0;
-                    if (!success) {
-                        logger.error(
-                                "Setting hostname returned non-zero codes (hostname/hosts) "
-                                        + setHostnameRetCode
-                                        + "|"
-                                        + addHostRetCode
-                                        + "!");
-                    } else {
-                        logger.info("Set hostname to " + config.hostname);
-                    }
-                } catch (Exception e) {
-                    logger.error("Failed to set hostname!", e);
-                }
-
-            } else {
-                logger.warn("Got empty hostname?");
-            }
-
-            if (config.connectionType == NetworkMode.DHCP) {
-                var shell = new ShellExec();
-                try {
-                    // set nmcli back to DHCP, and re-run dhclient -- this ought to grab a new IP address
-                    shell.executeBashCommand(
-                            config.setDHCPcommand.replace(
-                                    NetworkConfig.NM_IFACE_STRING, config.getEscapedInterfaceName()));
-                    shell.executeBashCommand("dhclient " + config.getPhysicalInterfaceName(), false);
-                } catch (Exception e) {
-                    logger.error("Exception while setting DHCP!");
-                }
-            } else if (config.connectionType == NetworkMode.STATIC) {
-                var shell = new ShellExec();
-                if (!config.staticIp.isEmpty()) {
-                    try {
-                        shell.executeBashCommand(
-                                config
-                                        .setStaticCommand
-                                        .replace(NetworkConfig.NM_IFACE_STRING, config.getEscapedInterfaceName())
-                                        .replace(NetworkConfig.NM_IP_STRING, config.staticIp));
-
-                        if (Platform.isRaspberryPi()) {
-                            // Pi's need to manually have their interface adjusted?? and the 5-second sleep is
-                            // integral in my testing (Matt)
-                            shell.executeBashCommand(
-                                    "sh -c 'nmcli con down "
-                                            + config.getEscapedInterfaceName()
-                                            + "; nmcli con up "
-                                            + config.getEscapedInterfaceName()
-                                            + "'");
-                        } else {
-                            // for now just bring down /up -- more testing needed on beelink et al.
-                            shell.executeBashCommand(
-                                    "sh -c 'nmcli con down "
-                                            + config.getEscapedInterfaceName()
-                                            + "; nmcli con up "
-                                            + config.getEscapedInterfaceName()
-                                            + "'");
-                        }
-                    } catch (Exception e) {
-                        logger.error("Error while setting static IP!", e);
-                    }
-                } else {
-                    logger.warn("Got empty static IP?");
+        if (physicalDevices.stream().noneMatch(it -> (it.devName.equals(config.networkManagerIface)))) {
+            try {
+                // if the configured interface isn't in the list of available ones, select one that is
+                var iFace = physicalDevices.stream().findFirst().orElseThrow();
+                logger.warn("The configured interface doesn't match any available interface. Applying configuration to " + iFace.devName);
+                // update NetworkConfig with found interface
+                config.networkManagerIface = iFace.devName;
+                ConfigManager.getInstance().requestSave();
+            } catch (NoSuchElementException e) {
+                // if there are no available interfaces, go with the one from settings
+                logger.warn("No physical interface found. Maybe ethernet isn't connected?");
+                if (config.networkManagerIface == null || config.networkManagerIface.isBlank()) {
+                    // if it's also empty, there is nothing to configure
+                    logger.error("No valid network interfaces to manage");
+                    // TODO: add a thread that monitors the network
+                    return;
                 }
             }
+        }
+
+        logger.info("Setting " + config.connectionType + " with team " + config.ntServerAddress + " on " + config.networkManagerIface);
+
+        // always set hostname (unless it's blank)
+        if (!config.hostname.isBlank()) {
+            setHostname(config.hostname);
         } else {
-            logger.info("Not managing network on non-Linux platforms");
+            logger.warn("Got empty hostname?");
+        }
+
+        if (config.connectionType == NetworkMode.DHCP) {
+            setConnectionDHCP(config);
+        } else if (config.connectionType == NetworkMode.STATIC) {
+            setConnectionStatic(config);
         }
     }
 
@@ -162,4 +126,162 @@ public class NetworkManager {
                                 "restartServer",
                                 true));
     }
+
+    private void setHostname(String hostname) {
+        try {
+            var shell = new ShellExec(true, false);
+            shell.executeBashCommand("cat /etc/hostname | tr -d \" \\t\\n\\r\"");
+            var oldHostname = shell.getOutput().replace("\n", "");
+            logger.debug("Old host name: >" + oldHostname +"<");
+            logger.debug("New host name: >" + hostname +"<");
+
+            if (!oldHostname.equals(hostname)) {
+                var setHostnameRetCode =
+                        shell.executeBashCommand(
+                                "echo $NEW_HOSTNAME > /etc/hostname".replace("$NEW_HOSTNAME", hostname));
+                setHostnameRetCode =
+                        shell.executeBashCommand("hostnamectl set-hostname " + hostname);
+
+                // Add to /etc/hosts
+                var addHostRetCode =
+                        shell.executeBashCommand(
+                                String.format(
+                                        "sed -i \"s/127.0.1.1.*%s/127.0.1.1\\t%s/g\" /etc/hosts",
+                                        oldHostname, hostname));
+
+                shell.executeBashCommand("systemctl restart avahi-daemon.service");
+
+                var success = setHostnameRetCode == 0 && addHostRetCode == 0;
+                if (!success) {
+                    logger.error(
+                            "Setting hostname returned non-zero codes (hostname/hosts) "
+                                    + setHostnameRetCode
+                                    + "|"
+                                    + addHostRetCode
+                                    + "!");
+                } else {
+                    logger.info("Set hostname to " + hostname);
+                }
+            }
+        } catch (Exception e) {
+            logger.error("Failed to set hostname!", e);
+        }
+    }
+
+    private void setConnectionDHCP(NetworkConfig config) {
+        String connName = "dhcp-" + config.networkManagerIface;
+
+        String addDHCPcommand = """
+            nmcli connection add
+            con-name "${connection}"
+            ifname "${interface}"
+            type ethernet
+            autoconnect no
+            ipv4.method auto
+            ipv6.method disabled
+            """;
+        addDHCPcommand = addDHCPcommand.replaceAll("[\\n]", " ");
+
+        var shell = new ShellExec();
+        try {
+            // set nmcli back to DHCP, and re-run dhclient -- this ought to grab a new IP address
+            if (NetworkUtils.connDoesNotExist(connName)) {
+                // create connection
+                logger.info("Creating the DHCP connection " + connName );
+                shell.executeBashCommand(
+                    addDHCPcommand
+                        .replace("${connection}", connName)
+                        .replace("${interface}", config.networkManagerIface)
+                    );
+            }
+            // activate it
+            logger.info("Activating the DHCP connection " + connName );
+            shell.executeBashCommand("nmcli connection up \"${connection}\"".replace("${connection}", connName), false);
+        } catch (Exception e) {
+            logger.error("Exception while setting DHCP!", e);
+        }
+    }
+
+    private void setConnectionStatic(NetworkConfig config) {
+        String connName = "static-" + config.networkManagerIface;
+        String addStaticCommand = """
+            nmcli connection add
+            con-name "${connection}"
+            ifname "${interface}"
+            type ethernet
+            autoconnect no
+            ipv4.addresses ${ipaddr}/8
+            ipv4.gateway ${gateway}
+            ipv4.method "manual"
+            ipv6.method "disabled"
+            """;
+        addStaticCommand = addStaticCommand.replaceAll("[\\n]", " ");
+
+        String modStaticCommand = "nmcli connection mod \"${connection}\" ipv4.addresses ${ipaddr}/8 ipv4.gateway ${gateway}";
+
+        if (config.staticIp.isBlank()) {
+            logger.warn("Got empty static IP?");
+            return;
+        }
+
+        // guess at the gateway from the staticIp
+        String[] parts = config.staticIp.split("\\.");
+        parts[parts.length-1] = "1";
+        String gateway = String.join(".", parts);
+
+        var shell = new ShellExec();
+        try {
+            // set nmcli back to DHCP, and re-run dhclient -- this ought to grab a new IP address
+            if (NetworkUtils.connDoesNotExist(connName)) {
+                // create connection
+                logger.info("Creating the Static connection " + connName );
+                shell.executeBashCommand(
+                    addStaticCommand
+                        .replace("${connection}", connName)
+                        .replace("${interface}", config.networkManagerIface)
+                        .replace("${ipaddr}", config.staticIp)
+                        .replace("${gateway}", gateway)
+                    );
+            } else {
+                // modify it in case the static IP address is different
+                logger.info("Modifying the Static connection " + connName );
+                shell.executeBashCommand(
+                    modStaticCommand
+                        .replace("${connection}", connName)
+                        .replace("${ipaddr}", config.staticIp)
+                        .replace("${gateway}", gateway)
+                    );
+            }
+            // activate it
+            logger.info("Activating the Static connection " + connName );
+            shell.executeBashCommand("nmcli connection up \"${connection}\"".replace("${connection}", connName), false);
+        } catch (Exception e) {
+            logger.error("Error while setting static IP!", e);
+        }
+    }
+
+    // Detects changes in the carrier and reinitializes after re-connect
+    private Runnable deviceStatus(String devName) {
+        Path file = Path.of("/sys/class/net/{device}/carrier".replace("{device}", devName));
+        logger.debug("Watching network interface at path: " + file.toString());
+        var last = new Object() {boolean carrier = true;};
+        return () ->
+        {
+            try {
+                boolean carrier = Files.readString(file).trim().equals("1");
+                if (carrier != last.carrier) {
+                    if (carrier) {
+                        // carrier came back
+                        logger.info("Interface " + devName + " has re-connected, reinitializing");
+                        reinitialize();
+                    } else {
+                        logger.warn("Interface " + devName + " is disconnected, check Ethernet!");
+                    }
+                }
+                last.carrier = carrier;
+            } catch (Exception e) {
+                logger.error("Could not check network status", e);
+            }
+        };
+    }
 }

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

@@ -102,13 +102,16 @@ public class NetworkUtils {
                     Pattern.compile("GENERAL.CONNECTION:(.*)\nGENERAL.DEVICE:(.*)\nGENERAL.TYPE:(.*)");
             Matcher matcher = pattern.matcher(out);
             while (matcher.find()) {
-                ret.add(new NMDeviceInfo(matcher.group(1), matcher.group(2), matcher.group(3)));
+                if (!matcher.group(2).equals("lo")) {
+                    // only include non-loopback devices
+                    ret.add(new NMDeviceInfo(matcher.group(1), matcher.group(2), matcher.group(3)));
+                }
             }
         } catch (IOException e) {
-            logger.error("Could not get active NM ifaces!", e);
+            logger.error("Could not get active network interfaces!", e);
         }
 
-        logger.debug("Found network interfaces:\n" + ret);
+        logger.debug("Found network interfaces: " + ret);
 
         allInterfaces = ret;
         return ret;
@@ -123,8 +126,14 @@ public class NetworkUtils {
     }
 
     public static List<NMDeviceInfo> getAllWiredInterfaces() {
-        return getAllActiveInterfaces().stream()
-                .filter(it -> it.nmType == NMType.NMTYPE_ETHERNET)
+        return getAllInterfaces().stream()
+                .filter(it -> it.nmType.equals(NMType.NMTYPE_ETHERNET))
+                .collect(Collectors.toList());
+    }
+
+    public static List<NMDeviceInfo> getAllActiveWiredInterfaces() {
+        return getAllWiredInterfaces().stream()
+                .filter(it -> !it.connName.isBlank())
                 .collect(Collectors.toList());
     }
 
@@ -136,4 +145,26 @@ public class NetworkUtils {
         }
         return null;
     }
+
+    public static NMDeviceInfo getNMinfoForDevName(String devName) {
+        for (NMDeviceInfo info : getAllActiveInterfaces()) {
+            if (info.devName.equals(devName)) {
+                return info;
+            }
+        }
+        logger.warn("Could not find a match for network device " + devName);
+        return null;
+    }
+
+    public static boolean connDoesNotExist(String connName) {
+        var shell = new ShellExec(true, true);
+        try {
+            // set nmcli back to DHCP, and re-run dhclient -- this ought to grab a new IP address
+            shell.executeBashCommand("nmcli -f GENERAL.STATE connection show \"" + connName + "\"");
+            return (shell.getExitCode() == 10);
+        } catch (Exception e) {
+            logger.error("Exception from nmcli!");
+        }
+        return false;
+    }
 }

photon-core/src/main/java/org/photonvision/common/util/TimedTaskManager.java

@@ -79,4 +79,8 @@ public class TimedTaskManager {
             activeTasks.remove(identifier);
         }
     }
+
+    public boolean taskActive(String identifier) {
+        return activeTasks.containsKey(identifier);
+    }
 }

shared/common.gradle

@@ -3,8 +3,8 @@ apply plugin: "java"
 apply plugin: "jacoco"
 
 java {
-    sourceCompatibility = JavaVersion.VERSION_11
-    targetCompatibility = JavaVersion.VERSION_11
+    sourceCompatibility = JavaVersion.VERSION_17
+    targetCompatibility = JavaVersion.VERSION_17
 }
 
 wpilibTools.deps.wpilibVersion = wpilibVersion