-
Notifications
You must be signed in to change notification settings - Fork 17.4k
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
Handle TIMESYNC mavlink messages #5888
Conversation
libraries/GCS_MAVLink/GCS_Common.cpp
Outdated
// Create new timestruct to return | ||
mavlink_timesync_t rsync; | ||
rsync.ts1 = tsync.ts1; | ||
rsync.tc1 = AP_HAL::micros64(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not normally a time guy, but wouldn't the best numbers for timing come from capturing the micros64() number as soon as you enter the function, given that you call it in 2 places? (I get that this is a tiny tiny tiny tiny amount of jitter but stil).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good point :) I've addressed it in a subsequent commit, even though I've also removed the second call to micros64.
libraries/GCS_MAVLink/GCS_Common.cpp
Outdated
|
||
// If message is sent with tc > 1, calculate the offset and return that instead | ||
if (tsync.tc1 > 0) { | ||
rsync.tc1 = tsync.tc1 - AP_HAL::micros64(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this is correct, tc1 should be the absolute time, not a delta. this could also lead to a timesync flood.
tsync.ts1 = time at send
tsync.tc1 = 0 > time at client (unknown)
rsync.ts1 = copy of tsync.ts1 (time of the origin)
rsync.tx1 = out local absolute time
if tsync.tc1 > 0, then this is a time reply that originated from ourself, so there is no need to resend the message at all. plus we now have the info of the local time we sent the request (stored as we asked for it), and the time we acquired the response. (ie calc the offset)
please refer to here https://pixhawk.ethz.ch/docs/mavlink__receiver_8cpp_source.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@meee1 Yes this is correct if using this message to try to sync the absolute time of the flight controller, but my intent was to return the time since boot as that is a more reliable and consistent timestamp and is more useful for correlating log timestamps, EKF etc. The absolute time changes according to quality or absence of GPS lock.
The two fields of the TIMESYNC message don't allow for the same type of synchronisation as NTP which uses 4 fields. With the two fields of TIMESYNC the best we can achieve is (roundtrip/2) unless using tc1>0 to estimate the incoming offset and return the offset to the remote end. I've removed that logic in a subsequent commit to clarify the main purpose of the message response.
The px4 code uses tc1>0 to publish a smoothed offset to the remote timestamp within the flight controller, which is probably not what we want to do.
So thanks for raising this important question :) Which is, do we want to return the time since boot, or the absolute time?
I've submitted an updated PR so closing this one. #5944 |
PR to support #5884
TIMESYNC mavlink message is used to synchronise the flight controller with systems connected over mavlink. It can be used in one of two ways:
Once the connected system receives the returned TIMESYNC message, it can calculate the roundtrip time as well as receiving the local time since boot in usec of the flight controller.
Once the connected system receives the returned TIMESYNC message, it can accurately calculate the timestamp of the flight controller, and can continue to estimate it against the local clock of the connected system. If TIMESYNC messages are exchanged at 10hz, time sync is typically as accurate as <1ms (fcoffset below is the difference between the two systems in microseconds):
Sample python/dronekit code:
To use the above class:
Note: This works quite differently to how PX4 processes the TIMESYNC request. PX4 returns nanoseconds instead of microseconds, and sending a non-zero timesync.tc1 field does not return an offset (instead it does something different internally in PX4 firmware).