Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-26384: Read telemetry timestamp from telemetry header #24

Merged
merged 2 commits into from Oct 29, 2020

Conversation

r-owen
Copy link
Collaborator

@r-owen r-owen commented Oct 27, 2020

No description provided.

tel_time = astropy.time.Time(
server.header.mjd, server.header.mjd_frac, format="mjd", scale="utc"
)
tel_tai_unix = salobj.tai_from_utc(tel_time)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about issuing a warning event when timestamps from low level differs significantly from local clock? This might prevent some time-synchronization problems.. Shall I start a JIRA ticket for this or you will be able to add this check now? I am not sure if there is any warning event in ts_rotator XML available for that..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe even better, rejecting low-level updates when timestamps differ..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry it will require too much code for little gain. It requires configuring a max tolerance, then dealing with the fact that the data arrives at 10-20 Hz and I certainly don't want to print warnings at that rate, so I need logic for "I have already reported this", and a way of resetting that after N "in spec" times. If you feel strongly about it feel free to file a ticket. A Watcher alarm is another option (though I'd rather not have it responding to an event at that rate if I can avoid it, which suggests polling...)

Copy link
Member

@pkubanek pkubanek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But please see comment on comparing local and low level clock. Rejecting / warning about messages coming from past/future (maybe due to clock synchronization issues, or due to other problems) is always a good idea.

@r-owen r-owen merged commit 0d3df37 into develop Oct 29, 2020
@r-owen r-owen deleted the tickets/DM-26384 branch October 29, 2020 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants