Mask measure blips end to end and show a running median cable length

bcm.go

@@ -45,8 +45,9 @@ const (
 	// Covers the handler's documented 2 s freeze during 10GBASE-T training.
 	bcmStatusTimeout = 3 * time.Second
 
-	ecdPoll     = 200 * time.Millisecond
+	ecdPoll        = 200 * time.Millisecond
-	ecdDeadline = 50 * time.Second
+	ecdArmDeadline = 2 * time.Second
+	ecdDeadline    = 50 * time.Second
 )
 
 type bcm struct {
@@ -272,6 +273,24 @@ func (b *bcm) cableDiag() (res ecdResult, err error) {
 		if err = b.mdioWrite(bcmMMDVendor, bcmRegECDCtrl, ctrl&^0xF400|0x8400); err != nil {
 			return
 		}
+		// A diag takes seconds against a 200 ms poll, so busy must be seen set
+		// before clear means finished; accepting an early clear reads the
+		// result of a diag that never armed, which is an instant all-zero
+		// "success" while restartAN still tears the link down.
+		arm := time.Now().Add(ecdArmDeadline)
+		for {
+			if ctrl, err = b.mdioRead(bcmMMDVendor, bcmRegECDCtrl); err != nil {
+				return
+			}
+			if ctrl&0x0800 != 0 {
+				break
+			}
+			if time.Now().After(arm) {
+				err = fmt.Errorf("%s: cable diag never armed within %s", b.ifname, ecdArmDeadline)
+				return
+			}
+			time.Sleep(ecdPoll)
+		}
 		deadline := time.Now().Add(ecdDeadline)
 		for {
 			if ctrl, err = b.mdioRead(bcmMMDVendor, bcmRegECDCtrl); err != nil {

main.go

@@ -56,8 +56,8 @@ func resetAll(dirs []*direction, mods []*phyModule) time.Time {
 // Nothing to hold or revert here: failures during a measure were never
 // counted, so the view is always the counters as they stand.
 func phyView(diag *cableDiag, modules []*phyModule) (phyDisplay, bool) {
-	info, rng, measuring := diag.snapshot()
+	info, lens, measuring := diag.snapshot()
-	return phyDisplayFrom(info, rng, measuring, modules[0].view(), modules[1].view()), measuring
+	return phyDisplayFrom(info, lens, measuring, modules[0].view(), modules[1].view()), measuring
 }
 
 var intervalCols = []colSpec{
@@ -346,7 +346,7 @@ func run(aName, bName string) (err error) {
 	// hide or revert afterwards.
 	kickMeasure := func() {
 		if diag.kick(&done) {
-			diag.resetRange()
+			diag.resetLengths()
 			start = resetAll(dirs, modules)
 			hist.reset()
 		}

phy.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+	"slices"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -155,6 +156,14 @@ func (m *phyModule) poll() error {
 		return err
 	}
 
+	// Setting busy is a measure's first act, so busy still clear here proves
+	// every read above finished before the measure disturbed anything; a
+	// measure that began mid-read makes the samples ambiguous, and forgive()
+	// re-baselines after it anyway.
+	if m.busy.Load() {
+		return nil
+	}
+
 	m.mu.Lock()
 	if m.link && !link {
 		m.notes = append(m.notes,
@@ -267,18 +276,22 @@ type cableInfo struct {
 	haveMaps [2]bool
 }
 
+// The median across the pairs rather than the mean: the pairs run the same
+// cable, so one pair's skewed TDR estimate is an outlier, not signal. Always
+// a value some pair actually measured — the lower middle when the count is
+// even, never an average.
 func (c cableInfo) metresValue() (int, bool) {
-	sum, n := 0, 0
+	var ok []int
 	for i, v := range c.ecd.verdicts {
 		if v == pairOK {
-			sum += c.ecd.metres[i]
+			ok = append(ok, c.ecd.metres[i])
-			n++
 		}
 	}
-	if n == 0 {
+	if len(ok) == 0 {
 		return 0, false
 	}
-	return (sum + n/2) / n, true
+	slices.Sort(ok)
+	return ok[(len(ok)-1)/2], true
 }
 
 func (c cableInfo) metresString() string {
@@ -289,26 +302,24 @@ func (c cableInfo) metresString() string {
 	return fmt.Sprintf("%d", m)
 }
 
-type cableRange struct {
+// One value per completed healthy measure; the running median steadies as
-	have     bool
+// they accumulate, and like every median here it is always a value that was
-	min, max int
+// actually measured.
+type cableLengths struct {
+	vals []int
 }
 
-func (r *cableRange) add(m int) {
+func (l *cableLengths) add(m int) {
-	if !r.have || m < r.min {
+	l.vals = append(l.vals, m)
-		r.min = m
-	}
-	if !r.have || m > r.max {
-		r.max = m
-	}
-	r.have = true
 }
 
-func (r cableRange) String() string {
+func (l cableLengths) median() (int, bool) {
-	if r.min == r.max {
+	if len(l.vals) == 0 {
-		return fmt.Sprintf("%d", r.min)
+		return 0, false
 	}
-	return fmt.Sprintf("%d-%d", r.min, r.max)
+	s := slices.Clone(l.vals)
+	slices.Sort(s)
+	return s[(len(s)-1)/2], true
 }
 
 const (
@@ -378,7 +389,7 @@ func cableSummary(cable cableInfo, measuring bool) (string, int) {
 
 // The margin is the worst pair across the ends that measure SNR (the Wiitek's
 // IEEE 1.133–136), gated on the whole pair being fresh and linked.
-func phyDisplayFrom(cable cableInfo, rng cableRange, measuring bool, a, b phyModView) phyDisplay {
+func phyDisplayFrom(cable cableInfo, lens cableLengths, measuring bool, a, b phyModView) phyDisplay {
 	d := phyDisplay{
 		haveSNR:   a.fresh && b.fresh && a.link && b.link && (a.haveSNR || b.haveSNR),
 		corrected: a.blocks + a.ber + a.retrain + b.blocks + b.ber + b.retrain,
@@ -399,8 +410,11 @@ func phyDisplayFrom(cable cableInfo, rng cableRange, measuring bool, a, b phyMod
 		}
 	}
 	d.metres, d.metresClass = cableSummary(cable, measuring)
-	if d.metresClass == clsGood && rng.have {
+	// The running median outlives moments without a fresh verdict — a
-		d.metres = rng.String()
+	// running measure, or one that returned no data — and only an actual
+	// fault verdict replaces it.
+	if m, ok := lens.median(); ok && (d.metresClass == clsGood || d.metresClass == clsNone) {
+		d.metres, d.metresClass = fmt.Sprintf("%d", m), clsGood
 	}
 	return d
 }
@@ -444,14 +458,16 @@ func measureCable(mods []*phyModule, done *atomic.Bool) (cableInfo, error) {
 	}
 	// A pair the diag just called faulted is not going to relink; waiting the
 	// full carrier timeout would hold the verdict hostage for 25 s on an
-	// unplugged cable.
+	// unplugged cable. A verdict of no data is not a fault: the restart above
-	healthy := true
+	// took a working link down, and returning before carrier drops the
+	// suppression while it is still retraining.
+	faulted := false
 	for _, v := range c.ecd.verdicts {
-		if v != pairOK {
+		if v != 0 && v != pairOK {
-			healthy = false
+			faulted = true
 		}
 	}
-	if healthy {
+	if !faulted {
 		waitCarrier([2]string{mods[0].dev.name(), mods[1].dev.name()}, done)
 	}
 	for i, m := range mods {
@@ -475,24 +491,24 @@ type cableDiag struct {
 	// hidden and reverted.
 	measuring atomic.Bool
 
-	mu   sync.Mutex
+	mu      sync.Mutex
-	info cableInfo
+	info    cableInfo
-	rng  cableRange
+	lengths cableLengths
 }
 
 func newCableDiag(mods []*phyModule, info cableInfo) *cableDiag {
 	return &cableDiag{mods: mods, info: info}
 }
 
-func (c *cableDiag) snapshot() (cableInfo, cableRange, bool) {
+func (c *cableDiag) snapshot() (cableInfo, cableLengths, bool) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	return c.info, c.rng, c.measuring.Load()
+	return c.info, c.lengths, c.measuring.Load()
 }
 
-func (c *cableDiag) resetRange() {
+func (c *cableDiag) resetLengths() {
 	c.mu.Lock()
-	c.rng = cableRange{}
+	c.lengths = cableLengths{}
 	c.mu.Unlock()
 }
 
@@ -513,7 +529,7 @@ func (c *cableDiag) kick(done *atomic.Bool) bool {
 		c.info = info
 		if _, cls := cableSummary(info, false); cls == clsGood {
 			if m, ok := info.metresValue(); ok {
-				c.rng.add(m)
+				c.lengths.add(m)
 			}
 		}
 		c.mu.Unlock()

phy_test.go

@@ -14,7 +14,7 @@ func TestPhyDisplayWorstMarginAndFault(t *testing.T) {
 	}
 	a := freshMod([4]float64{5, 5, 2, 0.5})
 	b := freshMod([4]float64{4, 5, 5, 5})
-	d := phyDisplayFrom(cable, cableRange{}, false, a, b)
+	d := phyDisplayFrom(cable, cableLengths{}, false, a, b)
 
 	if !d.haveSNR || d.worstMargin != 0.5 {
 		t.Errorf("worstMargin = %v (have %v), want 0.5", d.worstMargin, d.haveSNR)
@@ -27,15 +27,15 @@ func TestPhyDisplayWorstMarginAndFault(t *testing.T) {
 func TestPhyDisplaySNREndOwnsMargin(t *testing.T) {
 	wiitek := freshMod([4]float64{9.2, 7.2, 7.0, 8.7})
 	fs := phyModView{fresh: true, link: true}
-	d := phyDisplayFrom(cableInfo{}, cableRange{}, false, wiitek, fs)
+	d := phyDisplayFrom(cableInfo{}, cableLengths{}, false, wiitek, fs)
 	if !d.haveSNR || d.worstMargin != 7.0 {
 		t.Errorf("worstMargin = %v (have %v), want the SNR end's 7.0", d.worstMargin, d.haveSNR)
 	}
-	d = phyDisplayFrom(cableInfo{}, cableRange{}, false, fs, wiitek)
+	d = phyDisplayFrom(cableInfo{}, cableLengths{}, false, fs, wiitek)
 	if !d.haveSNR || d.worstMargin != 7.0 {
 		t.Errorf("swapped ends: worstMargin = %v, want 7.0", d.worstMargin)
 	}
-	d = phyDisplayFrom(cableInfo{}, cableRange{}, false, fs, fs)
+	d = phyDisplayFrom(cableInfo{}, cableLengths{}, false, fs, fs)
 	if d.haveSNR {
 		t.Error("a pair with no SNR end should withhold snr")
 	}
@@ -50,9 +50,9 @@ func TestCableSummary(t *testing.T) {
 		want      string
 		class     int
 	}{
-		{"clean", cableInfo{ecd: healthy, maps: [2]byte{pairIdentityMap, pairIdentityMap}, haveMaps: [2]bool{true, true}}, false, "49", clsGood},
+		{"clean", cableInfo{ecd: healthy, maps: [2]byte{pairIdentityMap, pairIdentityMap}, haveMaps: [2]bool{true, true}}, false, "48", clsGood},
 		{"far-end swap", cableInfo{ecd: healthy, maps: [2]byte{pairIdentityMap, 0xE1}, haveMaps: [2]bool{true, true}}, false, "xover", clsWarn},
-		{"one-end map only", cableInfo{ecd: healthy, maps: [2]byte{pairIdentityMap, 0}, haveMaps: [2]bool{true, false}}, false, "49", clsGood},
+		{"one-end map only", cableInfo{ecd: healthy, maps: [2]byte{pairIdentityMap, 0}, haveMaps: [2]bool{true, false}}, false, "48", clsGood},
 		{"fault", cableInfo{ecd: ecdResult{verdicts: [4]int{pairOK, pairShort, pairXtalk, pairOK}, metres: [4]int{50, 12, 30, 51}}}, false, "SHORT", clsBad},
 		{"fault beats swap", cableInfo{ecd: ecdResult{verdicts: [4]int{pairOpen, pairOK, pairOK, pairOK}}, maps: [2]byte{pairIdentityMap, 0xE1}, haveMaps: [2]bool{true, true}}, false, "OPEN", clsBad},
 		{"no diag yet", cableInfo{}, false, "-", clsNone},
@@ -71,11 +71,11 @@ func TestPhyDisplayStaleGoesDim(t *testing.T) {
 		maps:     [2]byte{pairIdentityMap, pairIdentityMap},
 		haveMaps: [2]bool{true, true},
 	}
-	d := phyDisplayFrom(cable, cableRange{}, false, freshMod([4]float64{5, 5, 5, 5}), phyModView{})
+	d := phyDisplayFrom(cable, cableLengths{}, false, freshMod([4]float64{5, 5, 5, 5}), phyModView{})
 	if d.haveSNR {
 		t.Error("one silent module should withhold snr")
 	}
-	if d.metres != "44" || d.metresClass != clsGood {
+	if d.metres != "45" || d.metresClass != clsGood {
 		t.Errorf("metres = %q class %d, want the diag verdict kept", d.metres, d.metresClass)
 	}
 }
@@ -85,26 +85,26 @@ func TestCableMetresString(t *testing.T) {
 		verdicts: [4]int{pairOK, pairOpen, pairOK, pairOK},
 		metres:   [4]int{45, 3, 41, 46},
 	}}
-	if got := c.metresString(); got != "44" {
+	if got := c.metresString(); got != "45" {
-		t.Errorf("metres = %q, want 44", got)
+		t.Errorf("metres = %q, want the healthy pairs' median 45", got)
 	}
 	if got := (cableInfo{}).metresString(); got != "-" {
 		t.Errorf("no diag = %q, want -", got)
 	}
 }
 
-func TestCableRange(t *testing.T) {
+func TestCableLengths(t *testing.T) {
-	var r cableRange
+	var l cableLengths
 	for _, m := range []int{42, 40, 44, 41} {
-		r.add(m)
+		l.add(m)
 	}
-	if r.String() != "40-44" {
+	if m, ok := l.median(); !ok || m != 41 {
-		t.Errorf("range = %q, want 40-44", r.String())
+		t.Errorf("median = %d (%v), want the lower middle 41", m, ok)
 	}
-	one := cableRange{}
+	one := cableLengths{}
 	one.add(42)
-	if one.String() != "42" {
+	if m, ok := one.median(); !ok || m != 42 {
-		t.Errorf("single measure = %q, want 42", one.String())
+		t.Errorf("single measure = %d (%v), want 42", m, ok)
 	}
 
 	cable := cableInfo{
@@ -112,14 +112,23 @@ func TestCableRange(t *testing.T) {
 		maps:     [2]byte{pairIdentityMap, pairIdentityMap},
 		haveMaps: [2]bool{true, true},
 	}
-	d := phyDisplayFrom(cable, r, false, freshMod([4]float64{5, 5, 5, 5}), freshMod([4]float64{5, 5, 5, 5}))
+	d := phyDisplayFrom(cable, l, false, freshMod([4]float64{5, 5, 5, 5}), freshMod([4]float64{5, 5, 5, 5}))
-	if d.metres != "40-44" || d.metresClass != clsGood {
+	if d.metres != "41" || d.metresClass != clsGood {
-		t.Errorf("metres = %q class %d, want the range shown good", d.metres, d.metresClass)
+		t.Errorf("metres = %q class %d, want the running median shown good", d.metres, d.metresClass)
 	}
 	faulted := cableInfo{ecd: ecdResult{verdicts: [4]int{pairOK, pairOpen, pairOK, pairOK}}}
-	d = phyDisplayFrom(faulted, r, false, freshMod([4]float64{5, 5, 5, 5}), freshMod([4]float64{5, 5, 5, 5}))
+	d = phyDisplayFrom(faulted, l, false, freshMod([4]float64{5, 5, 5, 5}), freshMod([4]float64{5, 5, 5, 5}))
 	if d.metres != "OPEN" {
-		t.Errorf("fault shows %q, want the verdict to beat the range", d.metres)
+		t.Errorf("fault shows %q, want the verdict to beat the median", d.metres)
+	}
+
+	d = phyDisplayFrom(cable, l, true, freshMod([4]float64{5, 5, 5, 5}), freshMod([4]float64{5, 5, 5, 5}))
+	if d.metres != "41" || d.metresClass != clsGood {
+		t.Errorf("measuring shows %q class %d, want the median held", d.metres, d.metresClass)
+	}
+	d = phyDisplayFrom(cableInfo{}, l, false, freshMod([4]float64{5, 5, 5, 5}), freshMod([4]float64{5, 5, 5, 5}))
+	if d.metres != "41" || d.metresClass != clsGood {
+		t.Errorf("no data shows %q class %d, want the median held", d.metres, d.metresClass)
 	}
 }