issue-70 UDDF dual-tank only shows one tank on dive profile#86
Conversation
|
@ericgriffin I added a couple logs on detection, but they don't seem to be showing up. Where would I see them or am I using the logger improperly? |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
6a1658e to
6ae767f
Compare
The provided UDDF file is non-conformant with the UDDF specification, which requires the <tankdata> element to have an id attribute. This change adds ordered fallback mapping during import when tankdata ids are missing, so tankpressure refs can still be associated with the correct tanks.
6ae767f to
0105055
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves UDDF import compatibility for Shearwater (and other non-conforming) exports by preserving multi-tank pressure series even when <tankdata> elements omit required id attributes, and updates the dive profile chart to reliably show multi-tank pressure lines.
Changes:
- Add fallback tank-ref resolution that maps unmatched
<tankpressure ref="...">values to<tankdata>entries by order when tank IDs are missing, with debug logging for spec violations. - Fix multi-tank chart visibility initialization so newly-discovered tank pressure series show up in the legend/rendering state.
- Add unit + integration tests covering conforming and non-conforming UDDF multi-tank pressure imports.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/uddf_round_trip_test.dart | Adds integration coverage for storing separate tank pressure series (including missing <tankdata id> cases). |
| test/features/dive_import/data/services/uddf_entity_importer_test.dart | Adds importer-level test ensuring pressure data for both tanks is passed to the repository. |
| test/core/services/export/uddf/uddf_import_service_test.dart | New tests validating order-based fallback mapping in UddfImportService. |
| test/core/services/export/uddf/uddf_full_import_service_test.dart | New tests validating the same fallback behavior in UddfFullImportService. |
| lib/features/dive_log/presentation/widgets/dive_profile_chart.dart | Initializes per-tank visibility state for newly-available tank pressure series; tweaks hidden-tank check. |
| lib/core/services/export/uddf/uddf_import_service.dart | Implements fallback ref→tank-index mapping and logs missing <tankdata id> spec violations. |
| lib/core/services/export/uddf/uddf_full_import_service.dart | Mirrors the same fallback mapping + logging in the “full import” path. |
Logs should appear in console output but only in debug mode ("flutter run", not "flutter run --release"). Are they not showing up in the console? |
ericgriffin
left a comment
ericgriffin
left a comment
There was a problem hiding this comment.
Looks good, thanks. Let's address the copilot suggestions, and then I'll merge.
|
@ericgriffin yeah, can't see any logs.
|
|
@ericgriffin sorry I missed that codex had checked in my workspace files. |
Summary
This PR improves UDDF import compatibility for Shearwater exports.
On the UDDF side, it handles multi-tank imports where waypoint pressure data references entries do not include explicit id attributes (a violation of the UDDF spec).
The new algorithm falls back to order-based entries based on the ref mapping and also logs a debug warning on spec violations.
Changes
Fixed multi-tank UDDF import to preserve separate tank pressure series when waypoint pressure data references default tank identifiers
Additional unit tests for both conforming and non-confirming UDDF formats
Test Plan
flutter testpassesflutter analyzepassesScreenshots