diff --git a/README.md b/README.md index b09c950..1464e84 100644 --- a/README.md +++ b/README.md @@ -488,6 +488,37 @@ fallback when the stream provides no usable time signal (it is then the sample's via the arrival estimate); it is never used when the stream's own time signal carries a span. +**One bad packet is bridged; only a run of them changes the clock.** A real device's +clock glitches, and a packet whose stamps jump far ahead of the previous packet's last +is a fault rather than a gap — so a jump of more than **256 packet spans** is rejected +alongside the three checks that were always there (a stamp that is not finite, a packet +with no span at all, a packet that does not advance past the previous one). A rejected +packet is **bridged**: its stamps are replaced by a continuation of the accepted timeline +at the declared cadence, so the next good packet is weighed against an un-poisoned +anchor and is accepted normally. Only **four consecutive** rejected packets abandon the +declared time signal for the arrival clock, because that is what a signal which is not a +time base at all looks like. The declared unit (`ns`, `us`, `ms`) is resolved *before* +these checks rather than after, since they weigh the stamps against a cadence in seconds. + +Believing a glitch instead is what produced *"a big spike in one channel and the +acquisition seems to reset"*. The stamps pass through a monotonic clamp, so a glitched +value poisoned the mark and pinned every later packet flat at it — the stream froze at one +x and never recovered — and the glitch also condemned the *next*, perfectly good packet +(now below the poisoned anchor, so it failed the advance test), which threw the whole +stream onto the arrival clock. `tools/mock_udpstreamer.py --time-glitch-at +--time-glitch-by ` injects the fault; with a 10 s hold-off the two latches either side +of it are 11.2 s apart, where before the fix they were 1.2 s apart. + +**Switching to the arrival clock does not drop history.** `arrival_epoch_` is derived from +the monotonic mark, so the arrival clock *continues* from it and the two timelines share a +zero by construction — the recorded stamps are therefore still on one timeline and are +kept. They used to be wiped 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 restarts from 0 with no change from the user"*. The same continuity is why +`TriggerEngine` needs no rebase across the switch: it holds four absolute stamps +(hold-off, pending edge, last trigger, last sample) and they stay valid. Anything that +makes the clock genuinely *discontinuous* would break that and would have to reset them. + **Trigger: what freezes is the view, not acquisition.** `TriggerEngine` runs the edge/hold-off/normal-single state machine on the selected channel, and once the requested post-trigger share of the *event span* (the event-window control, not the @@ -577,6 +608,23 @@ after a rebuild — a CONFIG, enabling the signal, or a calibration change all d history — which is exactly when the first event used to latch on an almost empty pre window and draw half a plot. +**The engine identifies its trigger channel by name, never by index**, and that is what +makes the hold-off hold. `TriggerEngine::configure()` drops an open capture when the +trigger moves to a different channel, and it is re-called on *every* packet. The config's +channel field is an index into the channel list, and that index is not stable: every +rebuild of the list renumbers it — enabling a signal that sorts before the trigger's own +is enough — and the model then 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: with a 1 s +window, a 10 % pre share and a 10 s hold-off, an edge 0.6 s into a capture stole it. That +needed no glitch and no user action beyond enabling a channel. A deliberate re-point still +resets, because the name changes with the source. Since the rebuild also replaces the +buffer, 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 rather than a defect: `[-pre_s, post_s]` still describes the samples the snapshot +holds. `UDPSCOPE_PROBE_TRIGRENUM_MS` drives the whole sequence through the real widgets +and prints `t0_held=1` as the assertion. + The event is published on the sample that latched it, but only after every channel of that sample has reached its ring: the cut is clamped to the shortest value ring, so publishing from the middle of the channel loop dropped the newest sample — the very one @@ -1833,13 +1881,15 @@ now — they are listed so they are not mistaken for anything else: - **The time reference is the first *enabled* channel's signal.** A CONFIG whose time signal belongs to a signal that is disabled contributes no stream time, and the app falls back to arrival-based timing (with a warning on stderr). -- **The choice of time source is sticky, and a change of source empties the rings.** - It is decided once (and re-decided on a CONFIG) and then kept: choosing per frame - let a stream whose declared time signal fails one check and passes the next - alternate between two timelines, so one ring held stamps from both and the x axis, - the crosshair and an export read nonsense — a plateau of one timestamp, then a jump - to another epoch. On the switch the recorded history is dropped rather than left - half on each clock. The stamps themselves are spaced by the stream's *measured* +- **The choice of time source is sticky.** It is decided once (and re-decided on a + CONFIG) and then kept: choosing per frame let a stream whose declared time signal + fails one check and passes the next alternate between two timelines, so one ring held + stamps from both and the x axis, the crosshair and an export read nonsense — a plateau + of one timestamp, then a jump to another epoch. The switch keeps the recorded history, + because the arrival clock is anchored to continue from the monotonic mark and the two + timelines therefore share a zero (see "Switching to the arrival clock does not drop + history" above); it used to empty the rings, which is a worse reading of the same + concern. The stamps themselves are spaced by the stream's *measured* arrival cadence whenever the declared rate disagrees with it, because a CONFIG's rate is a claim the stream may not keep (one that declared 1 MS/s and delivered a hundred samples every fifth of a second made every frame span microseconds while diff --git a/docs/claude/invariants-data-path.md b/docs/claude/invariants-data-path.md index f6bcfe4..1f7d25d 100644 --- a/docs/claude/invariants-data-path.md +++ b/docs/claude/invariants-data-path.md @@ -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. diff --git a/docs/claude/invariants-trigger-and-pinning.md b/docs/claude/invariants-trigger-and-pinning.md index 6c6dfb0..86105f8 100644 --- a/docs/claude/invariants-trigger-and-pinning.md +++ b/docs/claude/invariants-trigger-and-pinning.md @@ -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 diff --git a/docs/claude/probe-readings.md b/docs/claude/probe-readings.md index c36fe86..88a47a4 100644 --- a/docs/claude/probe-readings.md +++ b/docs/claude/probe-readings.md @@ -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 diff --git a/docs/claude/verification.md b/docs/claude/verification.md index bbf77b9..31d9fc4 100644 --- a/docs/claude/verification.md +++ b/docs/claude/verification.md @@ -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 --time-glitch-by ` 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/_.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` diff --git a/src/acquisition.cpp b/src/acquisition.cpp index 972c3aa..5ded4c1 100644 --- a/src/acquisition.cpp +++ b/src/acquisition.cpp @@ -208,47 +208,98 @@ std::uint32_t Acquisition::build_stamps(const udps_frame_t* frame, int time_sign } } + // A stream-time signal may be declared in a time unit rather than seconds ('ns' is + // the common one). The scale has to be known *here*, before the checks below, because + // they weigh the stamps' own spacing against the declared cadence, which is in + // seconds: on a 'ns' stream an unscaled comparison is off by a factor of 1e9, so + // every packet would read as an implausible jump. + double unit_scale = 1.0; + if (time_signal >= 0 && time_signal < static_cast(model_.signals().size())) { + const std::string& unit = model_.signals()[static_cast(time_signal)].unit; + if (unit == "ns") unit_scale = 1e-9; + else if (unit == "us" || unit == "\u00b5s" || unit == "\xce\xbc" "s") unit_scale = 1e-6; + else if (unit == "ms") unit_scale = 1e-3; + } + if (used_stream_time && n > 0) { + // One sample's worth of the time signal's *own* unit, which is what a synthesised + // continuation has to be spaced by and what a jump is measured against. + const double step = cadence / unit_scale; + // How far a packet's stamps may run ahead of the previous packet's last stamp + // before the jump is read as a fault in the producer's clock rather than a gap in + // the stream. Counted in packets, because that is what a loss burst costs: 256 + // packet spans in one go is an outage, not loss. Letting a real glitch through is + // unrecoverable -- the monotonic clamp below then pins every later packet flat at + // the glitched value, so the whole stream freezes at one x. + constexpr double kGlitchPackets = 256.0; + // How many packets in a row must fail before the declared time signal is abandoned + // for the arrival clock. A glitch is one packet; a signal that is not a time base + // at all fails every one of them. + constexpr unsigned kFailureLimit = 4; + // A time signal that does not actually vary -- a vector time signal, whose // values the client reads one element at a time, or a producer that leaves it // zero -- would put every sample at one x and collapse the axis to a point. // The arrival estimate is a worse clock but a usable one; a degenerate one is // not a time base at all. - bool usable = std::isfinite(stamps_[0]); + const char* fault = nullptr; double lo = stamps_[0]; double hi = stamps_[0]; - for (std::uint32_t k = 1; usable && k < n; ++k) { + if (!std::isfinite(stamps_[0])) fault = "carries a stamp that is not finite"; + for (std::uint32_t k = 1; fault == nullptr && k < n; ++k) { if (!std::isfinite(stamps_[k])) { - usable = false; + fault = "carries a stamp that is not finite"; break; } lo = std::min(lo, stamps_[k]); hi = std::max(hi, stamps_[k]); } - if (usable && hi > lo) { - // The stamps must also ADVANCE across packets. A producer that restarts its - // stamps every packet (or a signal mislabelled as time) keeps handing back - // the same values; the max-clamp would then pin every sample at one x, and - // the trigger's hold-off clock with it -- the engine waited forever and no - // second event ever fired. - if (have_stream_anchor_ && !(stamps_[n - 1] > last_stream_anchor_)) { - usable = false; - } else { - last_stream_anchor_ = stamps_[n - 1]; - have_stream_anchor_ = true; + if (fault == nullptr && !(hi > lo)) fault = "carries no span (every stamp is equal)"; + if (fault == nullptr && have_stream_anchor_) { + // The stamps must also ADVANCE across packets, and by a plausible amount. A + // producer that restarts its stamps every packet (or a signal mislabelled as + // time) keeps handing back the same values; the max-clamp would then pin every + // sample at one x, and the trigger's hold-off clock with it -- the engine waited + // forever and no second event ever fired. + if (!(stamps_[n - 1] > last_stream_anchor_)) { + fault = "does not advance across packets"; + } else if (stamps_[0] - last_stream_anchor_ > kGlitchPackets * double(n) * step) { + fault = "jumped implausibly far ahead of the previous packet"; } } - if (!usable || !(hi > lo)) { - used_stream_time = false; // no span to time with; fall back to arrival + if (fault == nullptr) { + stamp_failures_ = 0; + last_stream_anchor_ = stamps_[n - 1]; + have_stream_anchor_ = true; + } else if (have_stream_anchor_ && ++stamp_failures_ < kFailureLimit) { + // A short run of bad packets is bridged rather than believed: the stamps are + // replaced by a continuation of the accepted timeline at the declared cadence, + // so neither the monotonic mark nor the anchor is poisoned, and the next good + // packet -- which carries on from where the true timeline was -- is accepted + // normally. Believing one glitching packet instead is what condemned the *next*, + // perfectly good one and threw the whole stream onto the arrival clock. + const double from = last_stream_anchor_; + for (std::uint32_t k = 0; k < n; ++k) stamps_[k] = from + double(k + 1) * step; + last_stream_anchor_ = stamps_[n - 1]; + static bool bridged = false; + if (!bridged) { + bridged = true; + std::fprintf(stderr, + "[udpscope] warning: the stream's time signal %s (%g..%g against a " + "previous %g); bridging that packet at the declared cadence and " + "keeping the stream clock.\n", + fault, lo, hi, from); + } + } else { + used_stream_time = false; // not a time base; fall back to arrival static bool warned = false; if (!warned) { warned = true; std::fprintf(stderr, - "[udpscope] warning: the stream's time signal carries no usable span " - "(%g..%g) or does not advance across packets; timing samples by " - "arrival instead. Check the time signal's mode and index in the " - "CONFIG dump above.\n", - lo, hi); + "[udpscope] warning: the stream's time signal %s (%g..%g); timing " + "samples by arrival instead. Check the time signal's mode and index " + "in the CONFIG dump above.\n", + fault, lo, hi); } } } @@ -306,33 +357,25 @@ std::uint32_t Acquisition::build_stamps(const udps_frame_t* frame, int time_sign if (time_source_ == 0) { time_source_ = used_stream_time ? 1 : 2; } else if (time_source_ == 1 && !used_stream_time) { - // The declared time signal failed its checks: keep the arrival clock from here - // on, discard the stamps already recorded on the old timeline, and make the - // arrival clock continue from where the mark is, so the axis has no step. + // The declared time signal failed its checks for good: keep the arrival clock from + // here on, anchored so that it continues from the monotonic mark. The two timelines + // then share a zero *by construction*, which is the whole point of deriving the + // epoch from `last_stamp_` -- so the recorded history is still on one timeline and + // is kept. It used to be wiped here, on the reasoning that the old stamps belonged + // to a dead clock; they do not, and the wipe is what made a single glitching packet + // look like the stream restarting from zero. time_source_ = 2; const double end = frame->recv_time; arrival_epoch_ = end - (last_stamp_ + model_.epoch()); arrival_prev_ = end; - last_stamp_ = 0.0; - history_reset_pending_ = true; } - // A stream-time signal may be declared in a time unit rather than seconds - // ('ns' is the common one): scale it to seconds so the x axis is in seconds - // either way. The scale applies ONLY to stream time: the arrival fallback - // already produces seconds off the socket clock -- scaling those by the time - // signal's unit shrank the whole time base by 1e9 on 'ns' streams, which - // froze the trigger's hold-off clock and pinned the axis at ~0. - if (used_stream_time && time_signal >= 0 && - time_signal < static_cast(model_.signals().size())) { - double unit_scale = 1.0; - const std::string& unit = model_.signals()[static_cast(time_signal)].unit; - if (unit == "ns") unit_scale = 1e-9; - else if (unit == "us" || unit == "\u00b5s" || unit == "\xce\xbc" "s") unit_scale = 1e-6; - else if (unit == "ms") unit_scale = 1e-3; - if (unit_scale != 1.0) { - for (std::uint32_t k = 0; k < n; ++k) stamps_[k] *= unit_scale; - } + // The declared unit, applied last so the x axis is in seconds either way. It applies + // ONLY to stream time: the arrival fallback already produces seconds off the socket + // clock -- scaling those by the time signal's unit shrank the whole time base by 1e9 + // on 'ns' streams, which froze the trigger's hold-off clock and pinned the axis at ~0. + if (used_stream_time && unit_scale != 1.0) { + for (std::uint32_t k = 0; k < n; ++k) stamps_[k] *= unit_scale; } return n; @@ -371,12 +414,6 @@ void Acquisition::on_data(const udps_frame_t* frame) { } const std::uint32_t n = build_stamps(frame, time_signal); if (n == 0) return; - // A change of time source invalidated the recorded history: empty the rings before - // anything more goes in, so no reader can find two timelines under one index. - if (history_reset_pending_) { - history_reset_pending_ = false; - model_.reset_live_history_locked(); // this thread holds the model lock - } if (model_.epoch() == 0.0) { model_.set_epoch_if_unset(stamps_[0]); @@ -427,7 +464,14 @@ void Acquisition::on_data(const udps_frame_t* frame) { const double post_want = tc.post_span_s(event_span); const double oldest_s = store.time().size() > 0 ? double(store.time()[0]) : last_stamp_; - engine.configure(tc, post_want, event_span - post_want, oldest_s); + // The channel's *name* goes with it, because that is what identifies it across a + // rebuild of the channel list (see `TriggerEngine::configure`). + static const std::string kNoTriggerChannel; + const std::string& trig_name = + trig_channel >= 0 && trig_channel < static_cast(channels.size()) + ? channels[static_cast(trig_channel)].name + : kNoTriggerChannel; + engine.configure(tc, trig_name, post_want, event_span - post_want, oldest_s); // The engine's armed state follows the panel's, both ways: without the disarm // half, Disarm left the engine firing (its own `armed_` was only ever set), and // re-entering trigger mode kept the stale prev/edge/hold-off state. diff --git a/src/acquisition.hpp b/src/acquisition.hpp index 024c2fc..96898bf 100644 --- a/src/acquisition.hpp +++ b/src/acquisition.hpp @@ -70,6 +70,11 @@ class Acquisition { /// included -- so the stream source is dropped in favour of arrival time. double last_stream_anchor_ = 0.0; bool have_stream_anchor_ = false; + /// Consecutive packets whose stream-time stamps failed those checks. A single bad + /// packet is a glitch in the producer's clock and is bridged at the declared cadence; + /// only a run of them means the declared signal is not a time base, and only then is + /// the arrival clock taken. Any accepted packet resets it. + unsigned stamp_failures_ = 0; /// Arrival-clock origin: the first packet's receive time, so arrival stamps are /// small seconds (the time ring stores f32, which cannot hold Unix epochs). double arrival_epoch_ = 0.0; @@ -85,9 +90,6 @@ class Acquisition { /// signal carrying volt-like values, others from the arrival clock -- which is a /// time base no reader can make sense of (see `build_stamps`). int time_source_ = 0; - /// Set when the source changed under the recorded history: the rings have to be - /// emptied before more samples go in. - bool history_reset_pending_ = false; std::vector stamps_; /// The decimation filter's state: a running sum per channel (and one for the shared /// clock) over the group of base samples that becomes one stored sample. A boxcar diff --git a/src/model.cpp b/src/model.cpp index b70a9f5..a82012e 100644 --- a/src/model.cpp +++ b/src/model.cpp @@ -501,20 +501,6 @@ void ScopeModel::live_window_locked(double lo_s, double hi_s, u64& first, u64& l if (last < first) last = first; } -void ScopeModel::reset_live_history() { - std::lock_guard lock(mutex_); - reset_live_history_locked(); -} - -void ScopeModel::reset_live_history_locked() { - BufferStore& src = live(); - src.time().clear(); - for (std::size_t c = 0; c < channels_.size(); ++c) { - src.values(c).clear(); - src.channel(c).invalidate(); - } -} - void ScopeModel::publish_pause_view(double lo_s, double hi_s, double x_offset) { // Timed from before the lock: the wait for it is part of what freezing a view costs the thread // doing it, and this one is the UI's. diff --git a/src/model.hpp b/src/model.hpp index 9795af6..bbd8b1d 100644 --- a/src/model.hpp +++ b/src/model.hpp @@ -285,20 +285,6 @@ class ScopeModel { /// is armed. BufferStore& live(); - /// Forgets the live buffer's history: every ring is emptied and every channel's - /// published window is dropped, so the two stay index-aligned. Used when the - /// acquisition has to change time source (see `Acquisition::build_stamps`): the - /// stamps already recorded are on the old timeline, and a ring holding two - /// timelines is not a time base at all -- the x axis, the crosshair and an export - /// all read it by index. - /// - /// Takes `mutex()` itself: call it from the GTK thread. The acquisition thread - /// calls `reset_live_history_locked()` instead, already holding the lock (the - /// same split `publish_event()` follows). - void reset_live_history(); - /// The body of the above for a caller that already holds `mutex()`. - void reset_live_history_locked(); - /// Builds the captured event from the slice of the live buffer covering /// `[edge - pre_s, edge + post_s]` in stream seconds, and publishes it atomically. /// The stamps are re-based so the edge sits at x = 0. The plot binds to this diff --git a/src/trigger.cpp b/src/trigger.cpp index 16fa3cf..f199919 100644 --- a/src/trigger.cpp +++ b/src/trigger.cpp @@ -12,9 +12,10 @@ double TriggerConfig::post_span_s(double buffer_span_s) const { return buffer_span_s * (1.0 - pre); } -void TriggerEngine::configure(const TriggerConfig& cfg, double post_seconds, - double pre_seconds, double oldest_s) { - const bool channel_changed = cfg.channel != cfg_.channel; +void TriggerEngine::configure(const TriggerConfig& cfg, const std::string& channel_name, + double post_seconds, double pre_seconds, double oldest_s) { + const bool channel_changed = channel_name != channel_name_; + if (channel_changed) channel_name_ = channel_name; cfg_ = cfg; post_seconds_ = post_seconds > 0.0 ? post_seconds : 0.0; pre_seconds_ = pre_seconds > 0.0 ? pre_seconds : 0.0; diff --git a/src/trigger.hpp b/src/trigger.hpp index b9efe4b..c7817dd 100644 --- a/src/trigger.hpp +++ b/src/trigger.hpp @@ -4,6 +4,7 @@ #pragma once #include +#include namespace udpscope { @@ -39,7 +40,16 @@ struct TriggerConfig { class TriggerEngine { public: /// Configures the channel and the post-trigger window. Resets the state when the - /// channel changes. + /// trigger moves to a different channel. + /// + /// `channel_name` is that channel's name, and it -- not `cfg.channel` -- is what + /// identifies it here. The 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 the model re-resolves the trigger by name afterwards. Keying the reset on the + /// index therefore made any such rebuild abandon an open capture *and set no + /// hold-off*, so the next edge inside the post window became t0 -- which is the + /// "re-triggers mid-capture and ignores the hold-off" defect. Choosing a genuinely + /// different source still resets, because the name changes with it. /// /// `post_seconds` is the post-trigger window in **stream seconds**, the same clock /// `process()` is fed and the same clock the x axis reads, so the capture covers @@ -57,8 +67,8 @@ class TriggerEngine { /// event cannot latch onto a half-filled pre window. `oldest_s` has to be the real /// buffer's, not a timer since arming -- the ring keeps filling with the trigger /// off, so the history is usually already there and waiting would be wrong. - void configure(const TriggerConfig& cfg, double post_seconds, double pre_seconds, - double oldest_s); + void configure(const TriggerConfig& cfg, const std::string& channel_name, + double post_seconds, double pre_seconds, double oldest_s); /// Disarms without forgetting the configuration. void disarm() noexcept { armed_ = false; pending_ = false; } void arm(double now_s) noexcept; @@ -95,6 +105,7 @@ class TriggerEngine { private: TriggerConfig cfg_{}; + std::string channel_name_; ///< the trigger channel's stable identity (see `configure`). bool armed_ = false; bool have_prev_ = false; double prev_ = 0.0; diff --git a/src/ui.cpp b/src/ui.cpp index 7989b2c..f258fdb 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -8186,6 +8186,99 @@ gboolean arm_probe(gpointer user) { return G_SOURCE_REMOVE; } +/// The capture the renumber probe opened: the edge it opened on, and the latch count +/// before it, so step 3 can tell a new latch from the one already on screen. +double g_trigrenum_edge = 0.0; +std::uint64_t g_trigrenum_latches = 0; + +/// Headless self-check, step 3: waits for the capture to latch and reports whether the +/// edge it latched on is still the one it opened on. +/// +/// `t0_held=1` is the reading. With the engine's invalidation keyed on the trigger +/// channel's *index* rather than its name, step 2's renumber abandoned the capture and +/// set no hold-off, so the next edge inside the post window became t0 and this read 0. +gboolean trigrenum_probe_step3(gpointer user) { + App* a = static_cast(user); + if (a->window == nullptr) return G_SOURCE_REMOVE; + if (a->model->trigger_latches().load(std::memory_order_relaxed) == g_trigrenum_latches) + return G_SOURCE_CONTINUE; + const double latched = a->model->trigger_time().load(std::memory_order_relaxed); + std::fprintf(stderr, "[trigrenum-probe] latched t0=%.4f opened on %.4f t0_held=%d\n", latched, + g_trigrenum_edge, int(std::fabs(latched - g_trigrenum_edge) < 1.0e-6)); + std::fflush(stderr); + return G_SOURCE_REMOVE; +} + +/// Headless self-check, step 2: the moment a capture is open, enables a signal that sorts +/// *before* the trigger's own -- which is what renumbers the trigger channel under the +/// engine. Polls rather than times, so it lands inside the post window whatever the rate. +gboolean trigrenum_probe_step2(gpointer user) { + App* a = static_cast(user); + if (a->window == nullptr) return G_SOURCE_REMOVE; + if (!a->model->trigger_pending()) return G_SOURCE_CONTINUE; + g_trigrenum_edge = a->model->capture_edge(); + g_trigrenum_latches = a->model->trigger_latches().load(std::memory_order_relaxed); + int ch = -1; + { + std::lock_guard lock(a->model->mutex()); + ch = a->model->trigger().channel; + } + std::fprintf(stderr, "[trigrenum-probe] capture open: t0=%.4f ch=%d -- enabling signal 1 now\n", + g_trigrenum_edge, ch); + std::fflush(stderr); + a->model->set_signal_enabled(1, true); + { + std::lock_guard lock(a->model->mutex()); + ch = a->model->trigger().channel; + } + std::fprintf(stderr, "[trigrenum-probe] after enable: trigger ch=%d channels=%zu\n", ch, + a->model->channel_count()); + std::fflush(stderr); + g_timeout_add(25, trigrenum_probe_step3, a); + return G_SOURCE_REMOVE; +} + +/// Headless self-check, step 1: the reported geometry, driven through the real widgets -- +/// 1 s window, 10 % pre share, hold-off from `--holdoff` -- on the *square* alone, so that +/// enabling the sine later moves the trigger channel's index. +gboolean trigrenum_probe(gpointer user) { + App* a = static_cast(user); + if (a->window == nullptr || a->window_entry == nullptr) return G_SOURCE_REMOVE; + gtk_drop_down_set_selected(GTK_DROP_DOWN(a->window_unit_drop), 2); + gtk_editable_set_text(GTK_EDITABLE(a->window_entry), "1"); + on_window_entry_activate(GTK_ENTRY(a->window_entry), a); + // Signal 2 is the mock's square; signal 1 is its sine. Only the square to begin with. + a->model->set_signal_enabled(1, false); + a->model->set_signal_enabled(2, true); + a->model->set_signal_enabled(3, false); + if (a->model->channel_count() > 0) { + const ChannelInfo& c0 = a->model->channels()[0]; + a->rows->assign(0, 0, c0.name, c0.name, + signal_color_index(a->model->signals(), c0.signal)); + a->relayout_pending = true; + } + gtk_spin_button_set_value(GTK_SPIN_BUTTON(a->trigger_pre_spin), 10.0); + gtk_spin_button_set_value(GTK_SPIN_BUTTON(a->trigger_level_spin), 0.0); + publish_trigger_fields(a); + if (a->trigger_toggle != nullptr) + gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(a->trigger_toggle), TRUE); + { + std::lock_guard lock(a->model->mutex()); + const TriggerConfig& tc = a->model->trigger(); + std::fprintf(stderr, + "[trigrenum-probe] window=%.4f event_span=%.4f pre_pct=%.1f post=%.4f " + "holdoff=%.4f level=%.4g edge=%d mode=%d enabled=%d armed=%d ch=%d " + "channels=%zu\n", + a->rows->time_window(), a->model->event_span(), tc.pretrigger_pct, + tc.post_span_s(a->model->event_span()), tc.holdoff_s, tc.level, int(tc.edge), + int(tc.mode), int(tc.enabled), int(tc.armed), tc.channel, + a->model->channel_count()); + } + std::fflush(stderr); + g_timeout_add(25, trigrenum_probe_step2, a); + return G_SOURCE_REMOVE; +} + /// Headless self-check: changes the trigger panel's two dropdowns, which is what a user /// does to switch Normal/Single or the edge -- mid-capture, if the probe is timed into /// one. Reaching the dropdowns directly is the point: their `notify::selected` handler @@ -9581,6 +9674,9 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit if (const char* env = g_getenv("UDPSCOPE_ARM_TRIGGER_MS")) { g_timeout_add(static_cast(std::atoi(env)), arm_probe, &app); } + if (const char* env = g_getenv("UDPSCOPE_PROBE_TRIGRENUM_MS")) { + g_timeout_add(static_cast(std::atoi(env)), trigrenum_probe, &app); + } if (const char* env = g_getenv("UDPSCOPE_PROBE_FIELDS_MS")) { g_timeout_add(static_cast(std::atoi(env)), fields_probe, &app); } diff --git a/tools/mock_udpstreamer.py b/tools/mock_udpstreamer.py index 8115dfc..1b404a1 100644 --- a/tools/mock_udpstreamer.py +++ b/tools/mock_udpstreamer.py @@ -67,8 +67,15 @@ def build_config(rate, time_mode=TIME_FULL_ARRAY, unit="V"): return header(PKT_CONFIG, 0, len(payload)) + payload -def build_data(counter, t0, dt, batch, freq, time_mode=TIME_FULL_ARRAY, noise=0.0): +def build_data( + counter, t0, dt, batch, freq, time_mode=TIME_FULL_ARRAY, noise=0.0, time_spike=0.0 +): stamps = [t0 + i * dt for i in range(batch)] + # `time_spike` models a device whose clock glitches for one packet: the stamps jump + # far ahead and then the next packet carries on from where it would have been. The + # waveforms are still computed from the *true* stamps, so only the time signal is + # wrong -- which is what a glitching clock looks like on the wire. + wire_stamps = [t + time_spike for t in stamps] if time_spike > 0.0 else stamps # The interferer is a tone at 100x the fundamental with a randomly scaled amplitude, which # is the shape that is hardest on anything counting crossings: near the sine's own mid level # the fundamental's slope is 2*pi*f while the interferer's is 100x that, so the signal @@ -92,7 +99,9 @@ def build_data(counter, t0, dt, batch, freq, time_mode=TIME_FULL_ARRAY, noise=0. # signal in Accumulate mode still occupies `batch` slots on the wire (the client # reads `num_samples` values for any 1-element signal), so the anchor is repeated -- # which is exactly the shape that made udpscope derive a step of zero. - wire_time = [t0] * batch if time_mode == TIME_FIRST_SAMPLE else stamps + wire_time = ( + [wire_stamps[0]] * batch if time_mode == TIME_FIRST_SAMPLE else wire_stamps + ) payload = struct.pack(" 0.0: pdt = dt * (1.0 + random.uniform(-args.clock_jitter, args.clock_jitter)) + spike = 0.0 + if args.time_glitch_at >= 0.0 and not glitched and t_stream >= args.time_glitch_at: + spike = args.time_glitch_by + glitched = True + print( + "mock udpstreamer: time-signal glitch at t=%.3f, +%.3f s for one packet" + % (t_stream, spike), + flush=True, + ) sock.sendto( build_data( - counter, t_stream, pdt, args.batch, args.freq, time_mode, args.noise + counter, + t_stream, + pdt, + args.batch, + args.freq, + time_mode, + args.noise, + spike, ), peer, )