fix: a re-plan drops the holes it invalidated and restarts the tick they are addressed by
rebuild_locked() replaced the store, so produced() restarted at 0 while the producer's tick_ kept counting: a hole's slot and tick addressing then described a buffer that was gone, forget_holes_before() could not retire it, and a backfill's amend fell outside the ring -- repairing nothing while the hole was still marked repaired, which is the band reporting the opposite of the truth. A re-plan generation is the signal rather than the stride/channel-count guard, which a reconnect with an identical CONFIG leaves unchanged. slot_base_ is left alone: it is relative mode's clock origin and must not walk backwards when the user retypes the window. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
1 parent
176ef1df9f
commit
8320988d96
4 files changed
+41
No files matched your search
@@ -527,6 +527,25 @@ void Acquisition::on_data(const udps_frame_t* frame) {
|
||||
BufferStore& store = model_.live();
|
||||
const u64 g = part.time_stride;
|
||||
|
||||
// A re-plan replaced the store, so every ring's `produced()` restarted at 0 while `tick_`
|
||||
// kept counting. `backfill_from()` derives a slot index from a hole's `tick_begin`, which is
|
||||
// only an absolute slot index while `tick_ / period == produced()`, so the tick counter has
|
||||
// to restart with the buffer -- otherwise the amend lands outside the ring, repairs nothing,
|
||||
// and the hole is still marked repaired, which is the band saying the opposite of the truth.
|
||||
// The generation is the signal rather than the stride/channel-count comparison below: a
|
||||
// reconnect with an identical CONFIG rebuilds the store leaving both unchanged.
|
||||
// `slot_base_` is deliberately *not* reset -- it is relative mode's clock origin, samples
|
||||
// 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_) {
|
||||
seen_plan_generation_ = gen;
|
||||
tick_ = 0;
|
||||
last_tick_ = ~std::uint64_t{0};
|
||||
// Zeroing the stride makes the guard below fire too: the accumulators belong to the group
|
||||
// `tick_` was counting through, so they are stale for the same reason.
|
||||
acc_stride_ = 0;
|
||||
}
|
||||
|
||||
// The filter's groups are `g` long, so a re-plan has to start a fresh group rather than
|
||||
// finish the one it was in with the wrong length.
|
||||
if (acc_stride_ != g || acc_channels_ != channels.size()) {
|
||||
|
||||
@@ -127,6 +127,10 @@ class Acquisition {
|
||||
/// Warn-once latch for a loss past the reservation cap. A member, not a function-local
|
||||
/// `static`, matching the latch Task 2 added for the same reason.
|
||||
bool warned_outage_ = false;
|
||||
/// The re-plan generation this object last reacted to. The buffer is re-planned whenever the
|
||||
/// model rebuilds it, and the state tied to the old store (`tick_` and the accumulators) is
|
||||
/// reset when this falls behind `ScopeModel::plan_generation()`.
|
||||
unsigned seen_plan_generation_ = 0;
|
||||
std::vector<double> stamps_;
|
||||
std::vector<double> abs_stamps_;
|
||||
/// The decimation filter's state: a running sum per channel (and one for the shared
|
||||
|
||||
@@ -336,6 +336,15 @@ void ScopeModel::rebuild_channels() {
|
||||
}
|
||||
|
||||
void ScopeModel::rebuild_locked() {
|
||||
// The store is replaced below, so every hole's `slot_begin`/`tick_begin`/`x_begin` describes a
|
||||
// buffer that no longer exists: the new store's base is 0, so `forget_holes_before()` cannot
|
||||
// retire them and they would shade a band over the wrong x until the table pushed them out.
|
||||
// Dropping them is the honest answer -- the samples they reported absent are gone with the
|
||||
// history. `++plan_generation_` is what tells the producer the same thing about `tick_`, whose
|
||||
// relation to `produced()` is what addresses a backfill (see `Acquisition::on_data`).
|
||||
holes_.clear();
|
||||
++plan_generation_;
|
||||
|
||||
std::vector<double> dts;
|
||||
dts.reserve(channels_.size());
|
||||
for (const ChannelInfo& c : channels_) dts.push_back(c.dt);
|
||||
|
||||
@@ -551,6 +551,13 @@ class ScopeModel {
|
||||
/// Rebuilds buffers from the current channel list. Caller holds `mutex()`.
|
||||
void rebuild_locked();
|
||||
|
||||
/// Counts buffer re-plans: every `rebuild_locked()` bumps it. The producer caches the value
|
||||
/// it last saw and resets the state tied to the old store when it moves (see
|
||||
/// `Acquisition::on_data`). Not an atomic on purpose: `rebuild_locked()` only runs under
|
||||
/// `mutex()` and the producer holds that lock for all of `on_data`, so a plain read under the
|
||||
/// lock is correct, and a reconnect with an identical CONFIG is a re-plan this sees and the
|
||||
/// stride/channel-count comparison does not.
|
||||
unsigned plan_generation() const noexcept { return plan_generation_; }
|
||||
|
||||
private:
|
||||
void rebuild_channels();
|
||||
@@ -614,6 +621,8 @@ class ScopeModel {
|
||||
Partition part_;
|
||||
std::unique_ptr<BufferStore> store_;
|
||||
std::vector<Hole> holes_;
|
||||
/// See `plan_generation()`. Written and read under `mutex()` only.
|
||||
unsigned plan_generation_ = 0;
|
||||
TriggerConfig trigger_{};
|
||||
StreamStats stats_{};
|
||||
f64 epoch_ = 0.0;
|
||||
|
||||
Reference in new issue
Block a user