martino/udpscope
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: a clock that stalled on a re-plan, and a dialog that could not close

Browse Source 
**The clock.** Trigger mode showed a blank plot and never latched when the session defined a math
channel, because such a session gives the model a plan *before* the stream's own signals are in it:
one derived channel, declared rate 0, so `plan_partition()`'s 1 kHz fallback is the interval those
early frames are stamped at -- 1300x too fast for a 1.3 MS/s stream -- and they have nowhere to go,
since the value loop skips derived channels. Relative mode then stamped a *position* times the plan's
*current* interval, so when the CONFIG's channels landed and the plan took the real rate (769 ns) the
same positions produced times 1300x *smaller* than what was already in the ring, and the monotonic
clamp held the clock at the old value for the rest of the run: one repeated stamp, `history=0`, no
latch, and a window the renderer cannot draw. The same family as the restarted-client poisoning
fixed in 620473a, through a different door. The filter's own output was finite and correct
throughout, and disabling the math channel only appeared to cure it because that is a structural
change which rebuilds the store -- the third time that channel has looked guilty while being a
bystander.

Relative mode now accumulates *seconds* (`rel_base_s_ += n * dt`), so a re-plan is a slope change and
the clock cannot jump backwards by construction; a frame is only stamped into a model that has a
*streamed* channel; and the epoch is adopted once per connection, with "unset" a state rather than
the value zero. A one-shot warning names the family if it returns: `the clock stalled at ...: N
samples stamped [a..b] are all behind it`.

**The dialog.** The Configurations window's Close button and the window-manager close both did
nothing: `close-request` was connected to a bare `void` lambda, and that signal is `gboolean` -- TRUE
means "handled" and suppresses the default handler that destroys the window, while `G_CALLBACK` is a
cast, so GTK read whatever the compiler left in the return register. A standalone GTK program of that
shape reproduces it: `void` handler -> `window_alive=1` after `gtk_window_close()`, `gboolean FALSE`
-> 0. The same audit found `gtk_window_present(GTK_WINDOW(...))` on an `AdwDialog`, a type assertion
and a no-op; it is `adw_dialog_present()` now. The menu row stays: it is the only way to switch or
save a configuration without restarting, and what made it look useless was that it could not close.

Verified: trigger mode with that session's graph reads `ring=[0.0058..0.2446] -> [0.0058..1.9969]`,
`history 0.237 -> 2.0 s`, `edge at 1.120 s, latched at 2.120 s`, `display=event(frozen) latches=1`,
`traceless paints 266` against 1519 before the fix. Baselines hold: 0 errors, the documented three
warnings, test-ui 13 ok, test-help green, `gtk warnings: 0`.
This commit is contained in:
Martino Ferrari committed 2026-10-15 10:50:09 +02:00
1 parent 27c9b34277
commit 074cb611bc
9 files changed
+210 -34
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

src/acquisition.cpp
+72 -18
View File
@@ -206,6 +206,7 @@ void Acquisition::on_config(const udps_signal_t* signals, std::uint32_t count,
warned_phantom_gap_ = false;
warned_bridge_ = false;
warned_time_source_ = false;
warned_clock_stall_ = false;
// The clock choice is per connection too: a reconnect re-derives it from the frames
// the new session actually delivers, rather than trusting a decision made about a
// session that dropped.
@@ -246,6 +247,8 @@ void Acquisition::reset_session_state() noexcept {
arrival_prev_ = 0.0;
have_prev_frame_recv_ = false;
prev_frame_recv_ = 0.0;
// The clock origin is adopted again: the next session's first stamp is that session's epoch.
epoch_adopted_ = false;
}
void Acquisition::note_frame_arrival(double recv_time) noexcept {
@@ -310,7 +313,7 @@ std::uint32_t Acquisition::build_stamps(const udps_frame_t* frame, int time_sign
stamps_.assign(n, 0.0);
abs_stamps_.assign(n, 0.0);
// Samples per packet, needed in both modes: relative stamping must advance `slot_base_`
// Samples per packet, needed in both modes: relative stamping must advance its own clock
// past the reserved hole before placing this frame's own stamps, and `on_data` must size
// the hole before calling this function -- both operations require knowing how many samples
// a missing packet represents, which is only knowable from the first frame that arrives.
@@ -338,27 +341,32 @@ std::uint32_t Acquisition::build_stamps(const udps_frame_t* frame, int time_sign
}
}
// Relative mode: the stamp is a pure function of position and the *declared* rate. Nothing
// below this block runs -- not the arrival-cadence override, not the degeneracy guard, not
// the glitch bridge, not the monotonic clamp's reason for existing -- because a position
// cannot jitter, cannot fail to advance and cannot jump.
// Relative mode: the stamp is the *declared* interval counted from the stream's own origin.
// Nothing below this block runs -- not the arrival-cadence override, not the degeneracy guard,
// not the glitch bridge, not the monotonic clamp's reason for existing -- because a counted
// clock cannot jitter, cannot fail to advance and cannot jump.
//
// The clock is **accumulated in seconds**, never recomputed as `position * interval`: see
// `Acquisition::rel_base_s_` for the defect that distinction cost. The interval is read from the
// plan, which is the stream's declared rate as soon as a streamed channel exists -- and
// `on_data` refuses a frame until one does, so the fallback interval can never be the origin.
if (cfg_.relative_time) {
double dt = model_.partition().base_dt;
if (!(dt > 0.0)) dt = 1.0e-3;
// The reserved hole for the missing packets advances `slot_base_` so this frame's
// own stamps land *after* the hole rather than on top of it. The same conditions
// the reservation call uses: not late, gap present, batch known, within the cap --
// and not one the arrival clock or the ring refused, because the advance and the
// reservation are the *same* claim about the wire: believing a phantom gap here
// would stretch the timeline by exactly the loss that did not happen.
// The reserved hole for the missing packets advances the clock so this frame's own stamps land
// *after* the hole rather than on top of it. The same conditions the reservation call uses: not
// late, gap present, batch known, within the cap -- and not one the arrival clock or the ring
// refused, because the advance and the reservation are the *same* claim about the wire:
// believing a phantom gap here would stretch the timeline by exactly the loss that did not
// happen.
if (!frame->late && frame->lost > 0 && batch_ > 0 && !counter_gap_refused &&
double(frame->lost) <= kGlitchPackets) {
slot_base_ += u64(frame->lost) * batch_;
rel_base_s_ += double(frame->lost) * double(batch_) * dt;
}
for (std::uint32_t k = 0; k < n; ++k) {
stamps_[k] = double(slot_base_ + k) * dt;
stamps_[k] = rel_base_s_ + double(k) * dt;
}
slot_base_ += n;
rel_base_s_ += double(n) * dt;
// The absolute clock is still read, for export only. When the stream carries no usable
// time signal there is no absolute clock to record and the relative stamp is the honest
// answer -- an export then says what the counter says, which is all anyone had.
@@ -632,7 +640,23 @@ void Acquisition::on_data(const udps_frame_t* frame) {
const Partition& part = model_.partition();
const std::vector<ChannelInfo>& channels = model_.channels();
if (part.time_capacity == 0 || channels.empty()) return;
// A frame is only stored into a model that has a **streamed** channel. A model holding only
// *derived* ones -- a session's math channels, applied before the stream's own have arrived, or
// a file's channels while the math is re-applied -- has nowhere to put a streamed sample, and its
// plan is built from the 1 kHz fallback interval, which is **not the stream's rate**. Accepting
// such a frame (which the `channels.empty()` test below did) stamps the timeline at that fallback,
// and the frame after the stream's own channels arrive makes every later stamp *behind* the one
// already written -- the monotonic clamp holds the clock there for the rest of the run, the
// trigger's `history` never fills and the plot draws nothing. The reading is the `trig engine:`
// line: `n` climbing while `ring=[a..a]` stands still.
bool streamed = false;
for (const ChannelInfo& c : channels) {
if (!c.math) {
streamed = true;
break;
}
}
if (part.time_capacity == 0 || channels.empty() || !streamed) return;
// A frame delivered behind the sequence: the slots it owns were reserved as NaN by
// `reserve_hole()`, so it writes values into them and does nothing else. It is deliberately
@@ -672,7 +696,7 @@ void Acquisition::on_data(const udps_frame_t* frame) {
}
}
// The wire's own cadence, and the verdict on the counter's claim, both taken *before* the
// stamps: relative mode's `slot_base_` advance and the reservation below are the same claim
// stamps: relative mode's clock advance and the reservation below are the same claim
// about the wire (see `build_stamps`), so they have to agree about it. The verdict is taken
// before the cadence is updated, too: folding this frame's own arrival into the estimate would
// let one burst raise the very number that is supposed to corroborate it -- a 200-packet outage
@@ -687,7 +711,14 @@ void Acquisition::on_data(const udps_frame_t* frame) {
const std::uint32_t n = build_stamps(frame, time_signal, gap_refused);
if (n == 0) return;
if (model_.epoch() == 0.0) {
// The epoch -- the stream time the display's x = 0 means -- is the first frame's stamp, adopted
// **once per connection**. The test is a state and not a value: relative mode's first frame is
// stamped exactly 0, so an `epoch() == 0.0` test reads that as "no epoch yet" and lets the
// *second* frame move the origin, which stalls the timeline through that frame (the samples of
// one frame all land on one stamp) and shifts every later x by a frame's span. The flag is
// re-armed by `reset_session_state()`, because a new session's clock is a new clock.
if (!epoch_adopted_) {
epoch_adopted_ = true;
model_.set_epoch_if_unset(stamps_[0]);
std::fprintf(stderr, "[udpscope] time base: epoch=%.3f, first frame recv=%.3f, n=%u\n",
model_.epoch(), frame->recv_time, n);
@@ -717,7 +748,7 @@ void Acquisition::on_data(const udps_frame_t* frame) {
// The generation is the signal rather than the stride/channel-count comparison below: only
// a CONFIG that *changes* the stream re-plans the store -- a reconnect's identical CONFIG
// keeps it, history and all (see `ScopeModel::set_signals`).
// `slot_base_` is deliberately *not* reset -- it is relative mode's clock origin, samples
// `rel_base_s_` is deliberately *not* reset -- it is relative mode's clock origin, seconds
// since the stream began, and relative time must not jump backwards when the user retypes
// the window.
if (const unsigned gen = model_.plan_generation(); gen != seen_plan_generation_) {
@@ -906,6 +937,29 @@ void Acquisition::on_data(const udps_frame_t* frame) {
if (tc.mode == TriggerMode::Single) tc.armed = false;
};
// The monotonic clamp, and the one state it must not be allowed to sit in. `max()` is for a
// stream clock that glitches, fails to advance for a sample or arrives out of order -- one
// sample, or a few, held back. A **whole frame** whose stamps are all behind the clock is not
// that: the timeline has stopped while the wire has not, and nothing downstream can recover. It
// is staged as a reading because the picture it produces is a *blank* plot beside a healthy
// status bar, which names no cause at all: the trigger's `history` never reaches its pre-window,
// so it never latches and the display sits in `Waiting`; the renderer's bound window is empty, so
// nothing is drawn even with the trigger off. `relative_time` with a session's math channel was
// how this was reached (see `rel_base_s_`); the guard is `build_stamps`' own accumulator.
if (!warned_clock_stall_ && n > 0 && stamps_[0] - epoch < last_stamp_ &&
stamps_[n - 1] - epoch < last_stamp_) {
warned_clock_stall_ = true;
const u64 held = model_.live().time().size();
std::fprintf(stderr,
"[udpscope] warning: the clock stalled at %.6f s: %u samples stamped [%.6f..%.6f] s "
"are all behind it, so the timeline stands still while frames keep arriving -- "
"nothing can be drawn and no trigger can latch. The plan's interval is %.9g s now; "
"ring n=%llu\n",
last_stamp_, n, stamps_[0] - epoch, stamps_[n - 1] - epoch, part.base_dt,
static_cast<unsigned long long>(held));
std::fflush(stderr);
}
for (std::uint32_t k = 0; k < n; ++k) {
const u64 tick = tick_++;
// The shared clock's group boundary, read once and reused by the time ring, the channel