Conversation
Tracking mode now exits to a new gap mode on any missing sample, switching the PHC to the averaged frequency. The controller preserves the live tracking processor (servo, MAD window, avgFreq, bad-sample bookkeeping) across tracking<->gap transitions and restores it when the signal returns, instead of rebuilding processors as other transitions do. After a gap of at least recoveryThreshold missing samples, returning samples go through MAD recovery: the adaptive MAD threshold is relaxed by driftLimit so legitimate samples reflecting clock drift across the gap are accepted, fed to the servo, and collected; the MAD window is then re-baselined by the median drift (median.Window.ShiftAll) and seeded with the collected samples. If the median shift exceeds driftLimit, the controller falls back to reset. The unconditional outlier gate still applies throughout, and gap mode shares tracking's bad-sample limits. Gap mode counts as in sync: PTP clock quality is unchanged during a gap. The new [sync.gap] table configures recoveryThreshold, driftLimit, and recoverySamples; validation requires recoveryThreshold < track.badSampleRunLimit. A holdoverThreshold and a transition to holdover mode are deferred to #199. The syncsim mode observer now records the full mode sequence, with new expectModeSequence and expectGapSamples test assertions. Fixes #188
The sync table section now lists the gap subtable, describes the tracking -> gap -> tracking flow for brief PPS signal loss (#188), and adds track.badSampleRunLimit to the commonly-changed keys, pointing at the per-hardware values in configs/syncsim.
Gap mode (#188) makes it useful to raise track.badSampleRunLimit on hardware whose clock drifts slowly during signal loss, riding out longer gaps at full synchronization quality. Simulated outage sweeps (20 gap windows per duration, requiring every gap to recover without a reset and the worst in-sync offset to stay within ~100ns) give: timehat-f9t ~2-3 ppb drift, safe to 30s; cm4-f9t ~8-13 ppb, safe to 10s; i225-m8t ~25 ppb, safe only to ~6s, so the default of 5 stands. The timehat and cm4 configs carry the derived values (the timehat also raises badSampleWindow so the bad-sample ratio limit does not preempt the run limit during a long gap), the README documents the derivation and warns that gap.driftLimit encodes the accuracy budget and should not be raised to extend gap tolerance, and the NEWS entry for gap mode mentions the payoff.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5e30ad80b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
127
to
+128
| func (m Mode) InSync() bool { | ||
| return m == ModeTracking | ||
| return m == ModeTracking || m == ModeGap |
There was a problem hiding this comment.
In deployments using the NTP refclock or SHM outputs, marking ModeGap as in-sync here leaves those outputs inconsistent: time/internal/gpsevent/dispatcher.go:391 still returns unless d.controller.Mode() == phcsync.ModeTracking, so the first valid PPS samples after an outage are suppressed for the whole gap-recovery period even though PTP/metrics report the clock as synchronized. Update that gate to use InSync() (or avoid advertising gap as synchronized) so NTP clients don't see an avoidable sample outage after PPS returns.
Useful? React with 👍 / 👎.
The refclock/SHM sample gate in sysSample still required tracking mode, so during gap mode (#188) the NTP outputs suppressed samples while PTP clock quality and metrics reported the clock as synchronized, including the first valid samples after the PPS signal returned. Gate on Mode.InSync() instead, which is the condition every other consumer uses. The samples are GPS-message-time vs system-clock offsets, not PHC-derived, so they remain valid during gap recovery.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #188.
Tracking mode now exits to a new gap mode on any missing sample, switching the PHC to the averaged frequency. The controller preserves the live tracking processor (servo, MAD window, avgFreq, bad-sample bookkeeping) across tracking<->gap transitions and restores it when the signal returns, instead of rebuilding processors as other transitions do. Gap mode counts as in sync: PTP clock quality is unchanged during a gap.
After a gap of at least
recoveryThresholdmissing samples, returning samples go through MAD recovery: the adaptive MAD threshold is relaxed bydriftLimitso legitimate samples reflecting clock drift across the gap are accepted and collected, then the MAD window is re-baselined by the median drift and seeded with the collected samples. If the median shift exceedsdriftLimitthe controller falls back to reset. The new[sync.gap]table configuresrecoveryThreshold,driftLimitandrecoverySamples; aholdoverThresholdand the transition to holdover mode are deferred to #199.The user-visible payoff is that
sync.track.badSampleRunLimitcan now usefully be raised on hardware whose clock drifts slowly during signal loss: without recovery, post-gap samples showing legitimate drift are rejected as outliers and force a reset anyway. Simulated outage sweeps (20 gap windows per duration, requiring every gap to recover with worst in-sync offset within ~100ns) put the safe gap at 30s for the TimeHAT TCXO, 10s for the CM4, and ~6s for the i225-T1, so the default of 5 stands and the reference configs inconfigs/syncsimcarry the per-hardware values (README documents the derivation).Verified entirely in syncsim: new test cases cover post-gap drift acceptance, outlier rejection during recovery, the short-gap fast path, and the excessive-drift reset, with new mode-sequence and gap-sample-count assertions backing them. The satpulse.toml man page documents the new mode and the tuning knob.