feat(dive-computer): rank sensorless CCR bailout gases against each other - #61
Merged
Merged
Conversation
…ther Without a transmitter, dive_tanks.role was inferred per gas in isolation from a single O2/He threshold, ignoring the dive-mode context. That misclassified programmed CCR diluent and bailout gases the computer never attaches to a tank record. libdivecomputer's stock dc_gasmix_t already carries a per-gas dc_usage_t (oxygen/diluent/sidemount), independent of any tank record, but the plugin never bridged it past the C layer. Thread GasMix.usage through pigeon and all four platform converters, then use it in the sensorless resolver: - usage reported by the device (oxygen/diluent/sidemount) is authoritative, regardless of dive mode. - On a CCR dive, the remaining gases (bailout candidates) are ranked against each other instead of scored alone: lowest O2 is bailout (ties broken by higher helium, a further tie gives bailout to all tied gases), O2 >= 41% is deco, everything else is stage. - Every other dive mode keeps the original single-threshold heuristic unchanged. Ref: submersion-app#1747
|
📦 Build artifacts for this PR · commit
Artifacts expire in 7 days. Downloading requires being signed in to GitHub. macOS needs two extractions: unzip the downloaded artifact, then unzip the Updated automatically on each push. |
CI's dart format --set-exit-if-changed . flagged all three files touched by the previous commit. The pigeon-generated dive_computer_api.g.dart needs a format pass after regeneration too, not just the hand-written files -- no re-run of pigeon or build_runner needed, the generated content itself was already correct.
There was a problem hiding this comment.
🔵 Needs a closer look
It changes cross-platform native bridging (C/C++/JNI/Swift/Linux GObject) where build/ABI issues are hard to fully validate from diffs alone.
Pull request overview
Adds end-to-end support for per-gas usage metadata from libdivecomputer so sensorless CCR dives can correctly infer/compare bailout gases (rather than scoring each mix in isolation). This fits the dive-computer import pipeline by improving how downloaded dives are converted/bridged and how parsed gases are mapped into DownloadedTank roles.
Changes:
- Thread
GasMix.usage(dc_usage_t) through Pigeon + all platform converters (Android JNI/Kotlin, iOS/macOS Swift, Windows C++, Linux C/GObject) and the shared C download layer. - Update sensorless tank role inference to respect device-reported usage and to rank CCR bailout candidates against each other (with O2/He tie-break rules).
- Add/extend unit tests for the new CCR sensorless ranking and reported-usage mapping behavior.
File summaries
| File | Description |
|---|---|
| test/features/dive_computer/data/services/parsed_tank_resolver_test.dart | Adds new tests covering CCR sensorless bailout ranking and direct usage mapping. |
| lib/features/dive_computer/data/services/parsed_tank_resolver.dart | Implements sensorless CCR role inference using per-gas usage and bailout ranking rules. |
| packages/libdivecomputer_plugin/pigeons/dive_computer_api.dart | Adds nullable usage to the Pigeon GasMix model and documents its meaning. |
| packages/libdivecomputer_plugin/lib/src/generated/dive_computer_api.g.dart | Regenerated Dart bindings to include GasMix.usage in encode/decode. |
| packages/libdivecomputer_plugin/macos/Classes/libdc_wrapper.h | Extends libdc_gasmix_t with usage to carry dc_usage_t through the shared C layer. |
| packages/libdivecomputer_plugin/macos/Classes/libdc_download.c | Populates dive->gasmixes[i].usage from dc_gasmix_t.usage during extraction. |
| packages/libdivecomputer_plugin/darwin/Sources/LibDCDarwin/DiveComputerHostApiImpl.swift | Maps native gm.usage into Pigeon GasMix.usage for Darwin. |
| packages/libdivecomputer_plugin/ios/Classes/DiveComputerApi.g.swift | Regenerated Swift bindings to include GasMix.usage in list serialization. |
| packages/libdivecomputer_plugin/android/src/main/cpp/libdc_jni.cpp | Extends gasmix JNI return to include usage in the positional array consumed by Kotlin. |
| packages/libdivecomputer_plugin/android/src/main/kotlin/com/submersion/libdivecomputer/DiveComputerHostApiImpl.kt | Reads usage (if present) and sets nullable GasMix.usage (treating 0 as null). |
| packages/libdivecomputer_plugin/android/src/main/kotlin/com/submersion/libdivecomputer/SerialDownloadRunner.kt | Same usage mapping for the serial download runner path. |
| packages/libdivecomputer_plugin/android/src/main/kotlin/com/submersion/libdivecomputer/DiveComputerApi.g.kt | Regenerated Kotlin Pigeon bindings adding GasMix.usage to list serialization. |
| packages/libdivecomputer_plugin/windows/dive_converter.cc | Includes gasmix usage when constructing Windows Pigeon GasMix values. |
| packages/libdivecomputer_plugin/windows/dive_computer_api.g.h | Regenerated Windows Pigeon header with nullable GasMix.usage support. |
| packages/libdivecomputer_plugin/windows/dive_computer_api.g.cc | Regenerated Windows Pigeon implementation to encode/decode the added usage field. |
| packages/libdivecomputer_plugin/linux/dive_converter.c | Includes gasmix usage when constructing Linux Pigeon GasMix GObjects. |
| packages/libdivecomputer_plugin/linux/dive_computer_api.g.h | Regenerated Linux Pigeon header with GasMix.usage field/accessor. |
| packages/libdivecomputer_plugin/linux/dive_computer_api.g.cc | Regenerated Linux Pigeon implementation with usage storage/serialization and cleanup. |
Review details
- Files reviewed: 17/18 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… not a sidemount role Copilot review on #61: the doc comment and test name implied sidemount usage kept a distinct "sidemount" role, but it actually maps to TankRole.backGas -- the device flag says the gas is on a sidemount cylinder, not which side, so it cannot pick between sidemountLeft and sidemountRight. Correct the comment and rename the test to match the real behavior; no logic change.
alpheios-one
added a commit
that referenced
this pull request
Sep 13, 2026
…ther (submersion-app#1752) * feat(dive-computer): rank sensorless CCR bailout gases against each other Without a transmitter, dive_tanks.role was inferred per gas in isolation from a single O2/He threshold, ignoring the dive-mode context. That misclassified programmed CCR diluent and bailout gases the computer never attaches to a tank record. libdivecomputer's stock dc_gasmix_t already carries a per-gas dc_usage_t (oxygen/diluent/sidemount), independent of any tank record, but the plugin never bridged it past the C layer. Thread GasMix.usage through pigeon and all four platform converters, then use it in the sensorless resolver: - usage reported by the device (oxygen/diluent/sidemount) is authoritative, regardless of dive mode. - On a CCR dive, the remaining gases (bailout candidates) are ranked against each other instead of scored alone: lowest O2 is bailout (ties broken by higher helium, a further tie gives bailout to all tied gases), O2 >= 41% is deco, everything else is stage. - Every other dive mode keeps the original single-threshold heuristic unchanged. Ref: submersion-app#1747 * style: apply dart format to the CCR bailout gas role change CI's dart format --set-exit-if-changed . flagged all three files touched by the previous commit. The pigeon-generated dive_computer_api.g.dart needs a format pass after regeneration too, not just the hand-written files -- no re-run of pigeon or build_runner needed, the generated content itself was already correct. * fix(dive-computer): clarify that DC_USAGE_SIDEMOUNT maps to back gas, not a sidemount role Copilot review on #61: the doc comment and test name implied sidemount usage kept a distinct "sidemount" role, but it actually maps to TankRole.backGas -- the device flag says the gas is on a sidemount cylinder, not which side, so it cannot pick between sidemountLeft and sidemountRight. Correct the comment and rename the test to match the real behavior; no logic change. * fix(dive-computer): use an epsilon comparison for the CCR bailout tie-break Copilot review on submersion-app#1752: the lowest-O2 and helium tie-break comparisons used exact double equality. Each of the four platform converters computes o2Percent/hePercent independently as fraction * 100.0 from the native dc_gasmix_t, so two mixes the diver set to the same nominal percentage can differ by a few ULPs instead of being bit-identical -- exact == would then miss a real tie and misassign bailout/deco/stage roles. Add a small epsilon comparison and a regression test simulating that floating-point noise. * docs(dive-computer): fix malformed dartdoc link in the sidemount role comment Copilot review on submersion-app#1752: [TankRole.sidemountLeft]/`Right` only linked sidemountLeft and left Right as a stray inline-code fragment. Link both roles properly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Without a transmitter,
_inferRole()inparsed_tank_resolver.dartscored every reportedgas in isolation against a single fixed threshold (no helium and O2 >= 41% -> Deco,
otherwise Back Gas), regardless of the recognized dive mode. On a CCR dive with several
programmed diluent and bailout gases, that produced wrong roles whenever none of them had
a transmitter/tank record.
libdivecomputer's stock
dc_gasmix_talready carries a per-gasdc_usage_t(oxygen/diluent/sidemount), independent of any tank record - the Shearwater parser sets it - but
the plugin never bridged that field past the C layer (
GasMixonly ever exposedo2Percent/hePercent).This threads
GasMix.usagethrough pigeon and all four platform converters, then uses itin the sensorless path:
regardless of dive mode.
against each other instead of scored alone: the lowest O2 percentage becomes Bailout
(tie -> higher helium wins; a further tie gives Bailout to every tied gas; a gas that
only loses the helium tie-break falls through to the rules below), O2 >= 41% becomes
Deco, everything else becomes Stage.
unchanged.
Native changes
GasMix.usagerequired bridgingdc_gasmix_t.usagethrough the shared C download engine(
macos/Classes/libdc_download.c,libdc_wrapper.h, used by all platforms) and eachplatform converter (Linux, Windows, macOS/iOS Swift, Android JNI + Kotlin), following the
exact pattern already used for
TankInfo.usage.I could not compile-test the native changes in this environment (no CMake/MSVC/g++
toolchain available) - they were written by precisely mirroring the existing, already
-shipped
usageplumbing forTankInfoat each of the same call sites. Please give thenative diff a close look in CI/review.
Testing
flutter test test/features/dive_computer/data/services/parsed_tank_resolver_test.dartdiluent/sidemount), the bailout/deco/stage ranking, the helium tie-break (both the
win and the fall-through), the all-tied case, and that non-CCR dive modes are
unaffected.
dart analyzeclean onlib/features/dive_computer/andpackages/libdivecomputer_plugin/lib.dart run pigeon --input pigeons/dive_computer_api.dart.Not done here
dc_gasmix_t.usagethe same way Shearwater does - only verified against the vendoredShearwater parser source.
discussion on the upstream issue.
Ref: submersion-app#1747