Fix a permanent NaN latch in the clock servo - #15
Open
damiendusha wants to merge 1 commit into
Open
Conversation
A Sync cycle that does not advance in master time -- the same Sync
processed twice, or two Syncs carrying the same originTimestamp --
yields measSyncPeriodNs <= 0. Every servo divides by that value, so the
cycle produces NaN, and because NaN propagates through the Kalman
filter's state, a single bad sample disables clock correction for good.
The failure is silent, which is what makes it expensive: the servo keeps
reporting a correction of 0.0000 for ever while the clock free-runs, so
it reads as a healthy, perfectly locked slave.
Three guards, because each one is independently sufficient to lose the
clock:
slave.c ptp_perform_correction() skips a cycle whose measured Sync
period is not positive, before it reaches any servo.
slave.c ptp_tune_clock() refuses a non-finite tuning value. Casting
NaN or infinity to an integer is undefined; ARM yields 0 and
x86 yields INT64_MIN, so one poisoned sample either freezes
the addend for ever or destroys it outright.
kalman_filter.c
Skips a non-positive period, and resets the filter when its
state or output goes non-finite, so the filter recovers
instead of latching.
Target-independent: nothing here is specific to a port or a network
stack.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
A Sync cycle that does not advance in master time — the same Sync processed twice, or two Syncs carrying the same
originTimestamp— givesmeasSyncPeriodNs <= 0. Every servo divides by that, so the cycle produces NaN, and NaN propagates through the Kalman filter's state, so a single bad sample disables clock correction permanently.What makes this worth fixing rather than tolerating is that it is silent. The servo goes on printing a correction of
0.0000for ever, which is exactly what a perfectly locked slave prints, while the clock free-runs underneath. No error, no state change, no counter that moves.Three independent guards: reject the bad period before it reaches a servo, reject a non-finite tuning value before it reaches the hardware clock, and reset the filter if its state goes non-finite anyway. Each alone prevents the latch; together they also let the filter recover rather than merely refuse to break.
Nothing here is specific to a port, an RTOS or a network stack.
Found while porting flexPTP to an STM32H563 (NUCLEO-H563ZI) running ThreadX through the CMSIS-RTOS2 binding with lwIP as the network stack, in an application that samples two SCH16T IMUs at 8 kHz and timestamps them off the PTP-disciplined clock. This has been running with the fix applied since.
The change
One commit, based on
master76aaf67(RC1.0-11 — currentHEADwhen this was written). It rebases cleanly ontopdel_fixtoo, so this can target either branch.Rebasing or squashing to taste is fine by me — say the word and I will reshape it.