UAVCAN time synchronization is broken

[pavel-kirienko]

The context

Certain peripheral device that is currently being equipped with UAVCAN required that the FMU and the device itself ran synchronized clocks. UAVCAN does support time synchronization over the CAN bus, and so does libuavcan (spec, tutorial). The PR #2895 has extended the UAVCAN driver with time synchronization feature, at the same time breaking UAVCAN sensors (#2912, #2911).

The problem

Obviously, in order for the sensor fusion to function correctly, all sensor measurements must be timestamped using the same (or synchronized) time source. This condition used to be easy to meet, since all sensor drivers were simply using HRT. This changed when a time synchronization master was added to the UAVCAN driver, because the library now uses a dedicated hardware timer (namely TIM5), which runs with a phase offset of about a second (didn't measure it exactly) with respect to the HRT, and incoming UAVCAN messages are timestamped by the library incorrectly using this time source. These incorrect timestamps are then copied further to ORB sensor topics.

A temporary solution has been introduced (see #2914), in which the UAVCAN driver simply uses HRT instead of the library's provided timestamps, which is NOT a proper solution and provides degraded timestamping precision.

Solutions

Use HRT instead of TIM5 for libuavcan

This is the easy way, but it has a significant drawback: since HRT must be be monotonic, it cannot be synchronized with other time sources, because that would require it to change rate and/or jump.

Do we care about use cases where PX4 has to synchronize its time with an external time source, e.g. a companion computer or a GNSS receiver? I expect that we do.

Use TIM5 for timestamping everywhere

This approach permits to maintain monotonicity of HRT, adding another clock which can change rate in order to maintain synchronization with an external source.

The current code will have to be updated in order to use the new, synchronized clock for timestamping instead of HRT. Also, the developers will have to keep in mind the difference between monotonic clock and synchronized clock, and why they are not interchangeable.

FYI, grepping the codebase for hrt_absolute_time shows 464 matches.

Extend HRT with synchronized clock

From application-level standpoint this solution is identical to the previous one, the difference is in implementation - instead of using a dedicated hardware timer for synchronized clock, the HRT driver can be extended with notions of phase offset and rate offset between monotonic and synchronized clocks. By the way, this is how monotonic/synchronized clocks are implemented in STM32 and LPC11C24 drivers for libuavcan. Sources: STM32 , LPC11C24.

@davids5 @mhkabir @LorenzMeier

[pavel-kirienko]

I have just read https://pixhawk.org/dev/ros/timesync and invented another hack: use HRT for libuavcan's monotonic clock, and let the library maintain CAN-synchronized clock with TIM5. Also, the UAVCAN driver will have to maintain the skew between the CAN-synchronized clock and HRT. Then, whenever a UAVCAN measurement arrives, the driver will subtract the skew from the UAVCAN-provided timestamp, thus converting it into the HRT time reference. And vice versa, when PX4 publishes a timestamped UAVCAN message, it will add the skew to the HRT-sourced timestamp.

I'm not sure this will work, maybe I'm missing something important.

[mhkabir]

@pavel-kirienko That works, yes. I did the interface :) A better synchronization algorithm is desired though, so if you can come up with a better algorithm to smooth the deltas, it'd be awesome.

[pavel-kirienko]

@LorenzMeier if you vetted my latest proposition I could implement it this week.

[LorenzMeier]

Still broken?

[pavel-kirienko]

Yes, this is still broken, and I don't have the bandwidth to look into this now. Perhaps later. AFAIK, right now this feature is not critical.

[PX4BuildBot]

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 30 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.)