[PATCH 2/2] Add type for gaschange events, if missing

Miika Turkia miika.turkia at gmail.com
Sun Jul 20 21:54:48 PDT 2014


On Thu, Jul 17, 2014 at 7:45 PM, Anton Lundin <glance at acc.umu.se> wrote:

> On 17 July, 2014 - Miika Turkia wrote:
>
> > On Thu, Jul 17, 2014 at 10:05 AM, Anton Lundin <glance at acc.umu.se>
> wrote:
> >
> > > On 12 July, 2014 - Miika Turkia wrote:
> > >
> > > > On Sat, Jul 12, 2014 at 3:21 PM, Anton Lundin <glance at acc.umu.se>
> wrote:
> > > >
> > > > > On 12 July, 2014 - Miika Turkia wrote:
> > > > >
> > > > > > On Sat, Jul 12, 2014 at 2:24 PM, Anton Lundin <glance at acc.umu.se
> >
> > > wrote:
> > > > > >
> > > > > > > On 12 July, 2014 - Miika Turkia wrote:
> > > > > > >
> > > > > > > > Subsurface has saved gas change events without type
> attribute at
> > > some
> > > > > > > > point. Thus we need to add the type when reading in log
> files,
> > > if it
> > > > > is
> > > > > > > > missing. (Gas change logic relies on the type field
> nowadays.)
> > > > > > > >
> > > > > > > > Fixes #617
> > > > > > > > Fixes #600
> > > > > > > >
> > > > > > > > Signed-off-by: Miika Turkia <miika.turkia at gmail.com>
> > > > > > > > ---
> > > > > > > >  parse-xml.c | 6 ++++++
> > > > > > > >  1 file changed, 6 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/parse-xml.c b/parse-xml.c
> > > > > > > > index 5375e32..606e251 100644
> > > > > > > > --- a/parse-xml.c
> > > > > > > > +++ b/parse-xml.c
> > > > > > > > @@ -1337,6 +1337,12 @@ static void event_end(void)
> > > > > > > >                               pic->offset.seconds =
> > > > > > > cur_event.time.seconds;
> > > > > > > >                               dive_add_picture(cur_dive,
> pic);
> > > > > > > >                       } else {
> > > > > > > > +                             /* At some point gas change
> events
> > > did
> > > > > not
> > > > > > > have any type. Thus we need to add
> > > > > > > > +                              * one on import, if we
> encounter
> > > the
> > > > > type
> > > > > > > one missing.
> > > > > > > > +                              */
> > > > > > > > +                             if (cur_event.type == 0 &&
> strcmp(
> > > > > > > cur_event.name, "gaschange") == 0)
> > > > > > > > +                                     cur_event.type = 25;
> > > > > > >
> > > > > > > I would prefer if we used SAMPLE_EVENT_GASCHANGE2 instead of
> the
> > > enum
> > > > > > > number.
> > > > > > >
> > > > > > > Are you sure they are of type GASCHANGE2, aka with He? I would
> > > guess
> > > > > > > they where of type GASCHANGE , aka the ones without any He
> info.
> > > > > > >
> > > > > >
> > > > > > All the test dives use type 25 even though there is no helium in
> > > them and
> > > > > > manually added gas change event uses hard coded 25 as well, so I
> > > didn't
> > > > > > realize it should change based on gas. I'll send a fix shortly...
> > > > > >
> > > > >
> > > > > It depends on what format the source of the event was talking.
> There
> > > are
> > > > > DC's that only tell us the o2 content, even if the gas had a
> he-part.
> > > > >
> > > > > If you look at the get_cylinder_index function, if its a
> > > > > SAMPLE_EVENT_GASCHANGE, just don't care about any he-parts when
> were
> > > > > looking for the matching gas, eg:
> > > > >
> > > > > If a dive had a gaslist like:
> > > > > * 21/35
> > > > > * AIR
> > > > >
> > > > > Then a SAMPLE_EVENT_GASCHANGE with o2 = 21% would return the
> 21/35, its
> > > > > the fist best match, but a SAMPLE_EVENT_GASCHANGE2 with o2 = 21%
> he =
> > > 0%
> > > > > would return the air because the 21/35 has helium in it and the
> event
> > > > > says that the gas shouldn't have any helium in it.
> > > > >
> > > > >
> > > > > I would say in the unknown event type case, if the event data has
> a he
> > > > > part then its definitely a SAMPLE_EVENT_GASCHANGE2, otherwise i
> would
> > > > > guess at a SAMPLE_EVENT_GASCHANGE, and let get_cylinder_index
> figure
> > > the
> > > > > rest out.
> > > > >
> > > >
> > > > This patch should take care of adding proper type attribute for both
> XML
> > > > import and manually added gas change events.
> > > >
> > > > miika
> > >
> > > > From e60b73cf33f79f144fc15705ac4f11645fed68e9 Mon Sep 17 00:00:00
> 2001
> > > > From: Miika Turkia <miika.turkia at gmail.com>
> > > > Date: Sat, 12 Jul 2014 15:51:03 +0300
> > > > Subject: [PATCH] Detect proper event type based on helium content
> > > >
> > > > Select proper SAMPLE_EVENT_GASCHANGE "version" based on helium
> content
> > > > on the mix.
> > > >
> > > > Signed-off-by: Miika Turkia <miika.turkia at gmail.com>
> > > > ---
> > > >  parse-xml.c | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/parse-xml.c b/parse-xml.c
> > > > index 606e251..20eafc3 100644
> > > > --- a/parse-xml.c
> > > > +++ b/parse-xml.c
> > > > @@ -11,6 +11,7 @@
> > > >  #include <libxml/parserInternals.h>
> > > >  #include <libxml/tree.h>
> > > >  #include <libxslt/transform.h>
> > > > +#include <libdivecomputer/parser.h>
> > > >
> > > >  #include "gettext.h"
> > > >
> > > > @@ -712,7 +713,7 @@ void add_gas_switch_event(struct dive *dive,
> struct
> > > divecomputer *dc, int second
> > > >       he = (he + 5) / 10;
> > > >       value = o2 + (he << 16);
> > > >
> > > > -     add_event(dc, seconds, 25, 0, value, "gaschange"); /*
> > > SAMPLE_EVENT_GASCHANGE2 */
> > > > +     add_event(dc, seconds, he ? SAMPLE_EVENT_GASCHANGE2 :
> > > SAMPLE_EVENT_GASCHANGE, 0, value, "gaschange");
> > > >  }
> > > >
> > > >  static void get_cylinderindex(char *buffer, uint8_t *i)
> > > > @@ -1341,7 +1342,7 @@ static void event_end(void)
> > > >                                * one on import, if we encounter the
> type
> > > one missing.
> > > >                                */
> > > >                               if (cur_event.type == 0 && strcmp(
> > > cur_event.name, "gaschange") == 0)
> > > > -                                     cur_event.type = 25;
> > > > +                                     cur_event.type =
> cur_event.value
> > > >> 16 > 0 ? SAMPLE_EVENT_GASCHANGE2 : SAMPLE_EVENT_GASCHANGE;
> > > >
> > > >                               add_event(dc, cur_event.time.seconds,
> > > >                                         cur_event.type,
> cur_event.flags,
> > >
> > > I would say that this code is wrong.
> > >
> > > You can't choose between SAMPLE_EVENT_GASCHANGE and
> > > SAMPLE_EVENT_GASCHANGE2 just based on if there is helium in the gas or
> > > not.
> > >
> > > You should use SAMPLE_EVENT_GASCHANGE if the gaschange event only
> > > contains the oxygen part of a gasmix, and SAMPLE_EVENT_GASCHANGE2 if
> the
> > > event describes the whole gasmix.
> > >
> >
> > So in Subsurface we should always use SAMPLE_EVENT_GASCHANGE2? Unless
> > libdivecomputer tells us otherwise? If I am finally understanding you...
> >
>
> I hope i didn't mess things up too badly for you.
>
> Yes, if we added the event, we should always use
> SAMPLE_EVENT_GASCHANGE2, but we can't assume that all events we read
> without a event type will be SAMPLE_EVENT_GASCHANGE2.


Well, this has been surprisingly difficult concept to grasp, even though it
is actually quite simple.

Here is a patch that switches to  SAMPLE_EVENT_GASCHANGE2 for the manually
added gas change events, as then we do know the full mix contents. But for
input from saved files we still need to do our best guess. I suppose we
need an Ack from Anton for this one even though I think I have understood
the concept after the lengthy correspondence...

miika
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20140721/35af7ce0/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Use-SAMPLE_EVENT_GASCHANGE2-for-manually-added-chang.patch
Type: text/x-patch
Size: 1071 bytes
Desc: not available
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20140721/35af7ce0/attachment-0001.bin>


More information about the subsurface mailing list