On Thu, Jul 17, 2014 at 10:05 AM, Anton Lundin <[email protected]> wrote:
> 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. > So in Subsurface we should always use SAMPLE_EVENT_GASCHANGE2? Unless libdivecomputer tells us otherwise? If I am finally understanding you... miika
_______________________________________________ subsurface mailing list [email protected] http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface
