From 44fc4cdf36beb2164f49f583295121c5bb8ed52a Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Fri, 2 Oct 2026 14:03:57 +0200 Subject: [PATCH] improved timing acq --- CLAUDE.md | 8 ++++ README.md | 11 ++++++ src/acquisition.cpp | 92 +++++++++++++++++++++++++++++++++++++++------ src/acquisition.hpp | 16 +++++++- src/model.cpp | 14 +++++++ src/model.hpp | 14 +++++++ 6 files changed, 142 insertions(+), 13 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 334f70f..37746dc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -229,6 +229,14 @@ Two lines of the self-check output are assertions in disguise: first run's values, and the signals it names must come back *acquired* (a `row 0 slot 0 -> channel ... samples=N` line). Delete the file before a run whose layout you did not mean to inherit (`127.0.0.1_9540.toml` and friends accumulate otherwise). +- **The export's `time`/`stream_time` columns must be strictly increasing and evenly + spaced.** Check the written file, not just the probe: `np.diff(t)` with no `<= 0` and + `rows / (t[-1] - t[0])` equal to the stored rate (the `ch` line's figure). Duplicate or + backwards stamps mean the recorded timeline holds two clocks or a collapsed cadence — + the two ways this has broken: a per-frame choice of time source (now sticky, see the + README) and a per-sample step derived from the client's `lost` counter (now the + stream's measured arrival cadence). A `rows=` count far below the span times the rate + is the other signature, that of a table cut from the LOD pyramid. - `badge frames: lit=L dim=D idle=I` — the activity badge's phase per frame. Over a run that acquired anything **both `lit` and `dim` must be non-zero**: that is what "it blinks" means, and a badge stuck lit is indistinguishable from a working one in a diff --git a/README.md b/README.md index 8e1fe63..b77ae2e 100644 --- a/README.md +++ b/README.md @@ -699,6 +699,17 @@ 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* + 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 + the frames were 100 ms apart). ## Limitations diff --git a/src/acquisition.cpp b/src/acquisition.cpp index ed0295f..aa1d24c 100644 --- a/src/acquisition.cpp +++ b/src/acquisition.cpp @@ -138,9 +138,31 @@ std::uint32_t Acquisition::build_stamps(const udps_frame_t* frame, int time_sign double period = model_.partition().base_dt; if (!(period > 0.0)) period = 1.0e-3; + // The cadence of the samples actually stored: the partition's slot interval (base + // tick times the clock's decimation), as *declared* by the CONFIG. + const double slot = model_.partition().slot_dt(); + double cadence = slot > 0.0 ? slot : period; + // A declared rate is a claim the stream may not keep (a CONFIG declaring 1 MS/s + // while delivering a hundred samples every fifth of a second is the case that + // exposed this): stamps spaced by the declaration then span microseconds inside a + // frame and jump by the frame interval between frames, which reads as a staircase + // in the x column and, in an export, as data that is "downsampled" while the + // values are fine. When the arrivals disagree with the declaration, the arrivals + // are the truth -- the mean spacing of the samples that actually arrived. + if (have_arrival_interval_ && n > 0) { + const double measured = arrival_interval_ / double(n); + const bool agrees = cadence > 0.0 && measured >= 0.75 * cadence && measured <= 1.25 * cadence; + if (measured > 0.0 && std::isfinite(measured) && !agrees) { + cadence = measured; + } + } bool used_stream_time = false; - if (time_signal >= 0 && time_signal < static_cast(frame->num_signals)) { + // Once the stream has been judged to have no usable time base, it keeps the + // arrival clock for the rest of its life (until a CONFIG says otherwise): letting + // the decision flip per frame is what interleaved two timelines in one ring. + if (time_source_ != 2 && time_signal >= 0 && + time_signal < static_cast(frame->num_signals)) { const udps_signal_t& ts = frame->signals[time_signal]; // How the client indexes a signal's values depends on its shape // (`udps_frame_value`): a scalar carries one value per batched cycle, so the @@ -170,7 +192,7 @@ std::uint32_t Acquisition::build_stamps(const udps_frame_t* frame, int time_sign case UDPS_TIME_LAST_SAMPLE: { const double anchor = value_of(n > 0 ? n - 1 : 0); for (std::uint32_t k = 0; k < n; ++k) - stamps_[k] = anchor - double(n - 1 - k) * period; + stamps_[k] = anchor - double(n - 1 - k) * cadence; used_stream_time = true; break; } @@ -233,15 +255,59 @@ std::uint32_t Acquisition::build_stamps(const udps_frame_t* frame, int time_sign if (!have_arrival_epoch_) { arrival_epoch_ = end; have_arrival_epoch_ = true; + } else { + const double elapsed = end - arrival_prev_; + if (elapsed > 0.0 && std::isfinite(elapsed)) { + // Smoothed: the socket clock jitters by a millisecond or so per frame, and + // an unsmoothed step would make every frame's span disagree slightly. + arrival_interval_ = have_arrival_interval_ ? 0.8 * arrival_interval_ + 0.2 * elapsed + : elapsed; + have_arrival_interval_ = true; + } } + arrival_prev_ = end; const double base = end - arrival_epoch_; - double step = period; - if (have_recv_ && n > 0) { - const double elapsed = end - last_recv_; - const double cycles = double(n) * double(frame->lost + 1); - if (elapsed > 0.0 && cycles > 0.0) step = elapsed / cycles; - } - for (std::uint32_t k = 0; k < n; ++k) stamps_[k] = base - double(n - 1 - k) * step; + // The spacing between two *stored* samples is the partition's own cadence -- + // the exact interval the ingest filter decimates to, which is `slot_dt()` and + // not the base tick. Deriving it from the arrival gap divided by + // `n * (lost + 1)` collapsed the step to ~zero whenever the client reported + // losses, and a whole frame then shared one f32 instant: the x column became a + // staircase (many rows at one time, then a jump) while the values were fine -- + // which is what an export of it looks like, and what the plot's crosshair and + // trigger times were reading. The arrival clock still anchors the frame's LAST + // sample, so a lost packet or a stalled producer still shows up as a jump + // *between* frames. + // The frame's stamps start at the later of two anchors: where the arrival clock + // puts the frame's first sample, and one cadence past the previous frame's last. + // The second is what keeps the timeline strictly increasing -- anchoring the + // frame's *end* at the arrival let a cadence estimate even slightly long reach + // back past the previous end, and the monotonic clamp then flattened the overlap + // into equal timestamps (a consumer plotting against them draws a vertical line + // there, and an export reads as if sampled twice at one instant). A gap still + // shows: when the arrivals run ahead, the first anchor wins and the frame jumps. + const double base_rel = base - model_.epoch(); + const double start = std::max(base_rel - double(n - 1) * cadence, last_stamp_ + cadence); + for (std::uint32_t k = 0; k < n; ++k) stamps_[k] = start + double(k) * cadence + model_.epoch(); + } + + // A change of time source invalidates the running mark: the stream-time branch + // stamps in the time signal's own values and the arrival branch in seconds since + // the first packet, and the two timelines share no zero. Leaving the mark at the + // old source's last value made every arriving frame clamp flat onto it -- which is + // the 2.5 s plateau of one timestamp that a run reports when its declared time + // signal turns out unusable and the app falls back mid-stream. + 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. + 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 @@ -262,8 +328,6 @@ std::uint32_t Acquisition::build_stamps(const udps_frame_t* frame, int time_sign } } - last_recv_ = frame->recv_time; - have_recv_ = true; return n; } @@ -301,6 +365,12 @@ 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]); diff --git a/src/acquisition.hpp b/src/acquisition.hpp index 494840b..e65dac5 100644 --- a/src/acquisition.hpp +++ b/src/acquisition.hpp @@ -64,8 +64,6 @@ class Acquisition { std::uint64_t tick_ = 0; ///< base ticks consumed since the epoch std::uint64_t last_tick_ = ~std::uint64_t{0}; double last_stamp_ = 0.0; ///< last relative time written (monotonic guard) - double last_recv_ = 0.0; - bool have_recv_ = false; /// Last accepted stream-time stamp: a "time signal" whose values do not advance /// across packets (a producer that restarts them per packet, or a mislabelled /// signal) would freeze the whole time base -- the trigger's hold-off clock @@ -76,6 +74,20 @@ class Acquisition { /// small seconds (the time ring stores f32, which cannot hold Unix epochs). double arrival_epoch_ = 0.0; bool have_arrival_epoch_ = false; + /// Smoothed interval between two frames' arrivals, and the previous arrival: the + /// spine of the arrival-anchored clock (see `build_stamps`). + double arrival_interval_ = 0.0; + double arrival_prev_ = 0.0; + bool have_arrival_interval_ = false; + /// The stream's time source, chosen once and sticky: 0 undecided, 1 the declared + /// time signal, 2 arrival. Re-evaluated only on a CONFIG. A per-frame choice + /// interleaved two timelines in one ring -- some frames stamped from a declared + /// 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 d9345d6..f3c73e0 100644 --- a/src/model.cpp +++ b/src/model.cpp @@ -358,6 +358,20 @@ 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_event(double pre_s, double post_s, double edge) { auto snap = std::make_shared(); BufferStore& src = live(); diff --git a/src/model.hpp b/src/model.hpp index cff4a65..fdd6c05 100644 --- a/src/model.hpp +++ b/src/model.hpp @@ -204,6 +204,20 @@ 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