Few bugs and a feature req from testing Subsurface

Tomaz Canabrava tcanabrava at kde.org
Sat Mar 15 11:43:00 PDT 2014


Em 15/03/2014 14:46, "Linus Torvalds" <torvalds at linux-foundation.org>
escreveu:
>
> 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.

Linus, I'd love too. But this days I'm in bed with 40c fever ( got dengue
flu.) without the computer.

If anybody can help here on this bug I appreciate. The offending code is on
models. Cpp

beginRemoveRows(QModelIndex(), index.row(), index.row());
rows--;
remove_cylinder(current, index.row());
changed = true;
endRemoveRows();

I fail to see the bug there unless the remove-cylinder function call does
pointer magic.

Got to get some rest, fever raising up again.
Best regards,
Tomaz

> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20140315/45c53bd6/attachment.html>


More information about the subsurface mailing list