Skip to content

Restrict the default calibration override to the legacy layout - #1

Closed
readme42 wants to merge 1 commit into
submersion-app:submersion-patchesfrom
readme42:fix/pnf-o2-cell-calibration
Closed

Restrict the default calibration override to the legacy layout#1
readme42 wants to merge 1 commit into
submersion-app:submersion-patchesfrom
readme42:fix/pnf-o2-cell-calibration

Conversation

@readme42

@readme42 readme42 commented Aug 4, 2026

Copy link
Copy Markdown

Parser side of submersion-app/submersion#855.

The defect

shearwater_predator_parser.c clears parser->calibrated when every enabled O2 sensor still carries the calibration value 2100, treating that as a factory default and therefore as cells that were never calibrated. The three per-cell DC_SAMPLE_PPO2 callbacks then never fire -- for Bluetooth downloads and Shearwater Cloud database imports alike, since both run this parser. The aggregate is unaffected, so the loss is silent: a WARNING in the log, and no way for the caller to tell suppression from a computer that has no cells at all.

On Petrel Native Format logs the inference does not follow. PNF records an explicit per-cell calibrated flag in the same byte the sensor mask is read from, and a Petrel 3 sets it for all three cells while writing 2100 for each:

opening[3]: sensor mask=0x07  (calibrated: true, true, true)   values=[2100, 2100, 2100]
closing[3]: sensor mask=0x07  (calibrated: true, true, true)   values=[2100, 2100, 2100]

The computer states all three cells calibrated, at the start of the dive and again at the end. The parser reads that flag and overrides it on the strength of the accompanying value.

Shearwater's own Cloud schema keeps the two apart as well: dive_logs carries startsensorNCalibrated as booleans separate from startsensorNCalibrationValue and startsensorNADCOffset, and dive_log_records stores sensorNMillivolts per sample.

The honest state of the numbers

An earlier version of this description claimed 2100 is the factor the computer itself uses, citing a Shearwater Cloud screenshot reading ppO2 1.3 at 62/65/62 mV. That was wrong -- the screenshot is a different dive. Fitted across all 419 CCR samples of the reference log:

model mean error max error
stored factor, adc * 0.021 0.034 bar 0.097 bar
rescaled, adc * 0.02047 0.008 bar 0.062 bar
offset, (adc - 1.6) * 0.021 0.009 bar 0.064 bar

The factor implied by this dive is ~0.02047, so cells derived with the stored 2100 read about 2.6% high, up to ~0.1 bar, against the computer's own voted ppO2. No simple correction is exact either: the implied offset is not constant (~1.93 early, ~1.67 mid-dive), so the vote is not a plain single-factor function of the logged bytes -- per-cell constants, temperature compensation, or a vote that is not a straight median.

Two things did hold up under checking: libdc reads the correct bytes (offsets 12/14/15 fit far better than any neighbouring triple; +13 is something else), and the calibrated flags are as quoted above.

Is that good enough to show

Yes. Cells differing from each other and from the computer's vote by up to ~0.1 bar is ordinary on a rebreather -- that scatter is the thing divers read cells for in the first place. The deviation here is systematic rather than random (all three sit above the vote), but at this magnitude it is well inside what a diver sees on the handset.

What matters for the display is that cell-to-cell divergence -- one cell drifting away from its neighbours -- comes through exactly, since a common scale error cancels between them. Today all three are discarded instead, which is a total loss.

What is still unclear

This is not a tidy diagnosis, and the reasoning should be read with that in mind.

  • Why the calibration value is an identical placeholder is unknown. Three physical cells do not calibrate to the same constant, so 2100 is plainly not three measurements -- but whether the Petrel 3 keeps its real per-cell constants elsewhere, or applies them before writing the samples, or something else entirely, I cannot say. The Cloud schema's sensorNADCOffset is an unexamined candidate; libdc never reads it.
  • Whether this is universal is unknown. One log is the entire evidence base. If every Petrel 3 writes 2100 regardless of calibration state, the "never calibrated" inference clearly does not hold and this change is right. If other logs carry real per-cell values, then this dive is unusual and the guard may be doing its job on it. More exports have been requested from the reporter in Dives / Dive / Dive profile – CCR sensor readings are missing. submersion#810.
  • The conversion is therefore approximate by an amount we cannot currently explain, rather than by an amount we have accounted for. See the numbers above.

None of this changes what the log states plainly -- three cells recorded, all three flagged calibrated -- or that they are currently discarded in full. But if the answer is "wait for the other logs before touching the parser", that is a reasonable call.

The change

One condition: the override keeps applying to the legacy Predator layout it was written for (upstream 3e36cad, 2018, when calibration came from fixed offset 87), and no longer to PNF, where the record carries its own calibrated flag.

-  if (nsensors && nsensors == ndefaults) {
+  if (nsensors && nsensors == ndefaults && !pnf) {

Notes

  • Upstream master (8cd68fa) still carries the guard unchanged; 48 commits have touched this file since 2018 without revisiting it, and no issue upstream or on Subsurface mentions it.
  • Documented app-side as packages/libdivecomputer_plugin/patches/0005-shearwater-pnf-o2-cell-calibration.patch; the regression test and fixture are in test(libdc): pin the Petrel 3 O2 cell regression from #855 submersion#857.
  • Based on submersion-patches at 45627c5, so it carries the Shearwater download revert already on that branch. The app's submodule is pinned one commit behind that, at 1a47a01 -- bumping it to pick this up also picks up the revert.

When every enabled O2 sensor still carries the calibration value 2100,
the parser treats that as a factory default, concludes the cells were
never calibrated, and disables all of them. On Petrel Native Format logs
that conclusion does not follow.

PNF records an explicit per-cell calibrated flag in the same byte the
sensor mask is read from, and a Petrel 3 sets it for all three cells
while writing 2100 for each of them, at the opening and the closing
record alike. The default value therefore says nothing about whether the
diver calibrated, and suppressing on it discards cells the computer
itself reports as calibrated.

2100 is also the factor the computer uses. Its display shows ppO2 1.3
where the cells read 62/65/62 mV, and it votes by median:
62 * 0.021 = 1.302. Measured on a Petrel 3 log with 419 CCR samples, the
cells derived this way track the computer's own voted ppO2 within 0.05
bar across the whole dive.

The heuristic keeps applying to the legacy Predator layout it was written
for, where calibration is read from a fixed offset rather than from a
record carrying its own calibrated flag.
@readme42 readme42 changed the title Restrict the default calibration override to the legacy layout Restrict the default calibration override to the legacy layout (needs a decision, see description) Aug 4, 2026
@readme42
readme42 marked this pull request as draft August 4, 2026 17:29
@readme42 readme42 changed the title Restrict the default calibration override to the legacy layout (needs a decision, see description) Restrict the default calibration override to the legacy layout Aug 4, 2026
@readme42
readme42 marked this pull request as ready for review August 4, 2026 17:42
@readme42
readme42 marked this pull request as draft August 6, 2026 07:03
@readme42 readme42 closed this Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant