missing GUI elements

docs/verb-reference.md

@@ -3599,7 +3599,7 @@ Read the sweep back as one row per finished cell and one summary per arm, and pu
 
 **Takes** nothing.
 
-**Answers** `runs`, `arms`, `warning`. `runs` and `arms` are both lists, and an empty `runs` is the normal answer before anything has started. `warning` is present only where the numbers do not mean what they look like: nothing run, one seed, one arm, a cell that failed, or seeds that agree so exactly that a difference between arms has nothing to be called larger than. Each run and each arm summary carries `builds`: the firmware every role actually ran, as `{role, version, file, bytes}`, so two arms that came back identical can be told from two arms that never switched build.
+**Answers** `runs`, `arms`, `warning`. `runs` and `arms` are both lists, and an empty `runs` is the normal answer before anything has started. `warning` is present only where the numbers do not mean what they look like: nothing run, one seed, one arm, a cell that failed, or seeds that agree so exactly that a difference between arms has nothing to be called larger than. Each run and each arm summary carries `builds`: the firmware every role actually ran, as `{role, version, file, bytes}`, so two arms that came back identical can be told from two arms that never switched build. Each arm summary also carries `irq_suppressed` and `busy_ms`, its chips' counts per cell averaged over the seeds that counted them, and absent where none did.
 
 **Example** - read the table so far
 

docs/verbs.json

@@ -1472,7 +1472,7 @@
         "arms",
         "warning"
       ],
-      "answers": "`runs` and `arms` are both lists, and an empty `runs` is the normal answer before anything has started. `warning` is present only where the numbers do not mean what they look like: nothing run, one seed, one arm, a cell that failed, or seeds that agree so exactly that a difference between arms has nothing to be called larger than. Each run and each arm summary carries `builds`: the firmware every role actually ran, as `{role, version, file, bytes}`, so two arms that came back identical can be told from two arms that never switched build.",
+      "answers": "`runs` and `arms` are both lists, and an empty `runs` is the normal answer before anything has started. `warning` is present only where the numbers do not mean what they look like: nothing run, one seed, one arm, a cell that failed, or seeds that agree so exactly that a difference between arms has nothing to be called larger than. Each run and each arm summary carries `builds`: the firmware every role actually ran, as `{role, version, file, bytes}`, so two arms that came back identical can be told from two arms that never switched build. Each arm summary also carries `irq_suppressed` and `busy_ms`, its chips' counts per cell averaged over the seeds that counted them, and absent where none did.",
       "example": {
         "params": {},
         "what": "read the table so far",

internal/app/session/experiment/chipstats_test.go

@@ -93,3 +93,32 @@ func TestANodeThatRestartedMidCellReportsWhatItCountedSince(t *testing.T) {
 		t.Errorf("irq_reads %d, want 15", got.IRQReads)
 	}
 }
+
+// An arm's chip means come from the seeds whose chips counted, and are absent
+// rather than zero where none did: an emulated node sends no record, and its
+// zeros averaged in would read as a firmware that missed less.
+func TestAnArmsChipMeansLeaveOutWhatWasNotCounted(t *testing.T) {
+	chip := func(supp uint64, reporting int) *ChipStats {
+		return &ChipStats{BusyMs: 1000, IRQSuppressed: supp, Reporting: reporting,
+			Nodes: []NodeChip{{Name: "A"}}}
+	}
+	row := summariseArm("1.17.0", []Result{
+		{Arm: "1.17.0", Chip: chip(80, 159)},
+		{Arm: "1.17.0", Chip: chip(0, 0)},
+		{Arm: "1.17.0"},
+	})
+	if got, _ := row["irq_suppressed"].(float64); got != 80 {
+		t.Errorf("irq_suppressed %v, want 80 from the one seed that counted", row["irq_suppressed"])
+	}
+	if got, _ := row["busy_ms"].(float64); got != 1000 {
+		t.Errorf("busy_ms %v, want 1000 from the two seeds with a chip record", row["busy_ms"])
+	}
+
+	bare := summariseArm("emulated", []Result{{Arm: "emulated", TX: 3}})
+	if _, ok := bare["irq_suppressed"]; ok {
+		t.Error("an arm with no chip record carries a suppressed mean")
+	}
+	if _, ok := bare["busy_ms"]; ok {
+		t.Error("an arm with no chip record carries a busy mean")
+	}
+}

internal/app/session/experiment/report.go

@@ -334,9 +334,38 @@ func summariseArm(name string, rs []Result) map[string]any {
 	if at2n > 0 {
 		row["at_risk_2db"] = at2 / at2n
 	}
+	chipMeans(row, ran)
 	return row
 }
 
+// chipMeans adds the chip's counters to an arm's row, averaged over the seeds
+// whose chips counted them.
+//
+// Over those seeds only, and absent where there were none, for the reason
+// at_risk_2db is: a chip that predates the suppressed count, or an emulated
+// node that sends no record, measured nothing, and a mean with its zeros in
+// would read as a firmware that missed less.
+func chipMeans(row map[string]any, ran []Result) {
+	var busy, busyN, supp, suppN float64
+	for _, r := range ran {
+		if r.Chip == nil || len(r.Chip.Nodes) == 0 {
+			continue
+		}
+		busy += float64(r.Chip.BusyMs)
+		busyN++
+		if r.Chip.Reporting > 0 {
+			supp += float64(r.Chip.IRQSuppressed)
+			suppN++
+		}
+	}
+	if busyN > 0 {
+		row["busy_ms"] = busy / busyN
+	}
+	if suppN > 0 {
+		row["irq_suppressed"] = supp / suppN
+	}
+}
+
 // spreadOf is the range of receptions across seeds, which is what says whether
 // a difference between arms is bigger than the noise within one.
 // spreadOf is how much the seeds of one arm disagree, as a fraction of that

internal/app/session/experiment/results.go

@@ -45,7 +45,7 @@ func registerExperimentResults(st *state.Store, s *session.Sim, e *experiment) {
 			// store's goroutine - taking the whole application with it - for
 			// what is at worst one arm with a missing number.
 			arm, _ := m["arm"].(string)
-			w.Experiment = append(w.Experiment, state.ArmSummary{
+			sum := state.ArmSummary{
 				Arm:       arm,
 				Runs:      mapInt(m, "runs"),
 				Failed:    mapInt(m, "failed"),
@@ -57,7 +57,12 @@ func registerExperimentResults(st *state.Store, s *session.Sim, e *experiment) {
 				AirtimeMs: mapFloat(m, "airtime_ms"),
 				RXSpread:  mapFloat(m, "rx_spread"),
 				PerSecond: e.perSecondFor(arm),
-			})
+			}
+			// Present or not, rather than read as zero: an arm whose chips
+			// counted nothing draws a dash.
+			sum.IRQSuppressed, sum.HasSuppressed = m["irq_suppressed"].(float64)
+			sum.BusyMs, sum.HasBusy = m["busy_ms"].(float64)
+			w.Experiment = append(w.Experiment, sum)
 		}
 		w.ExperimentRuns = e.runRows()
 		w.ExperimentWarning = warn

internal/app/session/experiment/results.verbs.json

@@ -59,7 +59,7 @@
       "arms",
       "warning"
     ],
-    "answers": "`runs` and `arms` are both lists, and an empty `runs` is the normal answer before anything has started. `warning` is present only where the numbers do not mean what they look like: nothing run, one seed, one arm, a cell that failed, or seeds that agree so exactly that a difference between arms has nothing to be called larger than. Each run and each arm summary carries `builds`: the firmware every role actually ran, as `{role, version, file, bytes}`, so two arms that came back identical can be told from two arms that never switched build.",
+    "answers": "`runs` and `arms` are both lists, and an empty `runs` is the normal answer before anything has started. `warning` is present only where the numbers do not mean what they look like: nothing run, one seed, one arm, a cell that failed, or seeds that agree so exactly that a difference between arms has nothing to be called larger than. Each run and each arm summary carries `builds`: the firmware every role actually ran, as `{role, version, file, bytes}`, so two arms that came back identical can be told from two arms that never switched build. Each arm summary also carries `irq_suppressed` and `busy_ms`, its chips' counts per cell averaged over the seeds that counted them, and absent where none did.",
     "example": {
       "params": {},
       "what": "read the table so far",

internal/app/state/results.go

@@ -26,6 +26,13 @@ type ArmSummary struct {
 	// spread - and a difference between arms cannot be called larger than a
 	// noise nobody has measured.
 	RXSpread float64
+	// IRQSuppressed and BusyMs are what the arm's chips counted over a cell,
+	// summed across nodes and averaged across seeds: events the firmware's
+	// IrqMask kept out, and how long the detection flags were up. Two
+	// firmwares that deliver identically can differ only here. Each Has is
+	// false where no seed's chip counted it, which is a dash and not a zero.
+	IRQSuppressed, BusyMs  float64
+	HasSuppressed, HasBusy bool
 	// PerSecond is receptions in each second after the burst, summed over this
 	// arm's seeds. The shape of the flood rather than its total.
 	PerSecond []int

internal/ui/workbench/nodeview/nodewindowradio.go

@@ -110,12 +110,16 @@ func (p *WindowPanel) radio(t *theme.Theme, gtx layout.Context, s *state.Snapsho
 		radioRow{label: "IRQ mask", value: fmt.Sprintf("%#04x", r.IRQMask),
 			why: "what the firmware allowed into the chip's interrupt register"},
 		dio1MaskRow(r),
+		suppressedRow(st),
 		radioRow{label: "IRQ flags", value: fmt.Sprintf("%#04x", r.IRQFlags),
 			why: "what is raised now; a flag that sticks is a node that stops transmitting"},
 		radioRow{label: "IRQ reads", value: fmt.Sprintf("%d, %d found the air busy",
 			st.IRQReads, st.BusyReads),
 			why: "how often the driver asked, and how often it was told to wait"},
 		radioRow{label: "busy time", value: fmt.Sprintf("%d ms", st.BusyMs)},
+		radioRow{label: "spurious detections", value: fmt.Sprintf("%d", st.Spurious),
+			why:  "false detections the faulty chip variants inject; none on a chip that behaves",
+			warn: st.Spurious > 0},
 	)
 
 	// Vertical, said out loud. A list left on the zero value lays its rows out
@@ -165,6 +169,27 @@ func (p *WindowPanel) radio(t *theme.Theme, gtx layout.Context, s *state.Snapsho
 // A dash rather than 0x0000 where the node has not said: a radioserver older
 // than this field sends a shorter record, and zero is a legal mask for a chip
 // the firmware has not configured yet. "no data" is never drawn as zero.
+// suppressedRow is what the IRQ mask above it cost: events the chip raised and
+// the firmware never saw.
+//
+// "Not counted" rather than zero for a chip that predates the count, because
+// zero is the answer a firmware that enabled everything gives, and the two
+// must not look alike.
+func suppressedRow(st state.NodeStat) radioRow {
+	row := radioRow{label: "IRQ suppressed",
+		why: "events the chip raised that the IRQ mask kept out of the register; " +
+			"a firmware waiting on one it never enabled looks like a quiet mesh"}
+	if !st.SuppressedReported {
+		row.value = "not counted"
+		row.why = "this node's chip model predates the count, which arrived in " +
+			"virtual-sx1262 v1.3.2"
+		return row
+	}
+	row.value = fmt.Sprintf("%d", st.IRQSuppressed)
+	row.warn = st.IRQSuppressed > 0
+	return row
+}
+
 func dio1MaskRow(r state.RadioState) radioRow {
 	if !r.DIO1Reported {
 		return radioRow{label: "DIO1 mask", value: "-",

internal/ui/workbench/nodeview/radiorows_test.go

@@ -0,0 +1,32 @@
+package nodeview
+
+import (
+	"testing"
+
+	"github.com/MeshBench/meshbench/internal/app/state"
+)
+
+// A chip that does not keep the suppressed count says so, rather than reading
+// as a firmware that enabled every interrupt it needed.
+func TestASuppressedCountNobodyKeptIsNotAZero(t *testing.T) {
+	row := suppressedRow(state.NodeStat{IRQSuppressed: 0})
+	if row.value != "not counted" {
+		t.Errorf("a chip without the count reads %q", row.value)
+	}
+	if row.warn {
+		t.Error("a chip without the count is marked as worth a second look")
+	}
+}
+
+// A counted figure is shown as it is, and anything above zero is worth a
+// second look: an event the firmware never enabled is one it cannot act on.
+func TestASuppressedCountIsShownAndMarked(t *testing.T) {
+	none := suppressedRow(state.NodeStat{SuppressedReported: true})
+	if none.value != "0" || none.warn {
+		t.Errorf("a chip that kept nothing out reads %q, warn %v", none.value, none.warn)
+	}
+	some := suppressedRow(state.NodeStat{IRQSuppressed: 14, SuppressedReported: true})
+	if some.value != "14" || !some.warn {
+		t.Errorf("fourteen suppressed events read %q, warn %v", some.value, some.warn)
+	}
+}

internal/ui/workbench/nodeview/radioshot_test.go

@@ -0,0 +1,81 @@
+package nodeview
+
+import (
+	"image/png"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"gioui.org/layout"
+
+	"github.com/MeshBench/meshbench/internal/app/state"
+	"github.com/MeshBench/meshbench/internal/ui/theme"
+	"github.com/MeshBench/meshbench/internal/ui/uitest"
+)
+
+// Draw the radio tab for the three answers a chip can give about its mask, and
+// look at it.
+//
+// The figures are one repeater's from a scotland-strict cell: 1.16 enables no
+// PreambleDetected for receive and 1.17 does. The third chip predates the count,
+// and the row has to say so rather than read as a node that missed nothing.
+func TestDrawTheRadioTab(t *testing.T) {
+	if os.Getenv("MESHBENCH_SHOTS") == "" {
+		t.Skip("set MESHBENCH_SHOTS=<dir> to write the pictures")
+	}
+	dir := os.Getenv("MESHBENCH_SHOTS")
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+
+	radio := func(mask uint16) state.RadioState {
+		return state.RadioState{
+			Reported: true, GainReg: 0x96, Boosted: true, TxPowerDBm: 22,
+			Mode: 1, SF: 8, CR: 8, FreqHz: 869618000, BandwidthHz: 62500,
+			PreambleSyms: 32, IRQMask: mask, IRQFlags: 0, DIO1Mask: 0x0002,
+			DIO1Reported: true,
+		}
+	}
+	cases := []struct {
+		name string
+		stat state.NodeStat
+	}{
+		{"meshcore-1.16", state.NodeStat{
+			Name: "node-1", Running: true, Radio: radio(0x0272),
+			IRQReads: 2566, BusyReads: 224, BusyMs: 4478,
+			IRQSuppressed: 14, SuppressedReported: true,
+		}},
+		{"meshcore-1.17", state.NodeStat{
+			Name: "node-1", Running: true, Radio: radio(0x0276),
+			IRQReads: 2501, BusyReads: 67, BusyMs: 4821,
+			IRQSuppressed: 0, SuppressedReported: true,
+		}},
+		{"chip-without-the-count", state.NodeStat{
+			Name: "node-1", Running: true, Radio: radio(0x0276),
+			IRQReads: 2501, BusyReads: 67, BusyMs: 4821,
+		}},
+	}
+
+	for _, c := range cases {
+		c := c
+		t.Run(c.name, func(t *testing.T) {
+			snap := &state.Snapshot{Stats: []state.NodeStat{c.stat}}
+			p := &WindowPanel{Node: "node-1", Kind: "simple_repeater"}
+			p.Tab = TabRadio
+
+			img := uitest.RenderWidget(t, 900, 760, func(gtx layout.Context, th *theme.Theme) layout.Dimensions {
+				return p.radio(th, gtx, snap)
+			})
+			out := filepath.Join(dir, "radio-"+c.name+".png")
+			f, err := os.Create(out)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer func() { _ = f.Close() }()
+			if err := png.Encode(f, img); err != nil {
+				t.Fatal(err)
+			}
+			t.Logf("wrote %s", out)
+		})
+	}
+}

internal/ui/workbench/sweepfailedarm_test.go

@@ -87,3 +87,40 @@ func TestMoreIsBetterIsPaintedTheOtherWayRound(t *testing.T) {
 		t.Error("a change smaller than a rounding error was painted as one")
 	}
 }
+
+// An arm whose chips counted nothing draws the chip's columns as a dash, and a
+// baseline in that state leaves the arm beside it absolute rather than a delta
+// against a zero nobody measured.
+func TestAChipThatDidNotCountDrawsAsADash(t *testing.T) {
+	th := linkTestTheme()
+	counted := state.ArmSummary{Arm: "1.17.0", Runs: 5, RX: 192,
+		IRQSuppressed: 83.2, HasSuppressed: true, BusyMs: 283620, HasBusy: true}
+	silent := state.ArmSummary{Arm: "emulated", Runs: 5, RX: 192}
+
+	for _, title := range []string{"IRQ suppressed", "busy time"} {
+		got, colour := armCell(th, colNamed(title), silent, counted, false)
+		if got != "-" || colour != th.P.Faint {
+			t.Errorf("the %s of an arm whose chips did not count reads %q", title, got)
+		}
+		got, _ = armCell(th, colNamed(title), counted, silent, false)
+		if got == "-" || strings.Contains(got, "%") {
+			t.Errorf("against a baseline that did not count, the %s reads %q, "+
+				"not the arm's own figure", title, got)
+		}
+	}
+	if got, _ := armCell(th, colNamed("rx"), silent, counted, false); got == "-" {
+		t.Error("a chip that did not count took the arm's delivery numbers with it")
+	}
+}
+
+// Fewer suppressed events is the better direction, so 1.17 against 1.16 is
+// painted as a gain.
+func TestFewerSuppressedEventsIsPaintedAsBetter(t *testing.T) {
+	th := linkTestTheme()
+	old := state.ArmSummary{Arm: "1.16.0", Runs: 5, IRQSuppressed: 550.6, HasSuppressed: true}
+	fixed := state.ArmSummary{Arm: "1.17.0", Runs: 5, IRQSuppressed: 83.2, HasSuppressed: true}
+	got, colour := armCell(th, colNamed("IRQ suppressed"), fixed, old, false)
+	if colour != th.P.Good {
+		t.Errorf("%s fewer suppressed events is not painted as better", got)
+	}
+}

internal/ui/workbench/sweepresults.go

@@ -102,9 +102,10 @@ func (p *sweepResults) matrix(t *theme.Theme, gtx layout.Context, s *state.Snaps
 	children = append(children,
 		layout.Rigid(layout.Spacer{Height: t.Sp.S}.Layout),
 		layout.Rigid(comp.OneLine(t, t.Sz.Caption, t.P.Faint,
-			"against "+base.Arm+"; every column but rx and delivered is a cost, "+
+			"against "+base.Arm+"; every column but rx, delivered and busy time is a cost, "+
 				"so red is worse in both directions. A seed that failed is left out "+
-				"of its arm's mean, and an arm no seed got through is a dash", false)))
+				"of its arm's mean, and a dash is an arm that did not run or a chip "+
+				"that did not count", false)))
 	return layout.Flex{Axis: layout.Vertical}.Layout(gtx, children...)
 }
 
@@ -122,12 +123,13 @@ func armCell(t *theme.Theme, c armCol, a, base state.ArmSummary, isBase bool) (s
 		}
 		return c.plain(a), colour
 	}
-	if a.Runs == 0 {
+	if a.Runs == 0 || (c.has != nil && !c.has(a)) {
 		return "-", t.P.Faint
 	}
 	// Absolute where there is nothing to take it away from: the baseline's own
-	// row, a column with no good direction, and a baseline that did not run.
-	if isBase || c.better == 0 || base.Runs == 0 {
+	// row, a column with no good direction, and a baseline that did not run or
+	// whose chips did not count this.
+	if isBase || c.better == 0 || base.Runs == 0 || (c.has != nil && !c.has(base)) {
 		return fmt.Sprintf("%.0f%s", c.value(a), c.unit), t.P.Ink
 	}
 	ref := c.value(base)
@@ -210,6 +212,10 @@ type armCol struct {
 	value  func(state.ArmSummary) float64
 	unit   string
 	better int // -1 less is better, +1 more is better, 0 no delta
+	// has says whether the arm measured this column at all, for the chip's
+	// counters, which an emulated node or an older chip does not send. Nil
+	// means every arm that ran measured it.
+	has func(state.ArmSummary) bool
 }
 
 var armCols = []armCol{
@@ -221,6 +227,18 @@ var armCols = []armCol{
 		value: func(a state.ArmSummary) float64 { return a.RX }},
 	{title: "delivered", width: 100, better: +1,
 		value: func(a state.ArmSummary) float64 { return a.Delivered }},
+	// What the firmware let its chip report, which is where two builds that
+	// flood identically differ - so beside the delivery columns rather than at
+	// the end, where a docked panel cuts the table off. Named as the node
+	// window's radio rows are, so one word means one thing across the app.
+	// Busy time has no good direction: more of it is a firmware seeing more of
+	// the traffic that is really there.
+	{title: "IRQ suppressed", width: 130, better: -1,
+		value: func(a state.ArmSummary) float64 { return a.IRQSuppressed },
+		has:   func(a state.ArmSummary) bool { return a.HasSuppressed }},
+	{title: "busy time", width: 100, unit: " s",
+		value: func(a state.ArmSummary) float64 { return a.BusyMs / 1000 },
+		has:   func(a state.ArmSummary) bool { return a.HasBusy }},
 	{title: "redundant", width: 100, better: -1,
 		value: func(a state.ArmSummary) float64 { return a.Redundant }},
 	{title: "collisions", width: 100, better: -1,