Skip to content

fix(dive-computer): resolve CCR transmitter cylinders from every transmitter reading - #2319

Merged
ericgriffin merged 5 commits into
submersion-app:mainfrom
alpheios-one:github-issue-2318-ccr-transmitters
Sep 25, 2026
Merged

ericgriffin merged 5 commits into
submersion-app:mainfrom
alpheios-one:github-issue-2318-ccr-transmitters

Conversation

@alpheios-one

Copy link
Copy Markdown
Collaborator

Related Issue

Closes #2318

Summary

On a Shearwater CCR dive with a diluent and an oxygen transmitter, both transmitters report in the same sample, but ProfileSample.pressureBar / tankIndex only hold the last reading, which on the Petrel 3 is always the oxygen transmitter. The resolver relied on that pair, so:

All four were confirmed against the raw downloads of eight real dives and Shearwater Cloud (details in #2318).

Changes

All in parsed_tank_resolver.dart:

  • Oxygen cylinder (usage oxygen): the computer's own link, else an oxygen-tagged gas mix, else the 100/0 default. The breathed-gas rule no longer applies to it.
  • Diluent cylinder (usage diluent): the computer's own link, else the most-breathed diluent-tagged gas, else the first diluent-tagged gas. Without any diluent-tagged gas the previous rules still apply (keeps the fix(dive-log): auto-assign 100% O2 when a cylinder's role is set to oxygen supply #1972 test behavior).
  • Surfacing trim: surfacing points read every transmitter from tankPressuresBar (same fallback rule as groupPressuresByTank), so each cylinder is trimmed at surfacing, not only the last one reported.
  • Gases without a transmitter get the sensorless roles (usage tag, then the CCR bailout ranking from feat(dive-computer): rank sensorless CCR bailout gases against each other #1752). The bailout ranking compares against every untagged gas of the dive, including one on a transmitter, so a bailout cylinder with its own transmitter does not promote a leaner deco gas to Bailout. The breathed diluent is listed first among them, so the dive-level diluent (fix(dive-computer): derive the CCR diluent gas mix from the resolved Diluent cylinder #2034) is the one used when the diluent has no transmitter.
  • _firstFreeIndex also accounts for tankPressuresBar, so synthesized cylinders never collide with a transmitter index.

Deliberately unchanged: the breathed-gas rule for untagged tanks still keys on the sample's own tankIndex. Crediting every reporting transmitter would hand each tank the dive's main gas and override a computer's own tank-to-gas link on open circuit dives. Attributing gas per transmitter properly (e.g. by pressure drop) is left as a possible follow-up.

How to Test

Needs a Shearwater CCR dive downloaded with two AI transmitters named D1/O2 (or an HP CCR setup).

  1. Open Dive Computers, select the Shearwater, and use Re-parse all dives.
  2. Open a CCR dive: the O2 cylinder shows 100/0, the diluent cylinder the breathed diluent, the open circuit bottom gas is Bailout and 99/0 is Deco.
  3. The diluent cylinder's end pressure matches the value at surfacing (compare with Shearwater Cloud or the pressure graph), not the last reading after the dive.

Test Plan

  • New parsed_tank_resolver_ccr_transmitters_test.dart models the real download (gas list OC1 99/0, OC5 15/55, DIL 15/55; both transmitters in every sample, O2 reported last) and failed before the fix with exactly the values seen in the field (O2 15/55, diluent 99/0, diluent end 13.4 bar)
  • Guard tests: an open circuit tank keeps the computer's own gas link while another transmitter reports last; a CCR dive with only an O2 transmitter picks the breathed diluent over an unused programmed one; a bailout on its own transmitter keeps a 50% gas as Deco
  • flutter test test/features/dive_computer passes (606 tests)
  • flutter analyze clean on the changed areas, dart format clean
  • Pre-push hook (format, analyze, affected tests) passed
  • Manual testing: not done in this PR

Note: three tests under test/features/dive_log (dive_header_map_bottom_edge_test, deco_filter_providers_test, equipment_attr_filter_providers_test) fail locally on Windows. They do not import anything from dive_computer and are unrelated to this change.

…smitter reading

A sample's pressureBar/tankIndex pair holds only the last transmitter it
reported, which on a Shearwater CCR is always the oxygen transmitter. Read
every transmitter from tankPressuresBar instead, resolve oxygen and diluent
cylinders by their usage tag before the breathed-gas rule, and give gases
without a transmitter the sensorless roles.

Refs submersion-app#2318
Crediting every reporting transmitter with the breathed gas overrode a
computer's own tank->gas link. The CCR fix does not need it: oxygen and
diluent cylinders resolve by usage. Per-transmitter readings stay for the
surfacing trim. Put the breathed diluent first among the unclaimed gases so
the dive-level diluent is the one used, not the first programmed one.

Refs submersion-app#2318
A bailout cylinder on its own transmitter is claimed before the unclaimed
gases are ranked, so the leanest remaining gas was promoted to Bailout.
Rank against every gas with no reported usage and assign only the
unclaimed ones.

Refs submersion-app#2318
@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

📦 Build artifacts for this PR · commit 9738c41

Platform Download
Android (APK) android-apk
macOS macos-build
Windows windows-build
Linux linux-build

Artifacts expire in 7 days. Downloading requires being signed in to GitHub. macOS needs two extractions: unzip the downloaded artifact, then unzip the submersion-macos.zip inside it to get a runnable submersion.app. The build is ad-hoc signed — right-click → Open on first launch.

Updated automatically on each push.

@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.33333% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...e_computer/data/services/parsed_tank_resolver.dart 98.33% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Preserve legacy single-transmitter readings when applying surfacing trim.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Fixes CCR transmitter cylinder resolution, gas assignment, sensorless roles, and surfacing pressure trimming.

Changes:

  • Resolves oxygen and diluent gases across transmitter readings.
  • Corrects bailout/deco role inference and index allocation.
  • Adds comprehensive CCR regression tests.
File Summary
test/​features/​dive_computer/​data/​services/​parsed_tank_resolver_ccr_transmitters_test.dart Adds CCR multi-transmitter regression coverage.
lib/​features/​dive_computer/​data/​services/​parsed_tank_resolver.dart Updates cylinder, gas, role, pressure, and index resolution.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/features/dive_computer/data/services/parsed_tank_resolver.dart Outdated
Match groupPressuresByTank, which stores such a reading on tank 0, so the
surfacing trim does not drop it. Also covers the first-diluent fallback.

Refs submersion-app#2318
@alpheios-one

Copy link
Copy Markdown
Collaborator Author

@ericgriffin This is complete and ready for testing and review. All CI checks are green, the Copilot finding is fixed with a regression test and resolved, and the branch merges cleanly. The quickest check is "Re-parse all dives" on a Shearwater with CCR dives that use two transmitters (see How to Test in the description).

@JaspalX

JaspalX commented Sep 25, 2026

Copy link
Copy Markdown

out of interest, how does the petrel know which transmitter is oxygen? it just gets a pressure, not a gas mix..so i don't think that assumption is safe, but I may be wrong ofc. I happen to dive o2 as my 3rd sensor (first two dil/bailout), but I could easily put the o2 pressure sensor as the first one.

@alpheios-one

Copy link
Copy Markdown
Collaborator Author

Good question. It doesn't rely on the transmitter's position. On a CCR dive, libdivecomputer's Shearwater parser tags each transmitter from the name the diver gives it on the Petrel: a name starting with "O" becomes oxygen, one starting with "D" becomes diluent (shearwater_predator_parser.c, DC_FIELD_TANK). In HP CCR mode the diluent and O2 channels are fixed by Shearwater. So an O2 sensor named e.g. "O2" is recognized on T1 just as well as on T3.

If the transmitters keep their default names (T1..T4), no usage is reported and this PR changes nothing for them: the gas falls back to the previous rules and the role comes from the transmitter registry.

libdivecomputer already reported this correctly. The bug was on the Submersion side:

  1. The resolver checked which gas was "breathed while this transmitter reported" before looking at the transmitter's usage. On the loop the active gas is always the diluent, so the O2 cylinder was labeled with the diluent mix.
  2. Our plugin wrapper keeps a single pressureBar/tankIndex pair per sample, which only holds the last transmitter reported, on a two-transmitter Petrel always the O2 one. The resolver and the surfacing trim read only that pair, although tankPressuresBar carries every transmitter. So the diluent cylinder fell back to the first gas mix, and its end pressure kept the post-surfacing reading.
  3. Gases without a transmitter only got the open circuit role heuristic, ignoring the diluent tag and the CCR bailout ranking.

This PR fixes all three: oxygen and diluent cylinders resolve by their usage before the breathed-gas rule, the surfacing trim reads every transmitter, and gases without a transmitter get the same roles as on a sensorless dive.

@JaspalX

JaspalX commented Sep 25, 2026

Copy link
Copy Markdown

ah, cool. i'd forgotten about the prefixes. thanks for explaining

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified, and all approval assessments are positive.

Review effort: Lite
Findings: None

Resolved since last review (1)

@ericgriffin
ericgriffin merged commit 3232ae8 into submersion-app:main Sep 25, 2026
36 checks passed
@alpheios-one
alpheios-one deleted the github-issue-2318-ccr-transmitters branch September 25, 2026 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

CCR import with two transmitters: O2 cylinder gets the diluent mix, diluent end pressure skips the surfacing trim (Shearwater)

4 participants