From: Linus Torvalds <[email protected]> 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 <[email protected]> Signed-off-by: Linus Torvalds <[email protected]> --- 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 _______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
