fix: a clock glitch is bridged and a channel is known by name, so a capture keeps its t0

Two defects behind one report ("a trigger during a capture overrides the first,
and the hold-off does not apply"); the second is also what made a spike in one
channel look like the acquisition resetting.

The trigger engine keyed its invalidation on the trigger channel's *index*, which
every rebuild of the channel list renumbers -- enabling a signal that sorts before
it is enough. Any rebuild therefore abandoned an open capture and set no hold-off,
so the next edge inside the post window became t0. It keys on the channel's name
now, which changes only when the source genuinely does.

A packet whose stamps jump more than 256 packet spans ahead is now rejected as a
fault and *bridged* at the declared cadence, instead of poisoning the monotonic
mark (which pinned every later packet flat at the glitched value) and condemning
the next good packet. Only four consecutive failures abandon the declared time
signal, and that fallback no longer wipes the rings: the arrival clock is anchored
to continue from the monotonic mark, so the history is still on one timeline --
the wipe is what made one glitching packet look like the stream restarting from 0.
The declared unit is resolved before those checks, since they weigh the stamps
against a cadence in seconds.

Guards: UDPSCOPE_PROBE_TRIGRENUM_MS prints t0_held=1 through the real widgets,
and the mock's --time-glitch-at/--time-glitch-by injects the clock fault.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Martino FerrariandClaude Opus 4.6 committed 2026-10-07 10:29:05 +02:00
1 parent 4003084648
commit f179fbbb44
13 files changed
+406 -96

No files matched your search

+30
View File
@@ -36,6 +36,36 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear
length. The LOD min/max pyramid still preserves the extremes *of what is stored*; a spike
narrower than a group is now filtered away rather than aliased, which is the correct
trade and is why the status line's `rate` field is worth reading.
- **One bad packet is bridged; only a run of them changes the time source.** The stream-time
checks in `build_stamps()` reject three things: stamps that are not finite, a packet with no
span at all, and a packet whose stamps do not advance past the previous packet's last. A
fourth was added because a real device's clock glitches: a packet whose stamps jump **more
than 256 packet spans** ahead is a fault, not a gap, and must be rejected — letting it
through poisons `last_stamp_`, and the monotonic clamp (`max(stamps[k] - epoch,
last_stamp_)`) then pins every later packet flat at the glitched value, so the whole stream
freezes at one x and never recovers. A rejected packet is **bridged**: its stamps are
replaced by a continuation of the accepted timeline at the declared cadence, so neither the
monotonic mark nor `last_stream_anchor_` is poisoned and the next good packet is accepted
normally. Believing the glitch instead is what condemned the *next*, perfectly good packet
(it now sits below the poisoned anchor, so it fails the advance test) and threw the whole
stream onto the arrival clock. Only **four consecutive** rejected packets abandon the
declared time signal, because that is what a signal which is not a time base at all looks
like; a first packet with nothing to bridge from still falls back immediately. The unit
scale ('ns', 'us', 'ms') is therefore resolved **before** these checks rather than after:
they weigh the stamps against a cadence in seconds, and on a 'ns' stream an unscaled
comparison is off by 1e9, so every packet would read as an implausible jump. The guard is
`tools/mock_udpstreamer.py --time-glitch-at`/`--time-glitch-by` (see
[verification.md](verification.md)).
- **The arrival-clock fallback does not drop history.** `arrival_epoch_` is derived from
`last_stamp_`, so the arrival clock *continues* from the monotonic mark and the two
timelines share a zero by construction — the recorded stamps are therefore still on one
timeline and are kept. The fallback used to call `reset_live_history_locked()` on the
reasoning that the old stamps belonged to a dead clock; they do not, and that wipe is what
made a single glitching packet look like "the live stream restarted from 0 with no action
from the user". The continuity is also what makes a rebase of `TriggerEngine` unnecessary:
the engine holds four absolute stamps (`holdoff_until_s_`, `pending_start_s_`,
`last_trigger_s_`, `last_t_s_`) and they stay valid across the change. Anything that makes
the clock *discontinuous* breaks that and has to reset them.
- **The trigger sees the stored (filtered) signal, not the wire signal.** `engine.process()`
is fed the averaged value, inside the `if` that pushes it. An edge found on a sample that
is not in the buffer would be cut from data that does not contain it.
@@ -30,6 +30,21 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear
out empty: any stream whose time signal advances in large steps drove it to its
1 S/s floor, the count came out as 1, and the engine latched on the edge sample
itself. **Do not reintroduce a rate estimate on this path.**
- **The engine identifies its channel by *name*, never by index.** `TriggerEngine::configure()`
drops an open capture when the trigger moves to a different channel, and it is re-called from
`Acquisition::on_data()` on **every packet** out of `model_.trigger()`. `TriggerConfig::channel`
is an index into `channels()`, and that index is not stable: every rebuild of the channel list
renumbers it — enabling a signal that sorts before the trigger's own is enough — and
`ScopeModel::set_signal_enabled()` re-resolves the trigger by name and writes the *new* index
back. Keying the reset on the index therefore made any such rebuild abandon the capture **and
set no hold-off**, so the next edge inside the post window became t0: that is the "the trigger
re-triggers during a capture and the hold-off does not apply" defect, and it needed no glitch
and no user action beyond enabling a channel. The name changes when the source genuinely
changes, so a deliberate re-point still resets. The rebuild also replaces the `BufferStore`, so
the surviving capture's pre side is short and the snapshot reports its own `pre_s` — which may
be **negative** when the new ring starts after the edge. That is a faithful reading, not a
defect: `[-pre_s, post_s]` still describes the samples the snapshot holds. The guard is
`UDPSCOPE_PROBE_TRIGRENUM_MS`, whose `t0_held=1` is the assertion.
- **A pinned window is *clamped* into the event every frame, and a zoom in is allowed.**
While an event is held, a capture is filling, or the view is paused, `tick()` calls
`RowSet::hold_pinned_window()`, which caps the span at `[-pinned_pre_, +pinned_post_]`'s and
+14
View File
@@ -94,6 +94,20 @@ Two lines of the self-check output are assertions in disguise:
`events` does not move**: no event fired, so the partial publish deliberately leaves
`trigger_latches()` alone — which is also what keeps the `trigger events: N total, M at
the disarm` line in the dump meaning what it says.
- `[trigrenum-probe] window=W event_span=S pre_pct=P post=O holdoff=H level=L edge=E mode=M
enabled=1 armed=1 ch=C channels=N`, then `capture open: t0=T ch=C -- enabling signal 1 now`,
`after enable: trigger ch=C' channels=N'`, and `latched t0=T' opened on T t0_held=D` (stderr,
`UDPSCOPE_PROBE_TRIGRENUM_MS`) — the first line is the geometry the real widgets produced
(`window=1.0000 event_span=1.0000 pre_pct=10.0 post=0.9000` for the reported setup); it is
context, not the assertion. The third must show the index **actually moving** (`ch=0` →
`ch=1`), or the probe tested nothing — the sine has to be a channel that sorts before the
square. **`t0_held=1` is the assertion**: the capture that opened on `T` latched on `T`, so the
renumber neither abandoned it nor let a later edge inside the post window become t0. With the
engine's invalidation keyed on the channel's index instead of its name this read `t0_held=0`,
with `pending` back to 0 and `holdoff_until` *behind* `now_t` on the engine line — i.e. no
hold-off was set at all, which is the whole defect. The `[udpscope] trigger: edge at ... got
[--0.0199..0.9000]` beside it is expected: the rebuild replaces the `BufferStore`, so the
surviving capture has no pre side left and reports a negative `pre_s`.
- `[export-probe] mode=.. axis=[a..b] rows=N cols=M time=[c..d] abs=[..]` (stderr,
`UDPSCOPE_EXPORT_MS`) — **`time=` must equal `axis=`** and **`rows=` must be the
span times the stream's real rate, plus one** (2 s at 10 kS/s is 20001). The export
+34
View File
@@ -44,6 +44,13 @@ precedent, not an exception.
`UDPSCOPE_TRIG_TRACE_MS` — schedule individual probes. `UDPSCOPE_PROBE_DISARM_MS` only
*starts* its sequence: it switches the trigger on and then polls for a capture to open,
so the Disarm lands ~300 ms into one rather than wherever a fixed delay happened to fall.
- `UDPSCOPE_PROBE_TRIGRENUM_MS` — the trigger's capture must survive a **renumbering of its own
channel**. It sets the reported geometry through the real widgets (1 s window, 10 % pre share,
hold-off from `--holdoff`) on the mock's *square* alone, polls for a capture to open, and then
enables the sine — which sorts before the square and so moves the trigger channel's index under
the engine. Its three lines are in [probe-readings.md](probe-readings.md); **`t0_held=1` is the
assertion**. Run it with `--holdoff 10`, which makes the stolen-t0 reading unmistakable: a second
latch barely a second after the first cannot be a hold-off that applied.
- `UDPSCOPE_PROBE_FIELDS_MS` — change the trigger panel's edge and Normal/Single
dropdowns. As much a crash test as a state check (see the GTK signal-signature invariant
below), so time it inside a capture: `[fields-probe] ... capturing=1` followed by no
@@ -170,6 +177,33 @@ precedent, not an exception.
`/tmp/udpscope-export`) — collect the export table and write it both ways, skipping
only the file chooser.
The mock also carries one **fault injection** that no probe schedules, because what it exercises is
the ingest rather than the UI: `--time-glitch-at <stream seconds> --time-glitch-by <seconds>` makes
**one** packet's time signal jump ahead and then carries on from where the true timeline was, which
is what a device whose clock glitches puts on the wire. Run it with the trigger armed, e.g.
```sh
xvfb-run -a env UDPSCOPE_ENABLE_ALL_MS=400 UDPSCOPE_ARM_TRIGGER_MS=700 UDPSCOPE_EXIT_AFTER_MS=22000 \
./build/udpscope --server 127.0.0.1 --port 8000 --holdoff 10
# mock: --rate 10000 --batch 100 --freq 5 --time-glitch-at 6.0 --time-glitch-by 10.0
```
Four readings, all off the once-a-second `[udpscope] trig engine:` line:
- exactly one `warning: ... jumped implausibly far ahead of the previous packet (16.01..16.0199
against a previous 6.0099); bridging that packet ...` — the glitch was **caught**;
- **no** `timing samples by arrival instead` — one bad packet must not change the time source;
- `now_t` crosses the glitch continuously (`5.040 → 6.050 → 7.050`), and `history` keeps **growing**
(`1.01 … 23.15`) instead of collapsing — the ring was not wiped;
- the latches stay a hold-off apart: `edge at 1.060 / latched at 2.060 / holdoff_until=12.060`, then
`edge at 12.260 / latched at 13.260 / holdoff_until=23.260` — **11.2 s** for a 10 s hold-off plus
a 0.9 s post window.
Before the fix the same run read a fallback warning, `history` collapsing 4.03 → 0.95, `now_t`
jumping 5.040 → 16.040, and two latches **1.2 s** apart. Note the window here comes from the saved
session for that port; the figures above are the default 2 s window (50 % pre), so move
`~/.config/udpscope/<host>_<port>.toml` aside to reproduce them.
**Run one self-check at a time.** Two concurrent `xvfb-run -a` instances end up on the
same display and the second gets no GL context — it dumps `layout over 0 frames` and
`valid=-1`, which looks like a pipeline failure and is not one. `tools/mock_udpstreamer.py`