fix(udpscope): keep the clock-offset recalibration threshold symmetric
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>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
41ab151a2f
commit
e4817dd284
@@ -13,11 +13,13 @@ double TimeSignalScale(uint8_t typeCode) {
|
||||
}
|
||||
|
||||
double ClockOffset::map(double producerSec, double wallSec) {
|
||||
/* Recalibrate only when wall is significantly ahead of the prediction.
|
||||
* Early-arriving packets (wall < prediction) are normal network jitter and
|
||||
* must not reset the offset — doing so would wobble the trace. A large
|
||||
* positive error means the producer clock jumped back or restarted. */
|
||||
if (!valid_ || (wallSec - (offset_ + producerSec)) > kRecalibThresholdS) {
|
||||
/* Symmetric on purpose. Delivery jitter of a few tens of ms either side of
|
||||
* the prediction must not reset the offset or the whole trace wobbles, but a
|
||||
* producer clock that steps in EITHER direction has to be picked up: a
|
||||
* restart leaves the prediction behind the wall clock, an NTP correction on
|
||||
* the producer's host leaves it ahead. A one-sided test silently never fires
|
||||
* for the second case and the trace sits in the future for the whole run. */
|
||||
if (!valid_ || std::fabs(wallSec - (offset_ + producerSec)) > kRecalibThresholdS) {
|
||||
offset_ = wallSec - producerSec;
|
||||
valid_ = true;
|
||||
}
|
||||
|
||||
@@ -20,8 +20,24 @@ TEST(ClockOffset, HoldsTheOffsetThroughSmallArrivalJitter) {
|
||||
ClockOffset off;
|
||||
off.map(10.0, 1000.0); // offset = 990
|
||||
|
||||
EXPECT_DOUBLE_EQ(off.map(11.0, 1001.02), 1001.0);
|
||||
EXPECT_DOUBLE_EQ(off.map(12.0, 1000.97), 1002.0);
|
||||
// Arrival wanders either side of the prediction. wallSec is a local receive
|
||||
// timestamp, so it only ever advances — jitter shows up as the gap growing
|
||||
// and shrinking, never as the clock going backwards.
|
||||
EXPECT_DOUBLE_EQ(off.map(11.0, 1001.02), 1001.0); // +0.02 late
|
||||
EXPECT_DOUBLE_EQ(off.map(12.0, 1001.97), 1002.0); // -0.03 early
|
||||
}
|
||||
|
||||
// The threshold has to be symmetric. A producer whose clock steps FORWARD (an
|
||||
// NTP correction on the producer's host, say) puts the prediction permanently
|
||||
// ahead of the wall clock — a one-sided "recalibrate only when wall is ahead"
|
||||
// test never fires for it, and the trace sits in the future for the rest of the
|
||||
// run.
|
||||
TEST(ClockOffset, RecalibratesWhenTheProducerClockJumpsForward) {
|
||||
ClockOffset off;
|
||||
off.map(10.0, 1000.0); // offset = 990
|
||||
|
||||
// Producer leaps 100 s ahead while only 1 s of wall time passes.
|
||||
EXPECT_DOUBLE_EQ(off.map(111.0, 1001.0), 1001.0);
|
||||
}
|
||||
|
||||
TEST(ClockOffset, RecalibratesWhenDriftExceedsTheThreshold) {
|
||||
|
||||
@@ -1043,8 +1043,24 @@ TEST(ClockOffset, HoldsTheOffsetThroughSmallArrivalJitter) {
|
||||
ClockOffset off;
|
||||
off.map(10.0, 1000.0); // offset = 990
|
||||
|
||||
EXPECT_DOUBLE_EQ(off.map(11.0, 1001.02), 1001.0);
|
||||
EXPECT_DOUBLE_EQ(off.map(12.0, 1000.97), 1002.0);
|
||||
// Arrival wanders either side of the prediction. wallSec is a local receive
|
||||
// timestamp, so it only ever advances — jitter shows up as the gap growing
|
||||
// and shrinking, never as the clock going backwards.
|
||||
EXPECT_DOUBLE_EQ(off.map(11.0, 1001.02), 1001.0); // +0.02 late
|
||||
EXPECT_DOUBLE_EQ(off.map(12.0, 1001.97), 1002.0); // -0.03 early
|
||||
}
|
||||
|
||||
// The threshold has to be symmetric. A producer whose clock steps FORWARD (an
|
||||
// NTP correction on the producer's host, say) puts the prediction permanently
|
||||
// ahead of the wall clock — a one-sided "recalibrate only when wall is ahead"
|
||||
// test never fires for it, and the trace sits in the future for the rest of the
|
||||
// run.
|
||||
TEST(ClockOffset, RecalibratesWhenTheProducerClockJumpsForward) {
|
||||
ClockOffset off;
|
||||
off.map(10.0, 1000.0); // offset = 990
|
||||
|
||||
// Producer leaps 100 s ahead while only 1 s of wall time passes.
|
||||
EXPECT_DOUBLE_EQ(off.map(111.0, 1001.0), 1001.0);
|
||||
}
|
||||
|
||||
TEST(ClockOffset, RecalibratesWhenDriftExceedsTheThreshold) {
|
||||
|
||||
Reference in New Issue
Block a user