subsurface / subsurface

This is the official upstream of the Subsurface divelog program
https://subsurface-divelog.org
GNU General Public License v2.0
2.64k stars 512 forks source link

Transmitter data goes to wrong cylinder with no way to fix it #4291

Open steffen-kdab opened 2 months ago

steffen-kdab commented 2 months ago

Describe the issue:

Issue long description:

In recent versions, Subsurface has started to assign transmitter data to the wrong cylinder. I have a single transmitter, always attached to my twinset. But sometimes, the transmitter data gets assigned to a deco cylinder instead, and there seems to be no way change it without messing up.

I think a possible trigger of this is when there is a dive where a cylinder reported by the dive computer is unused. On import, the red warning bar at the bottom of the window says "Warning: different number of gasses (2) and cylinders (1)". The next dive in the same trip will then also be wrong.

Example: We were planning a single long dive on a wreck in the 20-30m range, with EAN28 bottom gas, and a AL80 with EAN47 deco gas (good examples because "odd" choice of non-standard gasses...). Plans changed because of strong current that day and difficulties hooking onto the wreck, so I ended up with first a short dive on back gas only to scout and retrieve the anchor that was nowhere near the wreck, and then a 2nd dive on the wreck where both gasses were used.

Both logged dives ended up having the transmitter data wrongly assigned to the EAN47 cylinder. Here the 2nd dive right after importing: image

In the graph, the order of cylinders is correct, but pressure graph is missing, since it's out of range(?): image

If I attempt to fix it by changing the cylinder contents to match reality and/or manually entering start and end pressures, it just gets worse: image

If there is anything I can do to help debug this, please let me know.

Operating system:

Ubuntu 24.04, English

Shearwater Perdix AI

Subsurface version:

6.0.5231

The problem started some months ago. Before that, it almost(?) always got the cylinders right

Official release, AppImage, but same problem with deb package

Steps to reproduce:

Import dive from Shearwater Perdix AI with an unused deco cylinder.

Current behavior:

Transmitter data from back gas gets assigned to deco cylinder in the log

Expected behavior:

Transmitter data should be assigned to the gas that was actually used, or at least it should be possible to fix it after importing.

Additional information:

Mentions:

Patrick-BS commented 2 months ago

Once i had that issue too. I was able to fix it in the ssrf file. If i remeber it right: Opening it with texteditor and correcting/adding the correct Tank-IDs to the tank list and the dive log.

steffen-kdab commented 2 months ago

Once i had that issue too. I was able to fix it in the ssrf file. If i remeber it right: Opening it with texteditor and correcting/adding the correct Tank-IDs to the tank list and the dive log.

Thanks, I was thinking I could do that, but to be honest I don't even know which file to edit these days. I've been using the cloud storage option for a long time, so I don't have a simple xml file any more (?)

I tried editing the file for the relevant dive under ~/.subsurface/cloudstorage/, but that just resulted in an error message about the cache being corrupted and a new copy of my whole log being made:

$ ls -la  ~/.subsurface/cloudstorage/
totalt 20
drwxrwxr-x  5 steffen steffen 4096 aug 18 11:57 .
drwxrwxr-x  7 steffen steffen 4096 aug 18 21:57 ..
drwxrwxr-x 10 steffen steffen 4096 aug 18 11:57 e5d98fd2d57714cf
drwxrwxr-x 10 steffen steffen 4096 jan  7  2024 e5d98fd2d57714cf.1
drwxrwxr-x  3 steffen steffen 4096 maj 25 16:20 localrepo
mikeller commented 2 months ago

The problem started some months ago. Before that, it almost(?) always got the cylinders right

Correct, this is not something new.

The problem here is that dive computers don't care about the mapping of gasmixes to tank sensor information - they treat these as two completely separate lists of things, that are sent to the dive log software independently. And on the dive log side there is not much that can be done to reliably figure out what the correct mapping between gasmixes and tank sensor data is, short of asking the user to enter it. Currently Subsurface has the shortcoming that it does not have functionality to allow (or force) the user to define the mapping between these lists, it justs assumes a 1 : 1 mapping of gasmixes to sensor data.

This is made worse for Shearwater computers by the fact that the Shearwater firmware automatically re-orders gasmixes based on their O2 / He content - thus changing what sensor a gasmix is mapped to when the gasmix is changed.

So the best (but not great) way to get this right is to make sure that the order that the tank sensors are registered with the dive computer is identical to the order that the respective gasmixes are listed on the dive computer - or else manually edit the .ssrf file after the dive.

mikeller commented 2 months ago

Thanks, I was thinking I could do that, but to be honest I don't even know which file to edit these days. I've been using the cloud storage option for a long time, so I don't have a simple xml file any more (?)

If you are using cloud storage, fixing after the fact is even more involved:

steffen-kdab commented 2 months ago

Thanks! I saved to XML, swapped the order of the cylinders in the XML file, and swapped the cylinder="" indexes on the gas change events, opened the file again and saved to cloud storage. Now it looks correct again.

I guess this means that Subsurface always just assigns transmitter data from the top of the cylinder list. I wonder if this could be done better, like trying to match the data with which cylinder is actually in use. Probably not feasible in all cases, like sidemount or extra bottom stage where multiple cylinders contain the same mix? There should probably be UI to manually assign transmitters, or reorder the cylinder list...

mikeller commented 2 months ago

@steffen-kdab: Glad it worked for you.

I guess this means that Subsurface always just assigns transmitter data from the top of the cylinder list. I wonder if this could be done better, like trying to match the data with which cylinder is actually in use. Probably not feasible in all cases, like sidemount or extra bottom stage where multiple cylinders contain the same mix? There should probably be UI to manually assign transmitters, or reorder the cylinder list...

It could definitely be done better. Unfortunately though I suspect that there won't be any easy ways to do this. The biggest hurdle is probably that in the current implementation the formats of the data persisted on disk / in git have both the gasmix and the tank sensor information linked to the same cylinder element - so without making a change to the format (and thus potentially breaking backwards compatibility to older versions of the application), it will not be possible to make this configurable without at the same time losing track of what the original data was that was downloaded from the dive computer, and is needed to display meaningful profiles (e.g. which gasmix was used at which point in the dive). And it will obviously need some extra UI for the user to enter and manage their mappings.

harrydevil commented 2 months ago

I also end up editing the XML to fix this issue. If a feature to change the starting gas like you change the a gas switch with a right click would be incredibly helpful.

mikeller commented 2 months ago

If a feature to change the starting gas like you change the a gas switch with a right click would be incredibly helpful.

You can already do this - right-click into the profile at the very start of the dive, then select 'Add gas change', and select the desired gas.

This isn't terribly intuitive, the command label should probably be 'Set initial gasmix' or similar in this case.