First step in cleaning up cylinder pressure sensor logic

[Dirk Hohndel]

Linus Torvalds <torvalds at linux-foundation.org> writes:
>> But I'm not happy with the way we implement the gas change event today. The
>> code that inserts additional plot info entries is utter crap... I really want
>> to get rid of that.
>
> I actually think it's the right thing to do. I don't see why you dislike 
> the plot info entries. 

See your next paragraph:

> That said, I do think we do the extra entries wrongly for *another* 
> reason. I don't think the extra entries should have anything to do with 
> gas change events. In particular, I think the gas change events should 
> simply be used to change the cylinder index in the plot-info events.

The inserted fake entries are just wrong on so many levels. But they are
needed to create even remotely attractive plots with older dive
computers.

> Look at why we create those silly extra events: it's not because the gas 
> change event actually needs it. No, it's because we have dives that DO NOT 
> HAVE SAMPLES. So then we create just a few silly samples for the basic 
> dive outline, and the *only* reason we create more plot entries for the 
> gas change events is so that we can have gas changes in those long 
> stretches of time when we don't have any actual normal plot events.
>
> In fact, I think they all come from the test-cases we have.

It's not quite just for the test dives. It's for computers that have a
low sample rate and record a tank change between two samples. We ran
into this when we dove the Zenobia and I had the Gekko as backup
computer and still tried to make things look right. And even on your
Vyper Air it didn't look right.

> So I would actually suggest that we stop doing extra events for gas 
> changes, and instead just have some minimum number of plotinfo events 
> (say, we could have a minimum of one plot-info per ten seconds or 
> something). Then we just set the active cylinder for those events.
>
> Hmm?

That should sufficiently work around the issue we had.

> Now, I do have *other* complaints about the gas change event, but that has 
> nothing to do with the extra bogus plot-info ones. I detest how we get the
> O2/He information, and we do need to fix that ambiguous issue somehow. But 
> that's a totally separate issue.

Yes, the (O2 | H2 << 16) thing will go away :-)

>> But that is the next step after cleaning up the cylinderindex mess.
>
> So I have to say, I don't love my patch below, but I do think it's a step 
> in the right direction. I tried it out on my recent dives, and it imports 
> things correctly from the Uemis. In fact, the import seems to be *more* 
> correct than the old one, because the old one got really confused when it 
> saw the gas switch indexes, and those indexes didn't match the indexes of 
> the *first* dive computer that had gotten imported.
>
> With this change, because we turn the gas switch into that O2/He 
> percentage thing, at least it now encodes the relevant information (not 
> the "cylinder index" that might be different between two different dive 
> computers, but the "cylinder contents" which had better be at least 
> roughly sane, or the deco information your dive computer gives is 
> worthless).
>
> I dunno. You probably never saw this, but look at my dive log that has the 
> Uemis as the *last* dive computer imported. I get a pO2 of 3.0 on the 
> Uemis, because there's some confusion about the indexing of the 40%/80% 
> bottles (I think it's because I just did He/40/80 in that order, but the 
> Uemis actually thinks they are "travel/bottom/deco" gases, and so what I 
> though was 0/1/2 was in fact numbered 0/2/1 or something.

No, I never saw this - but I saw other crap because of the confusion in
the old code.

> Hmm? Comments? Does this work for your data too?

Huge improevment. Works for my data as well.

It caused some conflict because of recent changes I had already
committed (the deco/ndl handling didn't match the semantics in
libdivecomputer... and that's quite embarrassing as I implemented that
in libdivecomputer).

I think I merged this correctly, please double check.

/D