First step in cleaning up cylinder pressure sensor logic

Linus Torvalds torvalds at linux-foundation.org
Sun Dec 30 20:00:51 PST 2012


This clarifies/changes the meaning of our "cylinderindex" entry in our 
samples. It has been rather confused, because different dive computers 
have done things differently, and the naming really hasn't helped.

There are two totally different - and independent - cylinder "indexes":

 - the pressure sensor index, which indicates which cylinder the sensor 
   data is from.

 - the "active cylinder" index, which indicates which cylinder we actually 
   breathe from.

These two values really are totally independent, and have nothing 
what-so-ever to do with each other. The sensor index may well be fixed: 
many dive computers only support a single pressure sensor (whether 
wireless or wired), and the sensor index is thus always zero.

Other dive computers may support multiple pressure sensors, and the gas 
switch event may - or may not - indicate that the sensor changed too. A 
dive computer might give the sensor data for *all* cylinders it can read, 
regardless of which one is the one we're actively breathing. In fact, some 
dive computers might give sensor data for not just *your* cylinder, but 
your buddies.

This patch renames "cylinderindex" in the samples as "sensor", making it 
quite clear that it's about which sensor index the pressure data in the 
sample is about. 

The way we figure out which is the currently active gas is with an 
explicit has change event. If a computer (like the Uemis Zurich) joins the 
two concepts together, then a sensor change should also create a gas 
switch event. This patch also changes the Uemis importer to do that.

Finally, it should be noted that the plot info works totally separately 
from the sample data, and is about what we actually *display*, not about 
the sample pressures etc. In the plot info, the "cylinderindex" does in 
fact mean the currently active cylinder, and while it is initially set to 
match the sensor information from the samples, we then walk the gas change 
events and fix it up - and if the active cylinder differs from the sensor 
cylinder, we clear the sensor data.

Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
---

On Sun, 30 Dec 2012, Dirk Hohndel wrote:
> 
> I think it's a good step in the right direction.
> 
> 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. 

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.

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.

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?

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.

> 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.

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

       Linus

---
 dive.c            | 14 ++++++++------
 dive.h            |  3 ++-
 divelist.c        | 22 +++++++++++++++++++---
 libdivecomputer.c |  2 +-
 parse-xml.c       | 40 +++++++++++++++++++++++++++++++++++++++-
 profile.c         |  3 ++-
 save-xml.c        | 44 ++++++++++++++++++++++++++++++++------------
 uemis.c           |  7 ++++++-
 8 files changed, 109 insertions(+), 26 deletions(-)

diff --git a/dive.c b/dive.c
index d49596532ece..62a698b85283 100644
--- a/dive.c
+++ b/dive.c
@@ -237,7 +237,9 @@ static void fixup_pressure(struct dive *dive, struct sample *sample)
 	pressure = sample->cylinderpressure.mbar;
 	if (!pressure)
 		return;
-	index = sample->cylinderindex;
+	index = sample->sensor;
+
+	/* FIXME! sensor -> cylinder mapping? */
 	if (index >= MAX_CYLINDERS)
 		return;
 	cyl = dive->cylinder + index;
@@ -427,7 +429,7 @@ struct dive *fixup_dive(struct dive *dive)
 		int depth = sample->depth.mm;
 		int temp = sample->temperature.mkelvin;
 		int pressure = sample->cylinderpressure.mbar;
-		int index = sample->cylinderindex;
+		int index = sample->sensor;
 
 		if (index == lastindex) {
 			/* Remove duplicate redundant pressure information */
@@ -502,7 +504,7 @@ struct dive *fixup_dive(struct dive *dive)
 		if (abs(pressure_delta[j]) != INT_MAX) {
 			cylinder_t *cyl = dive->cylinder + j;
 			for (i = 0; i < dc->samples; i++)
-				if (dc->sample[i].cylinderindex == j)
+				if (dc->sample[i].sensor == j)
 					dc->sample[i].cylinderpressure.mbar = 0;
 			if (! cyl->start.mbar)
 				cyl->start.mbar = cyl->sample_start.mbar;
@@ -709,8 +711,8 @@ add_sample_b:
 			sample.temperature = as->temperature;
 		if (as->cylinderpressure.mbar)
 			sample.cylinderpressure = as->cylinderpressure;
-		if (as->cylinderindex)
-			sample.cylinderindex = as->cylinderindex;
+		if (as->sensor)
+			sample.sensor = as->sensor;
 		if (as->cns)
 			sample.cns = as->cns;
 		if (as->po2)
@@ -1289,7 +1291,7 @@ static int same_sample(struct sample *a, struct sample *b)
 		return 0;
 	if (a->cylinderpressure.mbar != b->cylinderpressure.mbar)
 		return 0;
-	return a->cylinderindex == b->cylinderindex;
+	return a->sensor == b->sensor;
 }
 
 static int same_dc(struct divecomputer *a, struct divecomputer *b)
diff --git a/dive.h b/dive.h
index 355ec9ed774c..61b02e2da5ad 100644
--- a/dive.h
+++ b/dive.h
@@ -226,7 +226,7 @@ struct sample {
 	depth_t depth;
 	temperature_t temperature;
 	pressure_t cylinderpressure;
-	int cylinderindex;
+	int sensor;		/* Cylinder pressure sensor index */
 	duration_t ndl;
 	duration_t stoptime;
 	depth_t stopdepth;
@@ -502,6 +502,7 @@ extern struct dive *try_to_merge(struct dive *a, struct dive *b, gboolean prefer
 
 extern void renumber_dives(int nr);
 
+extern void add_gas_switch_event(struct dive *dive, struct divecomputer *dc, int time, int idx);
 extern void add_event(struct divecomputer *dc, int time, int type, int flags, int value, const char *name);
 
 /* UI related protopypes */
diff --git a/divelist.c b/divelist.c
index 316c4d547a07..2a6829e27d97 100644
--- a/divelist.c
+++ b/divelist.c
@@ -715,6 +715,24 @@ static void cns_data_func(GtkTreeViewColumn *col,
 	g_object_set(renderer, "text", buffer, NULL);
 }
 
+static int active_o2(struct dive *dive, struct divecomputer *dc, duration_t time)
+{
+	int o2permille = dive->cylinder[0].gasmix.o2.permille;
+	struct event *event = dc->events;
+
+	if (!o2permille)
+		o2permille = AIR_PERMILLE;
+
+	for (event = dc->events; event; event = event->next) {
+		if (event->time.seconds > time.seconds)
+			break;
+		if (strcmp(event->name, "gaschange"))
+			continue;
+		o2permille = 10*(event->value & 0xffff);
+	}
+	return o2permille;
+}
+
 /* calculate OTU for a dive */
 static int calculate_otu(struct dive *dive, struct divecomputer *dc)
 {
@@ -730,9 +748,7 @@ static int calculate_otu(struct dive *dive, struct divecomputer *dc)
 		if (sample->po2) {
 			po2 = sample->po2;
 		} else {
-			int o2 = dive->cylinder[sample->cylinderindex].gasmix.o2.permille;
-			if (!o2)
-				o2 = AIR_PERMILLE;
+			int o2 = active_o2(dive, dc, sample->time);
 			po2 = o2 / 1000.0 * depth_to_mbar(sample->depth.mm, dive) / 1000.0;
 		}
 		if (po2 >= 0.5)
diff --git a/libdivecomputer.c b/libdivecomputer.c
index 7b3087f9df51..9b042f325990 100644
--- a/libdivecomputer.c
+++ b/libdivecomputer.c
@@ -228,7 +228,7 @@ sample_cb(dc_sample_type_t type, dc_sample_value_t value, void *userdata)
 		sample->depth.mm = value.depth * 1000 + 0.5;
 		break;
 	case DC_SAMPLE_PRESSURE:
-		sample->cylinderindex = value.pressure.tank;
+		sample->sensor = value.pressure.tank;
 		sample->cylinderpressure.mbar = value.pressure.value * 1000 + 0.5;
 		break;
 	case DC_SAMPLE_TEMPERATURE:
diff --git a/parse-xml.c b/parse-xml.c
index aa9d5e0f57ca..077984a6f91e 100644
--- a/parse-xml.c
+++ b/parse-xml.c
@@ -155,6 +155,7 @@ static gboolean in_settings = FALSE;
 static struct tm cur_tm;
 static int cur_cylinder_index, cur_ws_index;
 static int lastndl, laststoptime, laststopdepth, lastcns, lastpo2;
+static int lastcylinderindex, lastsensor;
 
 static enum import_source {
 	UNKNOWN,
@@ -650,6 +651,39 @@ static void try_to_fill_dc(struct divecomputer *dc, const char *name, char *buf)
 	nonmatch("divecomputer", name, buf);
 }
 
+void add_gas_switch_event(struct dive *dive, struct divecomputer *dc, int seconds, int idx)
+{
+	/* The gas switch event format is insane. It will be fixed, I think */
+	int o2 = dive->cylinder[idx].gasmix.o2.permille;
+	int he = dive->cylinder[idx].gasmix.he.permille;
+	int value;
+
+	if (!o2)
+		o2 = AIR_PERMILLE;
+	o2 = (o2+5) / 10;
+	he = (he+5) / 10;
+	value = o2 + (he << 16);
+
+	add_event(dc, seconds, 11, 0, value, "gaschange");
+}
+
+static void get_cylinderindex(char *buffer, void *_i)
+{
+	int *i = _i;
+	*i = atoi(buffer);
+	if (lastcylinderindex != *i) {
+		add_gas_switch_event(cur_dive, cur_dc, cur_sample->time.seconds, *i);
+		lastcylinderindex = *i;
+	}
+}
+
+static void get_sensor(char *buffer, void *_i)
+{
+	int *i = _i;
+	*i = atoi(buffer);
+	lastsensor = *i;
+}
+
 /* We're in samples - try to convert the random xml value to something useful */
 static void try_to_fill_sample(struct sample *sample, const char *name, char *buf)
 {
@@ -660,7 +694,9 @@ static void try_to_fill_sample(struct sample *sample, const char *name, char *bu
 		return;
 	if (MATCH(".sample.cylpress", pressure, &sample->cylinderpressure))
 		return;
-	if (MATCH(".sample.cylinderindex", get_index, &sample->cylinderindex))
+	if (MATCH(".sample.cylinderindex", get_cylinderindex, &sample->sensor))
+		return;
+	if (MATCH(".sample.sensor", get_sensor, &sample->sensor))
 		return;
 	if (MATCH(".sample.depth", depth, &sample->depth))
 		return;
@@ -1000,6 +1036,7 @@ static gboolean is_dive(void)
 static void reset_dc_info(struct divecomputer *dc)
 {
 	lastcns = lastpo2 = lastndl = laststoptime = laststopdepth = 0;
+	lastsensor = lastcylinderindex = 0;
 }
 
 static void reset_dc_settings(void)
@@ -1123,6 +1160,7 @@ static void sample_start(void)
 	cur_sample->stopdepth.mm = laststopdepth;
 	cur_sample->cns = lastcns;
 	cur_sample->po2 = lastpo2;
+	cur_sample->sensor = lastsensor;
 }
 
 static void sample_end(void)
diff --git a/profile.c b/profile.c
index bd78c85dadb1..45249f26de1f 100644
--- a/profile.c
+++ b/profile.c
@@ -1638,7 +1638,8 @@ static struct plot_info *create_plot_info(struct dive *dive, struct divecomputer
 		entry->ndl = ndl;
 		entry->cns = cns;
 		entry->po2 = po2;
-		entry->cylinderindex = sample->cylinderindex;
+		/* FIXME! sensor index -> cylinder index translation! */
+		entry->cylinderindex = sample->sensor;
 		SENSOR_PRESSURE(entry) = sample->cylinderpressure.mbar;
 		entry->temperature = sample->temperature.mkelvin;
 
diff --git a/save-xml.c b/save-xml.c
index 9736a4f356c9..ee71582f916f 100644
--- a/save-xml.c
+++ b/save-xml.c
@@ -319,25 +319,47 @@ static void show_index(FILE *f, int value, const char *pre, const char *post)
 		fprintf(f, " %s%d%s", pre, value, post);
 }
 
-static void save_sample(FILE *f, struct sample *sample, const struct sample *prev)
+static void save_sample(FILE *f, struct sample *sample, struct sample *old)
 {
 	fprintf(f, "  <sample time='%u:%02u min'", FRACTION(sample->time.seconds,60));
 	show_milli(f, " depth='", sample->depth.mm, " m", "'");
 	show_temperature(f, sample->temperature, " temp='", "'");
 	show_pressure(f, sample->cylinderpressure, " pressure='", "'");
-	if (sample->cylinderindex)
-		fprintf(f, " cylinderindex='%d'", sample->cylinderindex);
+
+	/*
+	 * We only show sensor information for samples with pressure, and only if it
+	 * changed from the previous sensor we showed.
+	 */
+	if (sample->cylinderpressure.mbar && sample->sensor != old->sensor) {
+		fprintf(f, " sensor='%d'", sample->sensor);
+		old->sensor = sample->sensor;
+	}
+
 	/* the deco/ndl values are stored whenever they change */
-	if (sample->ndl.seconds != prev->ndl.seconds)
+	if (sample->ndl.seconds != old->ndl.seconds) {
 		fprintf(f, " ndl='%u:%02u min'", FRACTION(sample->ndl.seconds, 60));
-	if (sample->stoptime.seconds != prev->stoptime.seconds)
+		old->ndl = sample->ndl;
+	}
+
+	if (sample->stoptime.seconds != old->stoptime.seconds) {
 		fprintf(f, " stoptime='%u:%02u min'", FRACTION(sample->stoptime.seconds, 60));
-	if (sample->stopdepth.mm != prev->stopdepth.mm)
+		old->stoptime = sample->stoptime;
+	}
+
+	if (sample->stopdepth.mm != old->stopdepth.mm) {
 		show_milli(f, " stopdepth='", sample->stopdepth.mm, " m", "'");
-	if (sample->cns != prev->cns)
+		old->stopdepth = sample->stopdepth;
+	}
+
+	if (sample->cns != old->cns) {
 		fprintf(f, " cns='%u%%'", sample->cns);
-	if (sample->po2 != prev->po2)
+		old->cns = sample->cns;
+	}
+
+	if (sample->po2 != old->po2) {
 		fprintf(f, " po2='%u.%2u bar'", FRACTION(sample->po2, 1000));
+		old->po2 = sample->po2;
+	}
 	fprintf(f, " />\n");
 }
 
@@ -374,12 +396,10 @@ static void show_date(FILE *f, timestamp_t when)
 
 static void save_samples(FILE *f, int nr, struct sample *s)
 {
-	static const struct sample empty_sample;
-	const struct sample *prev = &empty_sample;
+	struct sample dummy = { };
 
 	while (--nr >= 0) {
-		save_sample(f, s, prev);
-		prev = s;
+		save_sample(f, s, &dummy);
 		s++;
 	}
 }
diff --git a/uemis.c b/uemis.c
index 8d05a99f98c7..bc9d05ffe498 100644
--- a/uemis.c
+++ b/uemis.c
@@ -288,6 +288,7 @@ void uemis_parse_divelog_binary(char *base64, void *datap) {
 	struct dive *dive = datap;
 	struct divecomputer *dc = &dive->dc;
 	int template, gasoffset;
+	int active = 0;
 
 	datalen = uemis_convert_base64(base64, &data);
 
@@ -342,11 +343,15 @@ void uemis_parse_divelog_binary(char *base64, void *datap) {
 		 * duration in the header is a) in minutes and b) up to 3 minutes short */
 		if (u_sample->dive_time > dive->duration.seconds + 180)
 			break;
+		if (u_sample->active_tank != active) {
+			active = u_sample->active_tank;
+			add_gas_switch_event(dive, dc, u_sample->dive_time, active);
+		}
 		sample = prepare_sample(dc);
 		sample->time.seconds = u_sample->dive_time;
 		sample->depth.mm = rel_mbar_to_depth(u_sample->water_pressure, dive);
 		sample->temperature.mkelvin = (u_sample->dive_temperature * 100) + 273150;
-		sample->cylinderindex = u_sample->active_tank;
+		sample->sensor = active;
 		sample->cylinderpressure.mbar =
 			(u_sample->tank_pressure_high * 256 + u_sample->tank_pressure_low) * 10;
 		sample->cns = u_sample->cns;


More information about the subsurface mailing list