Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wifi candidate selection to include non-interface-bound candidates #49

Merged
merged 3 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/viam-agent/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drive-by... didn't realize I'd copy/pasted the newlines from the CLI "version" output here (where it makes no sense.)


// if FastStart is set, skip updates and start viam-server immediately, then proceed as normal
var fastSuccess bool
Expand Down
2 changes: 0 additions & 2 deletions subsystems/provisioning/definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ const (
NetworkTypeWired = "wired"
NetworkTypeHotspot = "hotspot"

IfNameAny = "any"

HealthCheckTimeout = time.Minute
)

Expand Down
12 changes: 0 additions & 12 deletions subsystems/provisioning/generators.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package provisioning

import (
"encoding/binary"
"fmt"
"net"
"regexp"
"strconv"
Expand Down Expand Up @@ -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)
}
50 changes: 23 additions & 27 deletions subsystems/provisioning/networkmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -533,7 +528,6 @@ func (w *Provisioning) getCandidates(ifName string) []string {
return []string{nw.ssid}
}
}
return []string{}
}

// sort by priority
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
)
Expand All @@ -717,6 +710,7 @@ func (w *Provisioning) mainLoop(ctx context.Context) {
w.logger.Error(err)
} else {
pMode = w.connState.getProvisioning()
pModeChange = w.connState.getProvisioningChange()
}
}
}
Expand All @@ -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)
}
Expand Down
35 changes: 30 additions & 5 deletions subsystems/provisioning/networkstate.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package provisioning

import (
"fmt"
"sync"

gnm "github.com/Otterverse/gonetworkmanager/v2"
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}

Expand All @@ -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{}
Expand Down
2 changes: 2 additions & 0 deletions subsystems/provisioning/provisioning.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion subsystems/provisioning/scanning.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions subsystems/provisioning/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading