Repository navigation
Carry the input time with each SSE event and mode change - #518
Merged
Merged
Conversation
The fix for #497 will bound how far an SSE client can fall behind by giving each write a deadline counted from when the data it carries was produced. Neither the SSE events nor the PHC mode-change notifications carried such a time. Observer.ModeChanged now takes the system time of the event behind the transition: the sample's Sys (the estimated time of the pulse), the read time of the time message that triggered leap detection, or when carrier loss was seen, which the ts pause event now records. It is zero for the initial transition at startup. Controller.TimeMessage and Controller.Pause take those times from their callers, so phcsync still never reads the clock itself. sseobs sends a TimedEvent, an sse.Event with that time, on the event bcast: the read time of the GPS message, the sample's Sys, or the mode change's time. The SSE handler still writes the event as before and ignores the time, so there is no change in behaviour.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
modeProc and TestSampleModeChangeTime were inserted between TestTimeMessageDetectsLeap and its doc comment, so the comment documented modeProc instead and the test had none.
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
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.
Preliminary refactor for #497, with no change in behaviour. The fix for #497 will give each SSE write a deadline counted from when the data it carries was produced, so that a client that falls behind is disconnected; that needs each event on the daemon's SSE event bcast to carry that time.
phcsync.Observer.ModeChangedtakes the system time of the event behind the transition: the sample'sSys(the estimated time of the pulse), the read time of the time message that triggered leap detection, or when carrier loss was seen. It is zero for the initial transition at startup.Controller.TimeMessageandController.Pausetake those times from their callers, so phcsync still never reads the clock. Thetspause event records when carrier loss was seen.sseobs.sendSSEtakes the time, and events go on the bcast assseobs.TimedEvent, ansse.Eventplus that time: the GPS message's read time, the PHC sample'sSys, or the mode change's time.sseHandleRequeststill writes the event as before and ignores the time.Tests check that the controller passes the right time on the sample, leap-detection and carrier-loss paths, and that SSE events carry their handler's time.
make testand the http smoke tests pass.