[PATCH 2/2] Fix per-cylinder SAC rate calculations when cylinder use isn't known

Linus Torvalds torvalds at linux-foundation.org
Tue Apr 5 16:02:41 PDT 2016


From: Linus Torvalds <torvalds at linux-foundation.org>
Date: Tue, 5 Apr 2016 15:53:02 -0700
Subject: [PATCH 2/2] Fix per-cylinder SAC rate calculations when cylinder use isn't known

John Van Ostrand reports that when he dives using two cylinders using
sidemounts, the per-cylinder SAC rate display is very misleading.

What happens is that since the two cylinders are used together (but
without a manifold), John is alternating between the two but not
actually adding gas switches in the profile.  As a result, the profile
looks like only one cylinder is used, even though clearly the other
cylinder gets breathed down too.

The per-cylinder SAC rate calculations would entirely ignore the
cylinder that didn't have gas switch events to it, and looking at the
info window it would look like John had a truly exceptional SAC rate.

But then in the general statistics panel that actually takes the whole
gas use into account, the very different real SAC rate would show up.

The basic issue is that if we don't have full use information for the
different cylinders, we would account the whole dive to just a partial
set.  We did have a special case for this, but that special case only
really worked if the first cylinder truly was the only cylinder used.

This patch makes us see the difference between "only one cylinder was
used, and I can use the overall mean depth for it" and "more than one
cylinder was used, but I don't know what the mean depths might be".

Reported-by: John Van Ostrand <john at vanostrand.com>
Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
---

This is the patch that fixes John's case. 

So my fix ends up just being to not show the per-cylinder SAC rates at all 
when they cannot be computed. So you'll see empty SAC rates per cylinder 
in the info tab, but in the statistics tab you'll see the correct overall 
SAC-rate.

So at least we won't be showing bogus and misleading information. I 
waffled about showing the overall SAC-rate for all cylinders, but that has 
its own misleading issues too, so let's not make up any numbers.

It would be good to have this double-checked, but in  my testing it works 
for John's test-case, and also for some of my multi-cylinder dives that do 
have full gas switch information. 

 core/dive.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 84 insertions(+), 13 deletions(-)

diff --git a/core/dive.c b/core/dive.c
index 1b3af66f364b..c2ea11a0d47e 100644
--- a/core/dive.c
+++ b/core/dive.c
@@ -721,6 +721,58 @@ void fixup_dc_duration(struct divecomputer *dc)
 	}
 }
 
+/* Which cylinders had gas used? */
+#define SOME_GAS 5000
+static unsigned int get_cylinder_used(struct dive *dive)
+{
+	int i;
+	unsigned int mask = 0;
+
+	for (i = 0; i < MAX_CYLINDERS; i++) {
+		cylinder_t *cyl = dive->cylinder + i;
+		int start_mbar, end_mbar;
+
+		if (cylinder_nodata(cyl))
+			continue;
+		start_mbar = cyl->start.mbar ?: cyl->sample_start.mbar;
+		end_mbar = cyl->end.mbar ?: cyl->sample_end.mbar;
+
+		// More than 5 bar used? This matches statistics.c
+		// heuristics
+		if (start_mbar > end_mbar + SOME_GAS)
+			mask |= 1 << i;
+	}
+	return mask;
+}
+
+/* Which cylinders do we know usage about? */
+static unsigned int get_cylinder_known(struct dive *dive, struct divecomputer *dc)
+{
+	unsigned int mask = 0;
+	struct event *ev;
+
+	/* We know about using the O2 cylinder in a CCR dive */
+	if (dc->divemode == CCR) {
+		int o2_cyl = get_cylinder_idx_by_use(dive, OXYGEN);
+		if (o2_cyl >= 0)
+			mask |= 1 << o2_cyl;
+	}
+
+	/* We know about the explicit first cylinder (or first) */
+	mask |= 1 << explicit_first_cylinder(dive, dc);
+
+	/* And we have possible switches to other gases */
+	ev = get_next_event(dc->events, "gaschange");
+	while (ev) {
+		int i = get_cylinder_index(dive, ev);
+		if (i >= 0)
+			mask |= 1 << i;
+		ev = get_next_event(ev->next, "gaschange");
+	}
+
+	return mask;
+}
+
 void per_cylinder_mean_depth(struct dive *dive, struct divecomputer *dc, int *mean, int *duration)
 {
 	int i;
@@ -728,29 +780,48 @@ void per_cylinder_mean_depth(struct dive *dive, struct divecomputer *dc, int *me
 	uint32_t lasttime = 0;
 	int lastdepth = 0;
 	int idx = 0;
+	unsigned int used_mask, known_mask;
 
 	for (i = 0; i < MAX_CYLINDERS; i++)
 		mean[i] = duration[i] = 0;
 	if (!dc)
 		return;
-	struct event *ev = get_next_event(dc->events, "gaschange");
-	if (!ev || (dc && dc->sample && ev->time.seconds == dc->sample[0].time.seconds && get_next_event(ev->next, "gaschange") == NULL)) {
-		// we have either no gas change or only one gas change and that's setting an explicit first cylinder
-		mean[explicit_first_cylinder(dive, dc)] = dc->meandepth.mm;
-		duration[explicit_first_cylinder(dive, dc)] = dc->duration.seconds;
-
-		if (dc->divemode == CCR) {
-			// Do the same for the  O2 cylinder
-			int o2_cyl = get_cylinder_idx_by_use(dive, OXYGEN);
-			if (o2_cyl < 0)
-				return;
-			mean[o2_cyl] = dc->meandepth.mm;
-			duration[o2_cyl] = dc->duration.seconds;
+
+	/*
+	 * There is no point in doing per-cylinder information
+	 * if we don't actually know about the usage of all the
+	 * used cylinders.
+	 */
+	used_mask = get_cylinder_used(dive);
+	known_mask = get_cylinder_known(dive, dc);
+	if (used_mask & ~known_mask) {
+		/*
+		 * If we had more than one used cylinder, but
+		 * do not know usage of them, we simply cannot
+		 * account mean depth to them.
+		 *
+		 * The "x & (x-1)" test shows if it's not a pure
+		 * power of two.
+		 */
+		if (used_mask & (used_mask-1))
+			return;
+
+		/*
+		 * For a single cylinder, use the overall mean
+		 * and duration
+		 */
+		for (i = 0; i < MAX_CYLINDERS; i++) {
+			if (used_mask & (1 << i)) {
+				mean[i] = dc->meandepth.mm;
+				duration[i] = dc->duration.seconds;
+			}
 		}
+
 		return;
 	}
 	if (!dc->samples)
 		dc = fake_dc(dc, false);
+	struct event *ev = get_next_event(dc->events, "gaschange");
 	for (i = 0; i < dc->samples; i++) {
 		struct sample *sample = dc->sample + i;
 		uint32_t time = sample->time.seconds;
-- 
2.8.0.rc4.303.g3931579



More information about the subsurface mailing list