Few bugs and a feature req from testing Subsurface

[Linus Torvalds]

On Thu, Mar 13, 2014 at 2:06 PM, Miika Turkia <miika.turkia at gmail.com> wrote:
> On Mon, Mar 10, 2014 at 11:34 PM, Linus Torvalds <torvalds at linux-foundation.org> wrote:
>>
>> Yes. We currently have no really good way of *fixing* dive computer
>> event association with cylinders.
>
> So here is log of the two dives. Seems that Vyper, that was downloaded
> first, does not have any gas change events on it. On the other hand Stinger
> has one such event...
>
> Shouldn't a gaschange event be generated on download even if the computer
> does not have such? This dive is the only one with 1 gaschange event among
> the 35 downloaded from my two computers, all the other dives are without
> one. Do we only generate gaschange events when diving air?

Ok, finally got to look at this, and it does show bugs in subsurface
when I test things, but neither one is the behaviour you are finding
obnoxious.

There are two bugs that shows in subsurface:

 - the "default cylinder" behavior is broken, broken, broken. You have
set your default cylinder to AL80 (which is actually the default if
nothing has been set manually), and at least one of your dive
computers supports two cylinders, so EVEN THOUGH THERE IS NO CYLINDER
DATA, subsurface will generate these completely broken cylinder
entries in the XML:

  <cylinder size='11.094 l' workpressure='206.843 bar'
description='AL80' o2='32.0%' />
  <cylinder size='11.094 l' workpressure='206.843 bar' description='AL80' />
  <cylinder size='11.094 l' workpressure='206.843 bar' description='AL80' />

   where the two last ones are just pure garbage.

   The f*cking "default cylinder" crap needs to die, die, die. It's
wrong. I have that in my own files too from more recent dives, with
some of my files having this:

  <cylinder size='12.0 l' workpressure='200.0 bar' description='12L 200 bar' />
  <cylinder size='11.094 l' workpressure='206.843 bar' description='AL80' />
  <cylinder size='11.094 l' workpressure='206.843 bar' description='AL80' />
  <cylinder size='11.094 l' workpressure='206.843 bar' description='AL80' />
  <cylinder size='11.094 l' workpressure='206.843 bar' description='AL80' />
  <cylinder size='11.094 l' workpressure='206.843 bar' description='AL80' />
  <cylinder size='11.094 l' workpressure='206.843 bar' description='AL80' />
  <cylinder size='11.094 l' workpressure='206.843 bar' description='AL80' />

because the Suunto HelO2 supports up to eight cylinders. But 7 of
those cyliners are just crap.

Dirk, this is all your fault. It goes back to commit 904c20feef90 and
friends, as far as I can tell. It's broken. It's wrong. It's annoying.
At *most*, the default cylinder should affect the first cylinder only.
Or that thing should just be deleted, because it's worse than not
having a default cylinder at all.

 - more seriously, the cylinder equipment editing is odd. When I load
your XML file, I see only two cylinders (probably because the third
one is empty and not mentioned by any dive computer), but when I try
to delete the second bogus one, it *works*, but the equipment list
goes empty. I know it works because if I save the file, it gets saved
with one fewer cylinder entries. So this is a GUI error.

Tomaz, mind looking at that one? It's a pretty serious GUI error. Just
load Miika's xml file and try to delete the second cylinder.

Anyway, with those two bugs mentioned, let's look at your actual behaviour:

What happens is that normally, subsurface does not generate gas change
events at all, and just starts at gas 0. That is actually true for
both of your computers, and no gas change was ever generated.

However, when *merging* the dive data (which happens automatically
when you download a dive from a computer that matches a previous
dive), what happened is that you had two different computers with
different gas mixes, and so when merging, subsurface says "ok, you
were obviously diving with two cylinders". Quite reasonable of
subsurface, and it actually happens in real life that you have
different dive computers tracking different cylinders. I have several
cases of that. So the dives get merged, and the cylinder that *used*
to be the first cylinder of the second dive becomes the second
cylinder of the merged dive. All sane.

But that means that now the gas events from that second dive computer
are about the *second* cylinder, so subsurface makes sure that it
doesn't lose any information, and renumbers the cylinder indexes as it
merges the dive data. So while your Stinger *used* to start with
cylinder 0, it now starts with cylinder 1. And that means that
subsurface has to add an event saying so. That event is obviously the
gas change event.

See what happened?

So no, we do *not* generate gas change event when diving on air, and
your computers never generated any at all, it's subsurface that just
reminds itself "this dive computer is associated with cylinder 1, not
cylinder 0". And not starting on cylinder 0 is considered a gas
change.

In other words, "gas change" is a bit of a misnomer. It's a "cylinder
index change". It's just that we call it gas change, because that's
what it means in normal situations.

Anyway, what we probably *should* do (after the two bugs above get
sorted out) is likely to say that when you delete a cylinder, you
delete all gas change events associated with it too. That would give
us a way to fix up the above issue by the user just saying "I really
only had the first cylinder, so delete that bogus second cylinder",
and everything would work out right. But right now, deleting that
second cylinder has bigger problems than "the gas change event
remains".

                   Linus