Fix wifi candidate selection to include non-interface-bound candidates (

#49)

cmd/viam-agent/main.go

@@ -183,7 +183,7 @@ func main() {
 	}
 
 	// wait until now when we (potentially) have a network logger to record this
-	globalLogger.Infof("Viam Agent Version: %s\nGit Revision: %s\n", agent.GetVersion(), agent.GetRevision())
+	globalLogger.Infof("Viam Agent Version: %s Git Revision: %s", agent.GetVersion(), agent.GetRevision())
 
 	// if FastStart is set, skip updates and start viam-server immediately, then proceed as normal
 	var fastSuccess bool

subsystems/provisioning/definitions.go

@@ -37,8 +37,6 @@ const (
 	NetworkTypeWired             = "wired"
 	NetworkTypeHotspot           = "hotspot"
 
-	IfNameAny = "any"
-
 	HealthCheckTimeout = time.Minute
 )
 

subsystems/provisioning/generators.go

@@ -2,7 +2,6 @@ package provisioning
 
 import (
 	"encoding/binary"
-	"fmt"
 	"net"
 	"regexp"
 	"strconv"
@@ -178,14 +177,3 @@ func generateAddress(addr string) (uint32, error) {
 
 	return binary.LittleEndian.Uint32(outBytes), nil
 }
-
-func genNetKey(ifName, ssid string) string {
-	if ifName == "" {
-		ifName = IfNameAny
-	}
-
-	if ssid == "" {
-		return ifName
-	}
-	return fmt.Sprintf("%s@%s", ssid, ifName)
-}

subsystems/provisioning/networkmanager.go

@@ -126,7 +126,7 @@ func (w *Provisioning) checkConnections() error {
 		state, err := activeConnection.GetPropertyState()
 		nw.mu.Lock()
 		if err != nil {
-			w.logger.Error(errw.Wrapf(err, "getting state of active connection: %s", genNetKey(ifName, ssid)))
+			w.logger.Error(errw.Wrapf(err, "getting state of active connection: %s", w.netState.GenNetKey(ifName, ssid)))
 			w.netState.SetActiveConn(ifName, nil)
 			w.netState.SetActiveSSID(ifName, "")
 			nw.connected = false
@@ -221,16 +221,11 @@ func (w *Provisioning) activateConnection(ctx context.Context, ifName, ssid stri
 	nw.mu.Lock()
 	defer nw.mu.Unlock()
 
-	if nw.conn == nil && ssid != "" {
-		nw = w.netState.LockingNetwork(IfNameAny, ssid)
-		nw.mu.Lock()
-		defer nw.mu.Unlock()
-		if nw.conn == nil {
-			return errw.Errorf("no settings found for network: %s", genNetKey(ifName, ssid))
-		}
+	if nw.conn == nil {
+		return errw.Errorf("no settings found for network: %s", w.netState.GenNetKey(ifName, ssid))
 	}
 
-	w.logger.Infof("Activating connection: %s", genNetKey(ifName, ssid))
+	w.logger.Infof("Activating connection: %s", w.netState.GenNetKey(ifName, ssid))
 
 	var netDev gnm.Device
 	if nw.netType == NetworkTypeWifi || nw.netType == NetworkTypeHotspot {
@@ -253,7 +248,7 @@ func (w *Provisioning) activateConnection(ctx context.Context, ifName, ssid stri
 	activeConnection, err := w.nm.ActivateConnection(nw.conn, netDev, nil)
 	if err != nil {
 		nw.lastError = err
-		return errw.Wrapf(err, "activating connection: %s", genNetKey(ifName, ssid))
+		return errw.Wrapf(err, "activating connection: %s", w.netState.GenNetKey(ifName, ssid))
 	}
 
 	if err := w.waitForConnect(ctx, netDev); err != nil {
@@ -267,7 +262,7 @@ func (w *Provisioning) activateConnection(ctx context.Context, ifName, ssid stri
 	nw.lastError = nil
 	w.netState.SetActiveConn(ifName, activeConnection)
 
-	w.logger.Infof("Successfully activated connection: %s", genNetKey(ifName, ssid))
+	w.logger.Infof("Successfully activated connection: %s", w.netState.GenNetKey(ifName, ssid))
 
 	if nw.netType != NetworkTypeHotspot {
 		w.netState.SetActiveSSID(ifName, ssid)
@@ -300,14 +295,14 @@ func (w *Provisioning) deactivateConnection(ifName, ssid string) error {
 	nw.mu.Lock()
 	defer nw.mu.Unlock()
 
-	w.logger.Infof("Deactivating connection: %s", genNetKey(ifName, ssid))
+	w.logger.Infof("Deactivating connection: %s", w.netState.GenNetKey(ifName, ssid))
 
 	if err := w.nm.DeactivateConnection(activeConn); err != nil {
 		nw.lastError = err
-		return errw.Wrapf(err, "deactivating connection: %s", genNetKey(ifName, ssid))
+		return errw.Wrapf(err, "deactivating connection: %s", w.netState.GenNetKey(ifName, ssid))
 	}
 
-	w.logger.Infof("Successfully deactivated connection: %s", genNetKey(ifName, ssid))
+	w.logger.Infof("Successfully deactivated connection: %s", w.netState.GenNetKey(ifName, ssid))
 
 	if ifName == w.Config().HotspotInterface {
 		w.connState.setConnected(false)
@@ -375,7 +370,7 @@ func (w *Provisioning) addOrUpdateConnection(cfg NetworkConfig) (bool, error) {
 		return changesMade, errors.New("wifi passwords must be at least 8 characters long, or completely empty (for unsecured networks)")
 	}
 
-	netKey := genNetKey(cfg.Interface, cfg.SSID)
+	netKey := w.netState.GenNetKey(cfg.Interface, cfg.SSID)
 	nw := w.netState.LockingNetwork(cfg.Interface, cfg.SSID)
 	nw.lastTried = time.Time{}
 	nw.priority = cfg.Priority
@@ -442,8 +437,8 @@ func (w *Provisioning) addOrUpdateConnection(cfg NetworkConfig) (bool, error) {
 // this doesn't error as it's not technically fatal if it fails.
 func (w *Provisioning) lowerMaxNetPriorities(skip string) {
 	for _, nw := range w.netState.LockingNetworks() {
-		netKey := genNetKey(nw.interfaceName, nw.ssid)
-		if netKey == skip || netKey == genNetKey(w.Config().HotspotInterface, w.Config().hotspotSSID) || nw.priority < 999 ||
+		netKey := w.netState.GenNetKey(nw.interfaceName, nw.ssid)
+		if netKey == skip || netKey == w.netState.GenNetKey(w.Config().HotspotInterface, w.Config().hotspotSSID) || nw.priority < 999 ||
 			nw.netType != NetworkTypeWifi || (nw.interfaceName != "" && nw.interfaceName != w.Config().HotspotInterface) {
 			continue
 		}
@@ -533,7 +528,6 @@ func (w *Provisioning) getCandidates(ifName string) []string {
 				return []string{nw.ssid}
 			}
 		}
-		return []string{}
 	}
 
 	// sort by priority
@@ -607,11 +601,10 @@ func (w *Provisioning) mainLoop(ctx context.Context) {
 					priority = 100
 				}
 				cfg := NetworkConfig{
-					Type:      NetworkTypeWifi,
-					SSID:      userInput.SSID,
-					PSK:       userInput.PSK,
-					Priority:  priority,
-					Interface: w.Config().HotspotInterface,
+					Type:     NetworkTypeWifi,
+					SSID:     userInput.SSID,
+					PSK:      userInput.PSK,
+					Priority: priority,
 				}
 				var err error
 				changesMade, err = w.AddOrUpdateConnection(cfg)
@@ -652,7 +645,7 @@ func (w *Provisioning) mainLoop(ctx context.Context) {
 						nw.lastError = err
 						w.logger.Warn(err)
 					} else {
-						w.logger.Error("cannot find %s in network list", genNetKey("", newSSID))
+						w.logger.Error("cannot find %s in network list", w.netState.GenNetKey("", newSSID))
 					}
 					nw.mu.Unlock()
 					err = w.StartProvisioning(ctx, inputChan)
@@ -691,7 +684,7 @@ func (w *Provisioning) mainLoop(ctx context.Context) {
 
 		w.logger.Debugf("wifi: %t (%s), internet: %t, config present: %t",
 			isConnected,
-			genNetKey(w.Config().HotspotInterface, w.netState.ActiveSSID(w.Config().HotspotInterface)),
+			w.netState.GenNetKey(w.Config().HotspotInterface, w.netState.ActiveSSID(w.Config().HotspotInterface)),
 			isOnline,
 			isConfigured,
 		)
@@ -717,6 +710,7 @@ func (w *Provisioning) mainLoop(ctx context.Context) {
 					w.logger.Error(err)
 				} else {
 					pMode = w.connState.getProvisioning()
+					pModeChange = w.connState.getProvisioningChange()
 				}
 			}
 		}
@@ -743,9 +737,11 @@ func (w *Provisioning) mainLoop(ctx context.Context) {
 			}
 		}
 
+		hitOfflineTimeout := lastConnectivity.Before(now.Add(time.Duration(w.cfg.OfflineTimeout)*-1)) &&
+			pModeChange.Before(now.Add(time.Duration(w.cfg.OfflineTimeout)*-1))
 		// not in provisioning mode, so start it if not configured (/etc/viam.json)
-		// OR as long as we've been offline for at least OfflineTimeout (2 minute default)
-		if !isConfigured || lastConnectivity.Before(now.Add(time.Duration(w.cfg.OfflineTimeout)*-1)) {
+		// OR as long as we've been offline AND out of provisioning mode for at least OfflineTimeout (2 minute default)
+		if !isConfigured || hitOfflineTimeout {
 			if err := w.StartProvisioning(ctx, inputChan); err != nil {
 				w.logger.Error(err)
 			}

subsystems/provisioning/networkstate.go

@@ -1,6 +1,7 @@
 package provisioning
 
 import (
+	"fmt"
 	"sync"
 
 	gnm "github.com/Otterverse/gonetworkmanager/v2"
@@ -11,6 +12,9 @@ type networkState struct {
 	mu     sync.RWMutex
 	logger logging.Logger
 
+	// the wifi interface to default to when no interface is specified
+	hotspotInterface string
+
 	// key is ssid@interface for wifi, ex: TestNetwork@wlan0
 	// interface may be "any" for no interface set, ex: TestNetwork@any
 	// wired networks are just interface, ex: eth0
@@ -39,13 +43,36 @@ func NewNetworkState(logger logging.Logger) *networkState {
 	}
 }
 
+func (n *networkState) SetHotspotInterface(iface string) {
+	n.mu.Lock()
+	defer n.mu.Unlock()
+	n.hotspotInterface = iface
+}
+
+func (n *networkState) GenNetKey(ifName, ssid string) string {
+	n.mu.Lock()
+	defer n.mu.Unlock()
+	return n.genNetKey(ifName, ssid)
+}
+
+func (n *networkState) genNetKey(ifName, ssid string) string {
+	if ifName == "" && ssid != "" {
+		ifName = n.hotspotInterface
+	}
+
+	if ssid == "" {
+		return ifName
+	}
+	return fmt.Sprintf("%s@%s", ssid, ifName)
+}
+
 // LockingNetwork returns a pointer to a network, wrapped in a lockable struct, so updates are persisted
 // Users must lock the returned network before updates.
 func (n *networkState) LockingNetwork(iface, ssid string) *lockingNetwork {
 	n.mu.Lock()
 	defer n.mu.Unlock()
 
-	id := genNetKey(iface, ssid)
+	id := n.genNetKey(iface, ssid)
 
 	net, ok := n.network[id]
 	if !ok {
@@ -57,9 +84,7 @@ func (n *networkState) LockingNetwork(iface, ssid string) *lockingNetwork {
 		} else {
 			net.netType = NetworkTypeWired
 		}
-		if iface != IfNameAny && iface != "" {
-			net.interfaceName = iface
-		}
+		net.interfaceName = iface
 		n.logger.Debugf("found new network %s (%s)", id, net.netType)
 	}
 
@@ -70,7 +95,7 @@ func (n *networkState) LockingNetwork(iface, ssid string) *lockingNetwork {
 func (n *networkState) Network(iface, ssid string) network {
 	n.mu.Lock()
 	defer n.mu.Unlock()
-	id := genNetKey(iface, ssid)
+	id := n.genNetKey(iface, ssid)
 	ln, ok := n.network[id]
 	if !ok {
 		return network{}

subsystems/provisioning/provisioning.go

@@ -162,6 +162,7 @@ func (w *Provisioning) init(ctx context.Context) error {
 	}
 
 	w.updateHotspotSSID(w.cfg)
+	w.netState.SetHotspotInterface(w.cfg.HotspotInterface)
 
 	if err := w.writeDNSMasq(); err != nil {
 		return errw.Wrap(err, "writing dnsmasq configuration")
@@ -312,6 +313,7 @@ func (w *Provisioning) Update(ctx context.Context, updateConf *agentpb.DeviceSub
 	if cfg.HotspotInterface == "" {
 		cfg.HotspotInterface = w.Config().HotspotInterface
 	}
+	w.netState.SetHotspotInterface(cfg.HotspotInterface)
 
 	if reflect.DeepEqual(cfg, w.cfg) {
 		return needRestart, nil

subsystems/provisioning/scanning.go

@@ -169,11 +169,15 @@ func (w *Provisioning) updateKnownConnections(ctx context.Context) error {
 		}
 
 		ifName, ssid, netType := getIfNameSSIDTypeFromSettings(settings)
-		if ifName == "" {
+		if netType == "" {
 			// unknown network type, or broken network
 			continue
 		}
 
+		if ifName == "" && netType == NetworkTypeWifi {
+			ifName = w.Config().HotspotInterface
+		}
+
 		_, ok := highestPriority[ifName]
 		if !ok {
 			highestPriority[ifName] = -999

subsystems/provisioning/setup.go

@@ -124,6 +124,7 @@ func (w *Provisioning) initDevices() error {
 
 			if w.cfg.HotspotInterface == "" || ifName == w.cfg.HotspotInterface {
 				w.cfg.HotspotInterface = ifName
+				w.netState.SetHotspotInterface(ifName)
 				w.logger.Infof("Using %s for hotspot/provisioning, will actively manage wifi only on this device.", ifName)
 			}
 		default: