On Sun, Jun 01, 2014 at 12:05:44PM -0700, Dirk Hohndel wrote: > > Sadly, type cleanups tend to be *really* painful, because the type > > percolates all the way, and we have some helper functions that > > explciitly don't take the typed structures because they end up being > > used by code that doesn't _have_ the typed structures any more.. > > > > I can try to look at some of it. > > I hate saying this to any contributor... but wait a little bit and maybe > this will become easier.
OK, I should have just pushed this instead of sending the email first. This is step 0 of this painful cleanup; next step is to keep things as gasmix as long as possible. /D commit 1a040134538b7733f3088ea34f101cfedecc2c64 Author: Dirk Hohndel <[email protected]> Date: Sun Jun 1 12:07:29 2014 -0700 Encapsulate the horrid gas encoding in gas change events We should never pass permille values around as integers. And we shouldn't have to decode the stupid value in more than one place. This doesn't tackle all the places where we access O2 and He "too early" and should instead keep passing around a gaxmix. But it's a first step. Signed-off-by: Dirk Hohndel <[email protected]> diff --git a/dive.c b/dive.c index 0d8f4875459b..6545fecabf39 100644 --- a/dive.c +++ b/dive.c @@ -6,6 +6,7 @@ #include <limits.h> #include "gettext.h" #include "dive.h" +#include "libdivecomputer.h" struct tag_entry *g_tag_list = NULL; @@ -76,6 +77,19 @@ void remove_event(struct event* event) } } +/* this returns a pointer to static variable - so use it right away after calling */ +struct gasmix *get_gasmix_from_event(struct event *ev) +{ + static struct gasmix g; + g.o2.permille = g.he.permille = 0; + if (ev && (ev->type == SAMPLE_EVENT_GASCHANGE || ev->type == SAMPLE_EVENT_GASCHANGE2)) { + g.o2.permille = 10 * ev->value & 0xffff; + if (ev->type == SAMPLE_EVENT_GASCHANGE2) + g.he.permille = 10 * (ev->value >> 16); + } + return &g; +} + int get_pressure_units(int mb, const char **units) { int pressure; diff --git a/dive.h b/dive.h index 95de166a0652..a2d9c5425a83 100644 --- a/dive.h +++ b/dive.h @@ -69,6 +69,18 @@ typedef struct const char *description; /* "integrated", "belt", "ankle" */ } weightsystem_t; +/* + * Events are currently based straight on what libdivecomputer gives us. + * We need to wrap these into our own events at some point to remove some of the limitations. + */ +struct event { + struct event *next; + duration_t time; + int type, flags, value; + bool deleted; + char name[]; +}; + extern int get_pressure_units(int mb, const char **units); extern double get_depth_units(int mm, int *frac, const char **units); extern double get_volume_units(unsigned int ml, int *frac, const char **units); @@ -94,6 +106,7 @@ static inline int get_he(const struct gasmix *mix) extern void sanitize_gasmix(struct gasmix *mix); extern int gasmix_distance(const struct gasmix *a, const struct gasmix *b); +extern struct gasmix *get_gasmix_from_event(struct event *ev); static inline bool is_air(int o2, int he) { @@ -168,22 +181,6 @@ void taglist_init_global(); void taglist_free(struct tag_entry *tag_list); /* - * Events are currently pretty meaningless. This is - * just based on the random data that libdivecomputer - * gives us. I'm not sure what a real "architected" - * event model would actually look like, but right - * now you can associate a list of events with a dive, - * and we'll do something about it. - */ -struct event { - struct event *next; - duration_t time; - int type, flags, value; - bool deleted; - char name[]; -}; - -/* * NOTE! The deviceid and diveid are model-specific *hashes* of * whatever device identification that model may have. Different * dive computers will have different identifying data, it could diff --git a/divelist.c b/divelist.c index ca2e9827fe8d..421ea28c2a18 100644 --- a/divelist.c +++ b/divelist.c @@ -160,7 +160,7 @@ static int active_o2(struct dive *dive, struct divecomputer *dc, duration_t time break; if (strcmp(event->name, "gaschange")) continue; - o2permille = 10 * (event->value & 0xffff); + o2permille = get_o2(get_gasmix_from_event(event)); } return o2permille; } diff --git a/planner.c b/planner.c index 512034a2edee..41cc0baa1da7 100644 --- a/planner.c +++ b/planner.c @@ -78,8 +78,9 @@ void get_gas_from_events(struct divecomputer *dc, int time, int *o2, int *he) struct event *event = dc->events; while (event && event->time.seconds <= time) { if (!strcmp(event->name, "gaschange")) { - *o2 = 10 * event->value & 0xffff; - *he = 10 * event->value >> 16; + struct gasmix *g = get_gasmix_from_event(event); + *o2 = get_o2(g); + *he = get_he(g); } event = event->next; } diff --git a/profile.c b/profile.c index 35af28f25fa2..d3934e3089d6 100644 --- a/profile.c +++ b/profile.c @@ -578,13 +578,15 @@ int get_cylinder_index(struct dive *dive, struct event *ev) int i; int best = 0, score = INT_MAX; int target_o2, target_he; + struct gasmix *g; /* * Crazy gas change events give us odd encoded o2/he in percent. * Decode into our internal permille format. */ - target_o2 = (ev->value & 0xFFFF) * 10; - target_he = (ev->value >> 16) * 10; + g = get_gasmix_from_event(ev); + target_o2 = get_o2(g); + target_he = get_he(g); /* * Try to find a cylinder that best matches the target gas diff --git a/qt-ui/profile/diveeventitem.cpp b/qt-ui/profile/diveeventitem.cpp index 873c6abbe3ee..87b57bfab825 100644 --- a/qt-ui/profile/diveeventitem.cpp +++ b/qt-ui/profile/diveeventitem.cpp @@ -82,8 +82,9 @@ void DiveEventItem::setupToolTipString() int type = internalEvent->type; if (value) { if (type == SAMPLE_EVENT_GASCHANGE || type == SAMPLE_EVENT_GASCHANGE2) { - int he = value >> 16; - int o2 = value & 0xffff; + struct gasmix *g = get_gasmix_from_event(internalEvent); + int he = get_he(g); + int o2 = get_o2(g); name += ": "; if (he) _______________________________________________ subsurface mailing list [email protected] http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface
