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
