Time Stamps

[MikeGitb]

Can we add time stamps to the scan data? Either one time stamp per scan or for each individual sample? Especailly when rotating with a low frequency, it can become very difficult to accurately merge the lidar data with data from other sources like e.g. a gyroscope.

Ideally, the data should alredy be timestamped in the sensor itself to avoid jitter introduced by the host operating system., but that would probably open a whole bag of synchronization issues.
However, at least the worker thread in libsweep could create an utc / unix timestamp upon reception of a message over the serial interface.

[daniel-j-h]

Sounds like a duplicate of #19?

[MikeGitb]

@daniel-j-h: Ah right - sorry. Apparently I didn't have enough coffee this morning ;).

So let's be more specific: I'm currently implementing the timestamps for my private fork of libsweep, but would prefer to do so in a compatible manner. So the question is what should be the return value of a hypothetical function

... sweep_scan_get_time(sweep_scan_s scan, int32_t sample);

Personally I'd prefer if it where a value corresponding to "time since unix epoch without leap seconds" in milli- or microseconds. It is the de-facto time standard for time_t and std::chrono::system_clock and on most systems easily convertible to UTC. The main drawback I see however is that system_clock is not a steady clock, so there might be jumps. Also on some embedded systems "time since unix epoch" might actually be unknown, but I think those systems aren't targeted by the library anyway.

[daniel-j-h]

Is a timestamp actually needed or is a monotonically increasing steady integer value good enough? Could the frame of reference be the device's start instead of the unix epoch? What are the use-cases for a timestamp here?

[MikeGitb]

What are the use-cases for a timestamp here?

If the Lidar is mounted on a mobile platform, you might want to fuse the data with that from other sensors (e.g. GPS) in order to get absolute positions. The more precisely you can match the measurements from different sensors, the better the result. A monotonic clock would also work, but in my specific usecase, I'd have to convert it to UTC later anyway.

Is a timestamp actually needed or is a monotonically increasing steady integer value good enough
Are you talking about a packet counter? In that case the answer is definetively no. If that counter is supposed to represent e.g. milliseconds then yes.

[dcyoung]

At the very least I think we should add a timestamp associated with a completed scan.

Unless a future firmware update adds a timestamp to each sample (unlikely), any timestamps added by the SDK running on the host will include variance from the communication delay. So millisecond resolution seems plenty sufficient.

Could the frame of reference be the device's start instead of the unix epoch?

It is fairly trivial to convert milliseconds from the unix epoch to relative milliseconds referenced by some arbitrary start time (device construction, call to start scanning, sync reading of a new scan etc). However, it is impossible to convert in the other direction. Therefore it seems logical to use a timestamp starting with the unix epoch. If we want to be verbose, we could also include a construction timestamp in the device object and a completion timestamp in each scan object, adding appropriate getters for both.

[MikeGitb]

The one drawback I see with having one timestamp per 360° scan is that for slow rotation's that is quite coarse grained. On the other hand, one timestamp per measurement (every 1-10ms is far more than anyone needs and would most likely result in multiple scans having the same timestamp anyway (due to scheduling and OS buffers).
A third option would be a start and end timestamp per scan from which the others can be interpolated if needed.

One way or the other - if you tell me, which you want, I can provide a PR

[MikeGitb]

If I understand correctly, this project is being shut down so I'm closing this issue.

[mrod146]

Did you get the string of timestamp code to work because when I put in sweep_scan_get_time(sweep_scan_s scan, int32_t sample);
I got this syntax error -bash: syntax error near unexpected token '('