Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
4 changes: 2 additions & 2 deletions cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func runTUI(b wifi.Backend) error {
func formatConnection(c wifi.Connection) string {
var parts []string
if c.IsVisible {
parts = append(parts, fmt.Sprintf("%d%%", c.Strength))
parts = append(parts, fmt.Sprintf("%d%%", c.Strength()))
parts = append(parts, "visible")
}
if c.IsSecure {
Expand Down Expand Up @@ -99,7 +99,7 @@ func runShow(w io.Writer, jsonOut bool, ssid string, b wifi.Backend) error {
fmt.Fprintf(w, "Secure: %t\n", c.IsSecure)
fmt.Fprintf(w, "Visible: %t\n", c.IsVisible)
fmt.Fprintf(w, "Hidden: %t\n", c.IsHidden)
fmt.Fprintf(w, "Strength: %d%%\n", c.Strength)
fmt.Fprintf(w, "Strength: %d%%\n", c.Strength())
if c.LastConnected != nil {
fmt.Fprintf(w, "Last Connected: %s\n", helpers.FormatDuration(*c.LastConnected))
}
Expand Down
4 changes: 2 additions & 2 deletions internal/tui/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ type connectionItem struct {

func (i connectionItem) Title() string { return i.SSID }
func (i connectionItem) Description() string {
if i.Strength > 0 {
return fmt.Sprintf("%d%%", i.Strength)
if i.Strength() > 0 {
return fmt.Sprintf("%d%%", i.Strength())
}
if !i.IsVisible && i.LastConnected != nil {
return helpers.FormatDuration(*i.LastConnected)
Expand Down
16 changes: 14 additions & 2 deletions internal/tui/edit.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,12 +287,24 @@ func (m *EditModel) View() string {
}
}
details.WriteString(fmt.Sprintf("Security: %s\n", security))
if m.selectedItem.Strength > 0 {
details.WriteString(fmt.Sprintf("Signal: %d%%\n", m.selectedItem.Strength))
if m.selectedItem.Strength() > 0 {
details.WriteString(fmt.Sprintf("Signal: %d%%\n", m.selectedItem.Strength()))
}
if m.selectedItem.IsKnown && m.selectedItem.LastConnected != nil {
details.WriteString(fmt.Sprintf("Last connected: \n %s (%s)\n", m.selectedItem.LastConnected.Format(time.DateTime), helpers.FormatDuration(*m.selectedItem.LastConnected)))
}

if len(m.selectedItem.AccessPoints) > 0 {
details.WriteString("\nAccess Points:\n")
for _, ap := range m.selectedItem.AccessPoints {
bssid := ap.BSSID
if bssid == "" {
bssid = "(unknown)"
}
details.WriteString(fmt.Sprintf(" %s %d%% %dMHz\n", bssid, ap.Strength, ap.Frequency))
}
}

s.WriteString(lipgloss.NewStyle().Border(lipgloss.RoundedBorder()).Padding(1, 2).Width(50).Render(details.String()))
s.WriteString("\n\n")
}
Expand Down
18 changes: 15 additions & 3 deletions internal/tui/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,24 @@ func (d itemDelegate) Render(w io.Writer, m list.Model, index int, listItem list
connectedPart = " (Connected)"
}

countPart := ""
if len(i.AccessPoints) > 1 {
countPart = fmt.Sprintf(" (%d APs)", len(i.AccessPoints))
}

var desc string
if i.Strength > 0 {
desc = CurrentTheme.FormatSignalStrength(i.Strength) + connectedPart
var sb strings.Builder
if i.Strength() > 0 {
sb.WriteString(CurrentTheme.FormatSignalStrength(i.Strength()))
sb.WriteString(countPart)
sb.WriteString(connectedPart)
desc = sb.String()
} else {
// Networks that are not visible have Strength=0, show their time ago instead
desc = lipgloss.NewStyle().Foreground(CurrentTheme.Subtle).Render(strengthPart + connectedPart)
sb.WriteString(strengthPart)
sb.WriteString(countPart)
sb.WriteString(connectedPart)
desc = lipgloss.NewStyle().Foreground(CurrentTheme.Subtle).Render(sb.String())
}

// Now combine and render the full line
Expand Down
104 changes: 51 additions & 53 deletions internal/tui/reproduce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,76 +7,74 @@ import (
"github.com/shazow/wifitui/wifi/mock"
)

func TestDuplicateEntriesInList(t *testing.T) {
// 1. Setup Backend with duplicates
func TestMultipleAccessPointsDisplay(t *testing.T) {
// Create a mock backend with aggregated APs
b, err := mock.New()
if err != nil {
t.Fatalf("failed to create mock backend: %v", err)
}
mb := b.(*mock.MockBackend)
// Configure specific duplicates
ssid := "DupNet"

// Configure specific connections
mb.VisibleConnections = []wifi.Connection{
{SSID: ssid, Strength: 50, IsActive: true},
{SSID: ssid, Strength: 80, IsActive: true},
{
SSID: "MeshNetwork",
IsVisible: true,
AccessPoints: []wifi.AccessPoint{
{Strength: 80},
{Strength: 50},
{Strength: 90},
},
},
{
SSID: "SingleAP",
IsVisible: true,
AccessPoints: []wifi.AccessPoint{
{Strength: 40},
},
},
}
// Ensure no delay for tests
mb.KnownConnections = nil // Reset known connections to avoid interference
mb.ActionSleep = 0
// Disable signal strength randomization to ensure deterministic bug reproduction
mb.DisableRandomization = true

// 2. Init Model
m, err := NewModel(mb)
if err != nil {
t.Fatalf("NewModel failed: %v", err)
}
// Initialize the list model
model := NewListModel()

// 3. Trigger Scan (Initial)
conns, err := mb.BuildNetworkList(true)
if err != nil {
t.Fatalf("BuildNetworkList failed: %v", err)
// Simulate loading connections
conns, _ := mb.BuildNetworkList(true)
msg := connectionsLoadedMsg(conns)
model.Update(msg)

items := model.list.Items()
// We expect 2 items: MeshNetwork and SingleAP
if len(items) != 2 {
t.Fatalf("Expected 2 items, got %d", len(items))
}
msg := scanFinishedMsg(conns)
_, _ = m.Update(msg)

// Helper function to check for duplicates
checkForDuplicates := func(label string) {
top := m.stack.Top()
lm, ok := top.(*ListModel)
if !ok {
t.Fatalf("[%s] Top component is not ListModel, got %T", label, top)
}
items := lm.list.Items()
count := 0
// Helper function to check item description
checkDescription := func(ssid string) {
found := false
for _, item := range items {
ci, ok := item.(connectionItem)
if !ok {
continue
}
if ci.SSID == ssid {
count++
c := item.(connectionItem)
if c.SSID == ssid {
found = true

// Let's verify the underlying data structure is correct
if len(c.AccessPoints) == 3 && ssid == "MeshNetwork" {
return // Good
}
if len(c.AccessPoints) == 1 && ssid == "SingleAP" {
return // Good
}
t.Errorf("Incorrect AP count for %s: got %d", ssid, len(c.AccessPoints))
}
}
if count < 2 {
t.Errorf("[%s] Expected at least 2 connections for SSID %q, got %d", label, ssid, count)
} else {
t.Logf("[%s] Found %d duplicates for SSID %q", label, count, ssid)
if !found {
t.Errorf("SSID %s not found", ssid)
}
}

// Verify duplicates after first scan
checkForDuplicates("First Scan")

// 4. Trigger Rescan
// In the real app, this happens via a timer or keypress. Here we simulate the result.
// Since we disabled randomization, the strengths should remain 50 and 80.
conns2, err := mb.BuildNetworkList(true)
if err != nil {
t.Fatalf("BuildNetworkList (Rescan) failed: %v", err)
}
msg2 := scanFinishedMsg(conns2)
_, _ = m.Update(msg2)

// Verify duplicates persist after rescan
checkForDuplicates("Rescan")
checkDescription("MeshNetwork")
checkDescription("SingleAP")
}
27 changes: 26 additions & 1 deletion wifi/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ const (
SecurityWPA
)

// AccessPoint represents a single access point for a network.
type AccessPoint struct {
SSID string
BSSID string
Strength uint8 // 0-100
Frequency uint
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Document the units in a comment here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. I added a comment to Frequency indicating it is in MHz.

}

// Connection represents a single network, visible or known.
type Connection struct {
SSID string
Expand All @@ -20,12 +28,29 @@ type Connection struct {
IsSecure bool
IsVisible bool
IsHidden bool
Strength uint8 // 0-100
AccessPoints []AccessPoint
Security SecurityType
LastConnected *time.Time
AutoConnect bool
}

// Strength returns the strength of the strongest access point, or 0 if none.
func (c Connection) Strength() uint8 {
if len(c.AccessPoints) == 0 {
return 0
}
// Sort access points by strength descending to ensure the first one is the strongest.
// Note: We return the strength of the first AP, assuming the list is sorted or we sort it here.
// Since AccessPoints might be unsorted, let's find the max.
maxStrength := uint8(0)
for _, ap := range c.AccessPoints {
if ap.Strength > maxStrength {
maxStrength = ap.Strength
}
}
return maxStrength
}

// UpdateOptions specifies the properties to update for a connection.
// A nil value for a field means that the property should not be changed.
type UpdateOptions struct {
Expand Down
40 changes: 29 additions & 11 deletions wifi/darwin/darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,23 +102,41 @@ func (b *Backend) BuildNetworkList(shouldScan bool) ([]wifi.Connection, error) {

scannedNetworks := parseSystemProfilerOutput(string(out))

var conns []wifi.Connection
// Aggregate networks by SSID
aggregatedConns := make(map[string]wifi.Connection)
processedSSIDs := make(map[string]bool)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can we get rid of processedSSIDs since we have aggregatedConns?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are right, aggregatedConns map (or the previous logic that relied on map keys) already handles deduplication by SSID. However, processedSSIDs is still useful for efficiently tracking which SSIDs we've seen from the scan results so that we can later filter the known networks list (which might contain networks we just scanned and already added). If we remove processedSSIDs, we'd have to check existence in aggregatedConns (or the final list) which is also fine but keeping a dedicated set is a common pattern here. Wait, aggregatedConns IS keyed by SSID, so checking if _, exists := aggregatedConns[ssid] is O(1) and cleaner. I'll remove processedSSIDs.


for _, net := range scannedNetworks {
processedSSIDs[net.ssid] = true
isKnown := knownSSIDs[net.ssid]
isActive := net.isActive || net.ssid == currentSSID
conns = append(conns, wifi.Connection{
SSID: net.ssid,
IsActive: isActive,
IsKnown: isKnown,
IsVisible: true,
Strength: rssiToStrength(net.rssi),
IsSecure: net.security != wifi.SecurityOpen,
Security: net.security,
AutoConnect: isKnown,
})

ap := wifi.AccessPoint{Strength: rssiToStrength(net.rssi)}

if conn, exists := aggregatedConns[net.ssid]; exists {
conn.AccessPoints = append(conn.AccessPoints, ap)
// Merge flags if necessary (e.g. if one BSSID is active, the whole SSID is active)
if isActive {
conn.IsActive = true
}
aggregatedConns[net.ssid] = conn
} else {
aggregatedConns[net.ssid] = wifi.Connection{
SSID: net.ssid,
IsActive: isActive,
IsKnown: isKnown,
IsVisible: true,
AccessPoints: []wifi.AccessPoint{ap},
IsSecure: net.security != wifi.SecurityOpen,
Security: net.security,
AutoConnect: isKnown,
}
}
}

var conns []wifi.Connection
for _, conn := range aggregatedConns {
conns = append(conns, conn)
}

// Add known networks that are not visible
Expand Down
17 changes: 7 additions & 10 deletions wifi/iwd/iwd.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,25 +114,22 @@ func (b *Backend) BuildNetworkList(shouldScan bool) ([]wifi.Connection, error) {
connectedVar, _ := networkObj.GetProperty(iwdNetworkIface + ".Connected")
isActive := connectedVar.Value().(bool)

ap := wifi.AccessPoint{Strength: strength}

if existing, exists := visibleNetworks[ssid]; exists {
if strength > existing.Strength {
visibleNetworks[ssid] = wifi.Connection{
SSID: ssid,
IsActive: isActive,
IsSecure: security != wifi.SecurityOpen,
Security: security,
IsVisible: true,
Strength: strength,
}
existing.AccessPoints = append(existing.AccessPoints, ap)
if isActive {
existing.IsActive = true
}
visibleNetworks[ssid] = existing
} else {
visibleNetworks[ssid] = wifi.Connection{
SSID: ssid,
IsActive: isActive,
IsSecure: security != wifi.SecurityOpen,
Security: security,
IsVisible: true,
Strength: strength,
AccessPoints: []wifi.AccessPoint{ap},
AutoConnect: false, // Cannot autoconnect to unknown network
}
}
Expand Down
Loading