GPS/GPS_ACCURACY chunks corrupt the shared sample clock, producing wild depth spikes

[trustthegoose]

GPS/GPS_ACCURACY chunks corrupt the shared sample clock, producing wild depth spikes

suunto_nautic_parser_parse() sums every chunk's leading millisecond delta into one shared time_ms running clock:

// src/suunto_nautic_parser.c
if (chunk.id != CHUNK_TIMELINE_BASE && chunk.size >= 2)
    time_ms += (int16_t) array_uint16_le (chunk.data);

GPS (0x0B) and GPS_ACCURACY (0x0E) chunks don't belong on that clock. The watch buffers and writes them with a lag — their delta is relative to their own last GPS-type entry, not to whatever chunk immediately precedes them in file order. On a real capture this shows up as a time_ms rewind of several hundred ms every time one of these chunks appears:

...IMU_ALT ticking forward normally (100ms steps)...
0x0e (GPS_ACCURACY)  delta=-660  time_ms=3269440   <== rewinds the shared clock
...IMU_ALT resumes forward from the rewound point...

The very next DC_SAMPLE_TIME (typically the next IMU tick) inherits that corrupted, rewound timestamp.

Why it produces multi-kilometer depth spikes

Depth is only reported directly by CHUNK_EXTENDED_STATUS, roughly once every ~10s. Everything in between is NAN and gets linearly interpolated between the two real neighbors in the platform wrapper's fill_missing_depths() (libdc_download.c):

unsigned int t0 = dive->samples[prev].time_ms;
unsigned int t1 = dive->samples[i].time_ms;
...
double fraction = t1 > t0
    ? (double)(dive->samples[j].time_ms - t0) / (double)(t1 - t0)
    : 0.0;
dive->samples[j].depth = d0 + (d1 - d0) * fraction;

time_ms is unsigned int. When a corrupted sample's timestamp is less than t0, dive->samples[j].time_ms - t0 doesn't go negative — it underflows to a value near UINT_MAX, and dividing that by a small (t1 - t0) (typical right at the end of a dive, where real readings are close together) produces an enormous fraction. The unclamped formula then extrapolates the depth by that same enormous factor. On a real Suunto Ocean dive this produced two isolated one-sample spikes of 212,373 m and 751,979 m, each immediately adjacent to samples reading a few centimeters.

This single corrupted sample cascades into wrong dive-level statistics anywhere downstream that scans the profile for max depth or feeds it into a tissue-loading model — max depth off by 5 orders of magnitude, ppO2 in the thousands of bar, absurd gradient-factor and missed-deco-stop findings.

Reproduction

No real dive data needed — the mechanism reproduces from three synthetic chunks: two real depth readings 200ms apart, with one GPS_ACCURACY chunk between them carrying a realistic negative delta that lands its own timestamp 15 real seconds before both of them.

#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "libdc_wrapper.h"

static void put_u16le(unsigned char *p, unsigned int v) {
    p[0] = (unsigned char)(v & 0xFF);
    p[1] = (unsigned char)((v >> 8) & 0xFF);
}
static void put_f32le(unsigned char *p, float v) { memcpy(p, &v, sizeof(float)); }

// [id:1][length:1][timeDelta:2 LE][depth:4 LE float]
static unsigned char *put_extended_status(unsigned char *p, int16_t delta_ms, float depth_m) {
    p[0] = 0x16; p[1] = 6;
    put_u16le(p + 2, (uint16_t)delta_ms);
    put_f32le(p + 4, depth_m);
    return p + 8;
}
// [id:1][length:1][timeDelta:2 LE][dEHPE:1][dEVPE:1][pad:2]
static unsigned char *put_gps_accuracy(unsigned char *p, int16_t delta_ms) {
    p[0] = 0x0E; p[1] = 6;
    put_u16le(p + 2, (uint16_t)delta_ms);
    p[4] = p[5] = p[6] = p[7] = 0;
    return p + 8;
}

int main(void) {
    unsigned char buf[8 + 8 * 3];
    unsigned char *p = buf;
    memcpy(p, "SBEM0103", 8); p += 8;
    p = put_extended_status(p, 20000, 0.05f); // t=20000  (real)
    p = put_gps_accuracy(p, -15000);          // t=5000   (corrupted)
    p = put_extended_status(p, 15200, 0.08f); // t=20200  (real)

    libdc_parsed_dive_t result;
    char err[256] = {0};
    int rc = libdc_parse_raw_dive("Suunto", "Ocean", 1, buf, (unsigned int)sizeof(buf),
                                   &result, err, sizeof(err));
    if (rc != 0) { fprintf(stderr, "FAIL: %s\n", err); return 1; }

    for (unsigned int i = 0; i < result.sample_count; i++)
        printf("[%u] time_ms=%u depth=%.6f\n", i, result.samples[i].time_ms, result.samples[i].depth);
    return 0;
}

Output against current main:

[0] time_ms=20000 depth=0.050000
[1] time_ms=5000  depth=644242.840000
[2] time_ms=20200 depth=0.080000

Same run with the fix below applied:

[0] time_ms=20000 depth=0.050000
[1] time_ms=20000 depth=0.050000
[2] time_ms=35200 depth=0.080000

(time_ms=20000 twice and 35200 on the last line, not 20200, because with the fix the GPS_ACCURACY chunk no longer perturbs the shared clock at all — its own sample just holds the last known-good time, and the following real reading's delta of 15200 is now added on top of the correct 20000 instead of the rewound 5000. Either way the point holds: time_ms never goes backward, and depth never leaves [0.05, 0.08].)

Fix

Exclude CHUNK_GPS and CHUNK_GPS_ACCURACY from advancing the shared time_ms, the same way CHUNK_TIMELINE_BASE already is — their own emitted samples then fall back to the shared clock's last known-good position instead of a value reconstructed from sub-second GPS timing the format doesn't reliably encode once interleaved. As a second, independent safety net, fill_missing_depths()'s fraction should be clamped to [0, 1] — depth can never legitimately be extrapolated outside the two real bracketing samples, so this holds regardless of what causes a timeline irregularity.

Have both fixes ready to submit as a PR if that's useful.

[urbamax]

Confirmed both halves of this, and traced where each comes from.

Origin (relevant since the parser side is about to be reviewed upstream):

• The shared-clock advance — time_ms += (int16_t) array_uint16_le(chunk.data) for every non-TIMELINE_BASE chunk — is in 5da76d7 "suunto: add Suunto Nautic dive parser", i.e. it's in libdivecomputer#73 as it stands.
• The unclamped fill_missing_depths() fraction with the unsigned int underflow is abe6d703 on submersion-app/submersion:main, so that clamp has to land there separately.

Mechanism verified by hand-tracing your synthetic repro against the current parser + fill_missing_depths: EXT_STATUS(Δ20000) → time_ms 20000, GPS_ACC(Δ-15000) → time_ms 5000, then for the t=5000 sample fraction = (5000u - 20000u) / 200 = 21474761, depth = 0.05 + 0.03 * 21474761 = 644242.85 — matches your [1] ... depth=644242.840000 exactly.

fill_missing_depths clamp: fully agree, no caveats. An interpolated depth between two bracketing real samples can never legitimately fall outside [d0, d1], so fraction clamped to [0, 1] is correct regardless of what perturbs the timeline.

Excluding 0x0B / 0x0E from the shared clock: the reasoning holds against the RE notes (0x0E is documented as its own delta chain — "delta-encoded from zero", surface section only). One thing I couldn't verify without the descriptor decoder: could you paste the raw chunk trace around one of the negative deltas — id / length / raw first two bytes / running time_ms before and after? Just want the file-order evidence on record before this reaches #73. Also worth a look at the CHUNK_GPS datetime path: parser->datetime = (utc_ms - time_ms) / 1000 needs time_ms ≈ 0 at the first fix, which holds only if surface GPS is always first in the stream — your captures should confirm.

PR very welcome — target submersion-patches for the parser change and I'll shepherd it into #73; the clamp is a separate one-liner for main.

[trustthegoose]

Raw chunk trace

Here's one occurrence from the original capture, straight from the file bytes (not re-derived from the earlier decoded log). Sequence: IMU_ALT → 0x0f → GPS_ACCURACY → IMU_ALT, walking time_ms the same way the parser does:

IMU_ALT   (id=0x22, len=0x18)  payload[0:2]=28 00 (delta=+40)   time_ms: 3263030 -> 3263070
0x0f      (id=0x0f, len=0x03)  payload[0:2]=1e 00 (delta=+30)   time_ms: 3263070 -> 3263100
GPS_ACC   (id=0x0e, len=0x06)  payload[0:2]=6c fd (delta=-660)  time_ms: 3263100 -> 3262440  <== rewind
IMU_ALT   (id=0x22, len=0x18)  payload[0:2]=da 02 (delta=+730)  time_ms: 3262440 -> 3263170  <== over-corrects past the rewind

Raw bytes (hex), header + payload for each:

IMU_ALT   (before): 22 18 28 00 04 33 a7 35 79 f1 cb fb d0 ff 1b 00 29 fe b9 ff fb 03 98 0a 0c 01
0x0f            :   0f 03 1e 00 65
GPS_ACCURACY    :   0e 06 6c fd 12 1d 8b 0e
IMU_ALT   (after):  22 18 da 02 68 33 a7 35 92 f0 4e fb 6d 00 5a 00 dc fe 16 00 f0 03 a0 0a e8 00

GPS_ACCURACY's payload past the delta is 12 1d 8b 0e — dEHPE=0x12 (18), dEVPE=0x1d (29), then two pad bytes, matching the documented [timeDelta:2][dEHPE:1][dEVPE:1][pad:2] layout. -660 as int16 LE is 6c fd, which checks out: 0xFD6C = 64876, 64876 - 65536 = -660.

Note the following IMU_ALT's delta is +730, not something that lands back at 3263100 — it overshoots to 3263170, 70ms past where the timeline would have been without the dip at all. So it's not a clean "dip then exact recovery"; each GPS_ACCURACY occurrence leaves a small net residual on the shared clock even after the next chunk's larger compensating delta.

GPS-first-in-stream question

Checked this dive specifically: it has only 8 CHUNK_GPS (0x0B) chunks total, and the first one doesn't appear until chunk #46,730 of 48,155 — time_ms=3,262,440 (~54 min into a ~56 min dive), not near the start. So "surface GPS first" doesn't hold here; in this dive the only GPS chunks are clustered right at the very end (exit, not entry).

Whether that breaks parser->datetime = (utc_ms - time_ms) / 1000 is a separate question from when the fix occurs, though — that formula only needs time_ms to correctly reflect "elapsed ms since dive start" at the moment of this fix, not for the fix to be early. The real problem: this particular 0x0B chunk's own time_ms (3,262,440) is essentially the same value the GPS_ACCURACY rewind above lands on — i.e. this dive's one usable GPS fix sits right inside the same corrupted window. If time_ms is off by a few hundred ms at that exact chunk (from this same shared-clock bug), the derived dive-start datetime inherits that same error. Once the shared-clock fix is in, this should self-correct — but it's worth being aware the datetime path can be corrupted by the identical root cause, independent of stream position.

[urbamax]

@trustthegoose perfect, that's exactly the file-order evidence needed. And it changes the fix.

What the trace shows

The 0x0E timeDelta field is real and correctly located (6c fd = -660, and the [timeDelta:2][dEHPE:1][dEVPE:1][pad:2] layout checks out from 12 1d 8b 0e). So the parser isn't misreading a field — the field genuinely holds a small negative number.

The important part is the +730 on the next IMU_ALT. -660 + 730 = +70, and you're seeing that near-cancellation on every GPS_ACCURACY occurrence. That's not coincidence: the chunk after a GPS-family chunk has its delta encoded relative to the GPS chunk's back-dated position, not relative to the pre-GPS position. The watch rewinds to timestamp the buffered GPS reading, then the next real chunk's delta walks forward from there back to ~now (with ~70 ms of the watch's own slop left over).

Why "exclude 0x0B/0x0E from the shared clock" (the original proposal) is wrong

If you skip the -660:

after 0x0f:        time_ms = 3263100
GPS_ACC skipped:   time_ms = 3263100
IMU_ALT +730:      time_ms = 3263100 + 730 = 3263830   <- +730 ahead of truth (~3263170)

Because the +730 was measured from 3262440, dropping the -660 makes the following chunk overshoot by the full GPS delta. That turns a bounded transient (-660, self-corrected within one chunk, ~+70 ms net) into an unbounded forward drift of ~660 ms per GPS-family chunk. Strictly worse than today.

What actually fixes it

Leave the raw accumulator alone — it self-corrects via the +730, so divetime / total-diving-time stay right. Add a monotonic view, time_ms_emit = max(time_ms_emit, time_ms), updated right after the delta add, and use that for:

1. every DC_SAMPLE_TIME emission (including suunto_nautic_emit_vendor) — the GPS_ACC sample then lands at 3263100 instead of 3262440, no backward sample, no fill_missing_depths underflow;
2. the parser->datetime = (utc_ms - time_ms) / 1000 calc — covers your GPS-first-in-stream point: this dive's only 0x0B sits at time_ms ≈ 3,262,440, inside the same rewound window, so the monotonic value (≈ 3,263,170, the watch's own ~70 ms slop) is what should feed the datetime, not the -660 excursion.

This is model-agnostic — it doesn't matter whether 0x0E's delta is "its own chain" or a genuinely back-dated sensor reading; the timeline just never moves backward for anything user-facing.

Part 2 (fill_missing_depths fraction clamp to [0,1]) still lands on main regardless — it's the right backstop for any timeline irregularity from any driver, and it's independent of this parser change.

So: same two PRs, but Part 1 becomes "monotonic emit + datetime" rather than "exclude from the clock". Want to take it from here, or should I push the parser side onto submersion-patches?

[urbamax]

Implemented both, pushed to submersion-patches + suunto-nautic:

• 0d565ae (parser, submersion-patches) — time_ms stays the raw accumulator for the dive-time spans; sample_ms = max(sample_ms, time_ms) is updated right after each delta and drives every DC_SAMPLE_TIME and the dive-start datetime. A GPS-chunk rewind no longer produces a backwards sample, and the following chunk doesn't overshoot the way "exclude from the clock" would.
• 8990efb (superproject) — fill_missing_depths() clamps its interpolation fraction to [0, 1], plus the submodule bump.

Regression test test_suunto_nautic_gps_clock (in test/native/, runs in the C Wrapper Tests job): drives a synthetic stream of the exact shape — EXT_STATUS(+20000) -> GPS_ACCURACY(-15000) -> EXT_STATUS(+15200) — through the whole wrapper and asserts sample times stay monotonic and every depth stays in [0.05, 0.08]. With the fixes reverted it reproduces your 644242.84 m sample exactly (verified locally):

without fix:  sample 1: t=5000 ms,  depth=644242.840 m   <- assertion fails
with fix:     sample 1: t=20000 ms, depth=0.050 m

@trustthegoose — if you want to run it against your real Ocean capture that'd be the last confirmation; the datetime path (your 0x0B-in-the-rewind-window point) is covered by the same sample_ms switch. Parser change is also the one to fold into libdivecomputer#73 since the shared-clock advance originates there.

[trustthegoose]

Just saw both land — thanks for taking the parser side, and the write-up on why "exclude from the clock" would've been worse is clear.

I'll run test_suunto_nautic_gps_clock against my real Ocean capture and report back here.

This isn't my native field, so feel free to redirect my efforts where I'm missing cross-repository nuance.

[trustthegoose]

Ran it against my real Ocean capture (44,713 samples) through the actual production path (libdc_parse_raw_dive), checking for monotonic sample times and depths staying within range.

Before the fix (parser reverted to the pre-sample_ms version, clamp removed): 63 non-monotonic-timestamp violations, all clustered in the last ~2 minutes of the dive -- exactly where this capture's 8 CHUNK_GPS/CHUNK_GPS_ACCURACY chunks live.

With the fix (current submersion-patches + the clamp): 0 violations across all 44,713 samples.

Confirms both pieces on real data, not just the synthetic repro. Thanks again for tracking down the actual mechanism here.

[trustthegoose]

For the record on this thread -- the two submersion-app/submersion issues this turned up (#1640 the datetime fallback, and #1641/#1642/#1643 the general bottom-time/safety-review bugs) now have PRs open against suunto-nautic:

• Suunto Nautic/Ocean: fall back to the fingerprint for a dive with no GPS clock (#1640) submersion#1 -- Suunto Nautic/Ocean fingerprint datetime fallback (#1640)
• Fix bottom-time clamp and two stale-safety-review bugs (#1641, #1642, #1643) submersion#2 -- bottom-time clamp + stale safety-review fixes (#1641, #1642, #1643)

Both validated on-device against the real Ocean logbook.