From d7552381c3d25bab96c7f5f7970c2d1dc5d31245 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Thu, 26 Dec 2024 11:56:43 -0800 Subject: [PATCH] Garmin: Report correct local time offset We stupidly thought that the local time offset was in the "DEVICE_SETTINGS" message as the time_offset field. I'm pretty sure I've seen something like that before. But the FIT files from github issue #4401 clearly have that time offset field being zero, and there are two other ways to figure out what local time actually is, namely in the "local_time" field of the ACTIVITY message or the TIMESTAMP_CORRELATION message. Either of those seem to work for what we want, so let's parse both (we already did the ACTIVITY case), and let's ignore the "time_offset" field from DEVICE_SETTINGS at least if it is zero. There is probably some real explanation for what the proper way to deal with all this is, and what the whole time_offset and utc_offset fields from DEVICE_SETTINGS means, and maybe we can improve on this in the future if somebody figures it all out. In the meantime, this seems to be an improvement. Reported-by: @WetsuitSeasoning Link: https://github.com/subsurface/subsurface/issues/4401 Signed-off-by: Linus Torvalds --- src/garmin_parser.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/src/garmin_parser.c b/src/garmin_parser.c index 5dd46166..5535dac0 100644 --- a/src/garmin_parser.c +++ b/src/garmin_parser.c @@ -617,8 +617,23 @@ DECLARE_FIELD(RECORD, rmv, UINT16) { } // 100 * l/min DECLARE_FIELD(RECORD, ascent_rate, SINT32) { } // mm/s (negative is down) // DEVICE_SETTINGS -DECLARE_FIELD(DEVICE_SETTINGS, utc_offset, UINT32) { garmin->dive.utc_offset = (SINT32) data; } // wrong type in FIT -DECLARE_FIELD(DEVICE_SETTINGS, time_offset, UINT32) { garmin->dive.time_offset = (SINT32) data; } // wrong type in FIT +DECLARE_FIELD(DEVICE_SETTINGS, utc_offset, UINT32) +{ + garmin->dive.utc_offset = (SINT32) data; // wrong type in FIT +} +DECLARE_FIELD(DEVICE_SETTINGS, time_offset, UINT32) +{ + /* + * Crazy FIT files have a zero time offset in DEVICE_SETTINGS, + * but then have a local_timestamp in ACTIVITY and/or in the + * TIME_CORRELATION messages. + * + * So I have no idea what this field means then. + */ + if (!data) + return; + garmin->dive.time_offset = (SINT32) data; // wrong type in FIT +} // DEVICE_INFO // collect the data and then use the record if it is for device_index 0 @@ -677,6 +692,19 @@ DECLARE_FIELD(SPORT, sub_sport, ENUM) { DC_ASSIGN_FIELD(garmin->cache, DIVEMODE, val); } +/* + * What is the difference between 'system_timestamp' and the + * actual timestamp of the message itself? Who designs these + * crazy things? What is the meaning of it all? These are the + * kinds of unanswerable questions that keep me up at night. + */ +DECLARE_FIELD(TIMESTAMP_CORRELATION, system_timestamp, UINT32) { } +DECLARE_FIELD(TIMESTAMP_CORRELATION, local_timestamp, UINT32) +{ + int time_offset = data - garmin->record_data.timestamp; + garmin->dive.time_offset = time_offset; +} + // DIVE_GAS - uses msg index DECLARE_FIELD(DIVE_GAS, helium, UINT8) { @@ -988,6 +1016,14 @@ DECLARE_MESG(RECORD) = { } }; +DECLARE_MESG(TIMESTAMP_CORRELATION) = { + .maxfield = 6, + .field = { + SET_FIELD(TIMESTAMP_CORRELATION, 1, system_timestamp, UINT32), + SET_FIELD(TIMESTAMP_CORRELATION, 3, local_timestamp, UINT32), + } +}; + DECLARE_MESG(DIVE_GAS) = { .maxfield = 4, .field = { @@ -1181,6 +1217,8 @@ static const struct { SET_MESG(147, SENSOR_PROFILE), + SET_MESG(162, TIMESTAMP_CORRELATION), + SET_MESG(206, FIELD_DESCRIPTION), SET_MESG(216, WTF_216),