The counter is the denominator of the very period lastAccHrt is the
numerator of, so any packet that cannot move the tick reference must not
move the counter either. Two paths were violating that: a reordered
datagram rolled the counter back while the reference correctly held
(next burst drawn 0.048x too narrow at distance 20, 83.3% worst spacing
error under 2% sustained reordering), and a stray hrt == 0 packet
advanced the counter from the warm-up branch without a tick to match
(+22.5 ms of future-dating per stray packet).
Rules 1 and 2 now record a counter too. The duplicate-datagram guard is
keyed on one, so an array rule that recorded none was exempt and plotted
every doubly-delivered update twice.
Also: rule 2 divides by the count the anchor actually spans and falls
back to the last period it derived; the counter-gap test is wrap-safe so
2^32 rollover reads as no information rather than 2e9 lost packets.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On the hrt branch the derived period is not just a spacing: it is the
burst width ClockOffset latches against, so a wrong one shifts the whole
trace by an amount that is usually too small for kRecalibThresholdS to
ever heal. Three routes to a wrong period were open.
Packet loss. elapsed spans every packet since the last one seen, but it
was divided by prevAccCount alone, so a lost datagram scaled the period
by the whole counter gap. Since a burst is anchored on its LAST element,
too wide means it ends in the FUTURE: +22.5 ms for one loss, +225 ms for
ten, at 10 samples per 25 ms packet, mis-spacing 2.7% of all samples at
1% loss. The declared branch already reads the counter for exactly this;
the hrt branch now does too.
Producer restart and reorder. Both leave elapsed at zero, so no period
can be measured -- and the restart packet is also the one that re-latches
after offset.reset(). Falling back to kDefaultDt is only right at 1 kHz;
measured standing displacement was +13.5 ms at 10 samples per 25 ms and
-89 ms at 100 per 10 ms. Remember the last measured period instead.
A stray hrt == 0 packet re-enters the warm-up branch, which spans from
packetBurst's lastPacketWall -- a field the hrt branch never wrote, so it
still held the start of the session. After 153 packets that emitted a
burst 3.8 s in the past, worse the longer the scope had run.
Also: rule 2 with no declared rate stacked every element of the array on
one instant (as UDPSourceSession.cpp:522 does, harmlessly, for a
host-local consumer). Spread it from consecutive time-signal anchors,
which measure the burst on the producer's own clock.
Reverts the previous commit's wallElapsed <= 0 change: it was measurably
inert -- the step floor two lines below already yields the same number --
and its comment claimed a divergence it did not stop.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
An undeclared-rate accumulated scalar is served by packetBurst until
HrtRateFit is ready, then by the hrt branch. The two place a burst
differently -- packetBurst ends it at wallNow, the hrt branch at
wallNow - (nElems-1)*hrtDt -- and the warm-up left no state behind, so
the handover packet skipped the monotonic clamp and stepped the signal
backwards by up to a burst width (-6.5 ms at 10 samples per 2.5 ms
packet, -0.99 s at 1000 samples per 10 ms).
Seeding lastEmitted* alone would only restore ordering. Without
lastAccHrt/prevAccCount the first hrt packet also has no tick delta to
measure, falls back to kDefaultDt and latches ClockOffset against a
burst width that is wrong whenever the cadence is not 1 kHz -- 89 ms of
permanent displacement at 100 samples per 10 ms, below the
recalibration threshold that would otherwise heal it. Seed both.
Also close the wallElapsed <= 0 bypass in both branches: skipping the
bleed cap when the wall has not moved hands back the full proportional
advance, letting a run of same-tick arrivals gain lead while no wall
time passes at all.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Eight review findings on FrameDecoder's accumulated-scalar rule.
The squeeze that pulls a leading timeline back was expressed as a fraction of
the NOMINAL burst width, which cannot converge: inside one timestamps() call
the wall clock is frozen, so any positive step raises the lead measured at that
instant, and the lead only falls because the wall advances between packets. At
the kMinBleedFactor floor the timeline still gained 0.05 * nominal per packet,
so a declared SamplingRate of 30 against a producer really flushing 10 samples
at 1 kHz ran away without bound (667 s of lead after 1000 s of stream). Cap the
burst's total advance at half the wall time really elapsed since this signal's
previous burst instead, and the lead strictly falls for any declared rate.
SigState gained lastEmittedWall for that reference; lastPacketWall could not be
reused because it belongs to packetBurst.
The hrt branch contributed zero elapsed for a late datagram but still wrote the
hrt reference back to it, so the next packet's delta spanned two intervals and
fabricated a whole extra packet of producer time — permanently, since the
monotonic clamp discards the correction ClockOffset would have made. Reordering
is reachable in production: udps_client.c only counts counter gaps. Simply
never regressing the reference is not the fix either, because a producer restart
would then freeze the signal forever, so the two are now separated by the size
of the backward jump.
The hrt branch's clamp was also one-directional, reintroducing on that branch
exactly the defect the declared branch's squeeze exists to prevent: a backward
wall step (NTP, suspend/resume) left a permanent lead. It now shares the same
wall-elapsed cap.
Also: anchor an hrt-branch burst's LAST element on arrival, matching the
declared branch, so two accumulated scalars in one scope do not sit a burst
apart on the shared X axis; treat a non-finite samplingRate off the wire as
undeclared, since +inf produced a 0.0/0.0 factor the floor could not catch and
turned every stamp NaN; write lastCounter on the hrt branch so duplicate
datagrams are dropped there too; and correct two comments that argued for the
current code with claims that are false (a counter-gap clamp reaches the
opposite outcome, not the same one earlier, and the squeeze's steady state is a
sawtooth, not a fixed offset).
Seven new tests, each proven non-vacuous by sabotage; 54 pass. Plan document
Task 4 re-synced and its stale test count and "agree on the same stream" claim
corrected.
Round 4 of Task 4 review. Four defects in FrameDecoder's rule 3:
- The undeclared-rate (hrt) path positioned each burst at an ABSOLUTE
hrt/ticksPerSecond(). hrt counts from the producer's boot, so it is ~1e11
ticks by the time a scope attaches, and the rate is refitted every packet
with a few parts in 1e4 of wobble. The product is tens of milliseconds of
jitter in BOTH directions -- not merely imprecise, non-monotonic. Integrate
short tick deltas into accProdSec instead and let ClockOffset latch the
epoch that leaves behind.
- The lead bleed used a fixed 0.9 factor, which converges only while the
declared rate is within ~10%. Squeeze proportionally to the excess instead
(floored at kMinBleedFactor), settling it in a single burst.
- A single-sample flush fell through to the plain-scalar rule, dating it from
arrival and leaving lastCounter stale so the next real burst reinstated a
hole that never existed. Accumulate mode flushes on a timer, so a short
cycle legitimately yields one sample; keep it on the chain.
- kMaxCounterGap was inert: an absurd gap yields an absurd prediction that the
arrival backstop already rejects, and no input can distinguish the two
rules. Removed rather than left implying a behaviour it did not have.
FrameDecoder.h now states the deliberate divergence from StreamHub -- which
converts hrt with the LOCAL MARTe timer frequency, valid only because it runs
on the producer's host -- and why a remote scope's drift is irreducible.
Three new tests, each sabotage-proven non-vacuous: producer restart, short
flushes staying on the chain, and a 20000-packet undeclared run after a
day of producer uptime that asserts SPACING as well as ordering (the
monotonic guard alone restores order while leaving positions wrong).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Round 3 of the Task 4 review. Three defects, all in FrameDecoder rule 3.
The resync backstop was one-directional. `predicted` is never below
lastEmittedEnd + dt, so rejecting a correction that would step backwards
meant only a LAGGING chain could ever be pulled back; a chain running fast
drifted ahead without bound. Two hosts' crystals differ by tens of ppm, so a
declared SamplingRate is always slightly wrong in one direction or the other
and this is certain on a long session. A leading timeline cannot be corrected
in one burst without going backwards -- lastEmittedEnd is by definition past
arrival -- so the excess is bled off by drawing each burst 10 % narrower until
the timeline is back inside the threshold.
A repeated packet counter was treated as a normal packet. The C client
de-duplicates fragments only, so an unfragmented update reaching a host that
joined the group on two interfaces was emitted twice, doubling the values and
advancing the timeline by a burst that never existed.
The samplingRate == 0 path differenced two HrtRateFit::toSeconds() results.
toSeconds() divides an absolute tick count -- ~1e11 on a producer that has
been up a day -- by a rate refitted on every packet, so its few-parts-in-1e4
wobble arrives multiplied by the whole elapsed epoch: tens of milliseconds of
jitter on a value whose consecutive difference is a few milliseconds. Raw
ticks are differenced instead, anchored on the first usable packet so the
wobble applies only to the interval since attach.
The existing hrt-gap test could not have caught the last one: its 10 ms
producer period made the expected answer exactly kDefaultDt, so a decoder
that derived nothing passed. It now uses 25 ms.
Four tests added, all sabotage-proven. The plan is updated to match.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Review found the decoder was estimating something the wire states exactly.
FrameView::counter increments once per update, so a gap of g means g-1 lost
datagrams; reinstating their duration restores the hole precisely, with no
threshold and no dependence on arrival time. The arrival-anchor comparison
survives only as a backstop for what the counter cannot express — a producer
restart, a counter stuck at zero, a wrong declared rate — and can no longer
step a signal's timestamps backwards, which the ring and trigger forbid.
Also from review: guard the time-signal lookup against a frame carrying more
signals than the installed table, and give FrameBuilder a counter parameter.
Leaving it at zero had hidden the counter rules from every test, and made the
hrt-gap test vacuous — under uniform arrivals the hrt path and packetBurst
agree by construction, so it could not tell which branch answered. Its
arrivals now carry zero-mean jitter.
Each new assertion was proven non-vacuous by sabotage: dropping the gap term,
the backward guard, or the hrt branch fails exactly its own test.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Forward-chaining each accumulated burst onto the previous one suppresses
arrival jitter, but an unchecked chain never recovers: one lost datagram, or a
declared sampling rate that differs from the producer's real one, dates every
later sample early for the rest of the run. The chain is now a prediction,
compared each packet against the arrival anchor and abandoned beyond
kBurstResyncThresholdS, which bounds the error instead of accumulating it.
Plan amended so the hrt-fit fallback (unusable here: the fit needs 32 packets
and is itself corrupted by bursty arrivals) cannot come back.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implements FrameDecoder with five timing rules that mirror
UDPSourceSession.cpp: FullArray (per-element time signal), FirstSample
and LastSample (rate-spread from anchor), accumulated scalar with
declared rate (forward-chain anchoring, immune to arrival jitter), and
PACKET burst (backward-span from previous arrival). 9 new tests, 38
pass total.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The fit keeps the slope and discards the intercept, so the result counts from
the producer's boot. Tasks 4 and 7 compose it with ClockOffset::map, which is
correct, but the bare name invites passing it straight to a plot axis.
Also unwrapped the stalled-clock assertion from behind `if (fit.ready())` —
that branch never runs, so the test confirmed nothing.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The jitter test fed a receive timestamp that went backwards (1001.02 then
1000.97), putting the second reading 1.03 s from the prediction — twice the
threshold, so not jitter under any reading. That is a digit slip for 1001.97.
It had been worked around by making the threshold one-sided, which passes the
test but never fires when the producer's clock steps forward: the prediction
stays ahead of the wall clock, the error stays negative, and the trace sits in
the future for the rest of the run. Restored std::fabs, corrected the test data,
and added the forward-jump case that the one-sided version silently failed.
Plan amended so the bad data does not come back.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds TimeBase.h/cpp with ClockOffset (latched wall-clock offset with
one-sided recalibration on positive drift only, so early-arriving packets
do not wobble the trace) and HrtRateFit (sliding-window OLS that recovers
an unknown hrt tick rate from receive timestamps). Also adds
TimeSignalScale() which maps UDPS type codes to seconds-per-count.
9 new tests, all 28 pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Gap 1: closeLeaf was only tested closing the first sibling; added test
closing the second sibling to cover the else branch of parent->a.get()==leaf.
The test verifies the correct sibling survives with its signal intact.
Gap 2: closeLeaf was only tested at depth 1 (root's direct children);
added test with depth-2 leaf (in right subtree) to exercise findParent's
recursive search in both subtrees. Tests that the correct leaf is promoted
and remaining signals are preserved.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The ramp data in EmitsPointsInTimeOrder never produced a bucket whose maximum
preceded its minimum, so an implementation ordering the emitted pair by value
instead of by time would have passed. Replaced with an explicit two-bucket case
whose second bucket reverses the order.
file(COPY) is a hard configure error on a missing source, so a checkout without
the sibling StreamHub resources failed to configure despite the ASCII-icon
fallback the block above had just selected.
Plan amended to match on both points.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
