I am trying to track down an issue where Subsurface thinks that a cylinder is used that clearly isn't used. This got me back to looking at our handling of cylinders and sensors and ... oh my.
So here's the scenario. User has a dive computer that allows them to create multiple cylinders with different sizes and gases (note, this is one of the very few dive computers that truly has a concept of cylinders - this is the Cobalt2). Here it's a total of four, two each air and EAN32, and for each gas either AL80 or HP100. In this instance the user happens to have picked what is essentially the third cylinder in that list. Because of our rules for hiding unused cylinders, the two before that one are also shown, so the user wants to delete them. So far, so good. This is truly recreational diving, so no actual gas changes (but on the Cobalt2 we have a gas change event on the very first sample that indicates the cylinder that is actually in use. When we load this dive from git, every sample has sensor[0] = 2 (makes sense, we are using the third cylinder) and sensor[1] = 1 Wait, what? Why is our internal idea of the 'o2 sensor' randomly the second tank on the list. Even though this tank is never used? Now let's try to delete the second tank, which is clearly unused, right? Well, the user gets a warning that there are sensor readings on that cylinder. Which makes no sense. Even worse, apparently when the user says "delete anyway", on Windows they get a Subsurface crash (I can't reproduce that on Linux - not even illegal accesses with ASAN). But let's go back to that. Why is that cylinder shown as used? Well, turns out that both in load-git.c and parse.c we have essentially this code: sample->sensor[0] = sanitize_sensor_id(state->active_dive, !state->o2pressure_sensor); sample->sensor[1] = sanitize_sensor_id(state->active_dive, state->o2pressure_sensor); But, if we don't have an o2 pressure sensor, why would we put a valid cylinder number there? That sensor should say "NO_SENSOR" (or -1) unless we have actually identified an O2 sensor... But it gets better. Let's say we fix the loading and mark the second sensor (which is the O2 sensor) as -1. And now we delete cylinder 0 first. At that point all samples have sensor[0] = 2 (still, makes sense) and sensor[1] = -1 (which to me also makes sense). But now we go through cylinder_renumber() which calls dc_cylinder_renumber() which calls sample_renumber() And there we quite explicitly say "oh, if the sensor was -1, make it 0 now"... and now all samples have sensor[0] = 1 (makes sense, the old cylinder 2 is now cylinder 1), and sensor[1] = 0 !!! Yeah, this is not doing what it should be doing. At first I was trying to figure out all these complicated ways how to deal with the second sensor value. But then I thought... for an OC dive we should completely ignore sensor[1]... there is no O2 cylinder in use... - that seems the easiest fix. Thoughts? /D _______________________________________________ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface