On 12 July, 2014 - Miika Turkia wrote: > On Sat, Jul 12, 2014 at 3:21 PM, Anton Lundin <[email protected]> wrote: > > > On 12 July, 2014 - Miika Turkia wrote: > > > > > On Sat, Jul 12, 2014 at 2:24 PM, Anton Lundin <[email protected]> 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 <[email protected]> > > > > > --- > > > > > 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 <[email protected]> > 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 <[email protected]> > --- > 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. Eg. a <event time='53:20 min' type='11' value='21' name='gaschange' /> Can't quantify of its a switch to 21/35 or air. A: <event time='53:20 min' type='25' value='21' name='gaschange' /> Is most definitely a switch to air. Yea, the event handling is a mess. //Anton -- Anton Lundin +46702-161604 _______________________________________________ subsurface mailing list [email protected] http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface
