[uxrce_dds_client] Set PX4 clock from UTC

[dakejahl]

In systems with a companion computer without a GPS, it would be nice to use the timestamps from the DDS bridge to set the PX4 system time.

[dagar]

This is likely what we want most of the time, but annoyingly not all the time.

At a minimum can you add some tolerance (so that we don't fight with GPS or Mavlink time setting) and probably best to add a parameter so that this can be disabled if necessary.

PX4-Autopilot/src/drivers/gps/gps.cpp

Lines 436 to 441 in d6dbf38

	if (drift_time >= SET_CLOCK_DRIFT_TIME_S) {
	// as of 2021 setting the time on Nuttx temporarily pauses interrupts
	// so only set the time if it is very wrong.
	// TODO: clock slewing of the RTC for small time differences
	px4_clock_settime(CLOCK_REALTIME, &rtc_gps_time);
	}

Note for GPS right now that UTC time is coming from the navigation solution that's already ~100 milliseconds old. I have notes/plans on how to do it all perfectly with the TIMEPULSE, extending to CAN nodes, etc, but so far it hasn't been a priority.

[dakejahl]

At a minimum can you add some tolerance (so that we don't fight with GPS or Mavlink time setting)

@dagar This logic occurs before the main loop and will only get executed once when the dds_client starts. Should I add it to the main loop such that it works the same way as GPS/Mavlink?

[Ryanf55]

Is there a reason you chose not to use MicroXRCE DDS's built in time sync callback support in uxr_set_time_callback?
https://micro-xrce-dds.docs.eprosima.com/en/latest/time_sync.html

I am happy to open up a discussion with eProsima to see if it's intended for this use case.

[dagar]

At a minimum can you add some tolerance (so that we don't fight with GPS or Mavlink time setting)

@dagar This logic occurs before the main loop and will only get executed once when the dds_client starts. Should I add it to the main loop such that it works the same way as GPS/Mavlink?

Likely yes, but we could come back to that later if/when we're ready to tackle the larger story with GPS (TIMEPULSE, etc).

For now at least add the parameter so this can be disabled if it's not ideal for a particular setup?

[dakejahl]

@Ryanf55 it does use uxr_set_time_callback

@dagar okay I added a parameter to enable clock synchronization along with the same logic in GPS and Mavlink to only set the clock if dt > 5_s. I also refactored a little bit.. 😅

[dakejahl]

Rebased and retested functional except for this issue which I think was introduced in the latest commits
#22382

[dakejahl]

@dagar asked about if this waits for the Timesync to converge -- the answer is "no", but neither does the current logic for setting the session->time_offset. Also I think it's really not important that the time is perfectly synchronized for this case anyway, we really just want to be close enough for the purpose of analyzing and organizing log files by time/date.

This PR stems from me trying to analyze logs from a system without GPS and all of the log files have the same garbage date which is the start of the epoch

[dagar]

All you'd need to do is make this public, then call _timesync.sync_converged(). We can do it later when tightening up the timesync in general (GPS, CAN nodes, etc).

PX4-Autopilot/src/lib/timesync/Timesync.hpp

Lines 116 to 120 in 630dc99

	/**
	* Return true if the timesync algorithm converged to a good estimate,
	* return false otherwise
	*/
	bool sync_converged() const { return _sequence >= CONVERGENCE_WINDOW; }