[PATCH] Interpolate depth for samples that have no depth

Linus Torvalds torvalds at linux-foundation.org
Sat Oct 24 20:22:58 PDT 2015


From: Linus Torvalds <torvalds at linux-foundation.org>
Date: Sun, 25 Oct 2015 12:02:08 +0900
Subject: [PATCH] Interpolate depth for samples that have no depth

When downloading from libdivecomputer, we used to initialize the depth
of a sample to the previous depth.  However, at least for the Suunto EON
Steel, you can get sample times without any actual depth reading - the
time might be associated with some ranbdom event rather than a new depth
sample.

Rather than initialize these samples to have the same depth as the
previous one (and then perhaps getting a very sudden jump when the
*real* depth event comes in a second later), initialize the depth
samples to -1, and if that sample doesn't get a real depth, we'll create
an interpolated depth.

It is possible that we should just carry the sample around as not
actually having a depth, and instead just interpolate in the plot_info
generation, but at least right now we have a ton of code that "knows"
that every sample has a depth.  Not the least of which is our own save
format.

So generating an interpolated depth seems the path of least resistance,
and at least makes the graph look correct - no odd staircase effect from
other events that happen in between depth samples.

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

This fixes my profile from the EON Steel, where I before had the 
occasional red "steep ascent" warning segments because we would generate 
what looked like a staircase profile when an event happened in between two 
depth events, and rather than interpolate it would take the previous 
sample depth, and then the next sample would look like a very sudden jump.

Using a negative depth to indicate "this sample has no depth" is a new 
situation for us, but it gets fixed up very early in the dive parsing 
stage, so no other code should hopefully ever see it. 

So the upside of interpolating the depth early is that this change is 
fairly localized. The negative depth only exists during the download, and 
as we fix up the dive we get rid of it and replace it with the 
interpolated depth and nobody else will ever see a depth of "-1 mm".

The downside is that we save that fake depth and use it as a real depth, 
even though it's "fake". Of course, we used to do that before too - it's 
just that it was another - and far worse - fake value.

But arguably we *could* carry the "this sample has no depth" around for 
much longer, and only interpolate when actually displaying it. That would 
more accurately reflect the real download, but it would also break a lot 
of existing code that just knows that all samples have a depth.

The upside of carrying the "we have no depth value" around in the samples 
seems to be very questionable, and the downside would be that we'd have 
to find every single place we use a sample depth and either not use it at 
all, or interpolate it. So I think this patch is the right approach. But 
if somebody wants to do a more full conversion, go right ahead..

And even though this patch does the interpolation pretty early, having 
another set of eyes verify that there really is nothing else that sees the 
negative depth might be a good idea.

 dive.c            | 22 ++++++++++++++++++++++
 libdivecomputer.c | 11 +++++++----
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/dive.c b/dive.c
index dfd5b495c1ee..2ae84ca1e56c 100644
--- a/dive.c
+++ b/dive.c
@@ -1243,6 +1243,23 @@ static void fixup_dc_events(struct divecomputer *dc)
 	}
 }
 
+static int interpolate_depth(struct divecomputer *dc, int idx, int lastdepth, int lasttime, int now)
+{
+	int i;
+	int nextdepth = lastdepth;
+	int nexttime = now;
+
+	for (i = idx+1; i < dc->samples; i++) {
+		struct sample *sample = dc->sample + i;
+		if (sample->depth.mm < 0)
+			continue;
+		nextdepth = sample->depth.mm;
+		nexttime = sample->time.seconds;
+		break;
+	}
+	return interpolate(lastdepth, nextdepth, now-lasttime, nexttime-lasttime);
+}
+
 static void fixup_dive_dc(struct dive *dive, struct divecomputer *dc)
 {
 	int i, j;
@@ -1276,6 +1293,11 @@ static void fixup_dive_dc(struct dive *dive, struct divecomputer *dc)
 		int o2_pressure = sample->o2cylinderpressure.mbar;
 		int index;
 
+		if (depth < 0) {
+			depth = interpolate_depth(dc, i, lastdepth, lasttime, time);
+			sample->depth.mm = depth;
+		}
+
 		/* if we have an explicit first cylinder */
 		if (sample->sensor == 0 && first_cylinder != 0)
 			sample->sensor = first_cylinder;
diff --git a/libdivecomputer.c b/libdivecomputer.c
index a9d90d837979..b5f90cb35acd 100644
--- a/libdivecomputer.c
+++ b/libdivecomputer.c
@@ -231,7 +231,6 @@ static void handle_event(struct divecomputer *dc, struct sample *sample, dc_samp
 void
 sample_cb(dc_sample_type_t type, dc_sample_value_t value, void *userdata)
 {
-	unsigned int mm;
 	static unsigned int nsensor = 0;
 	struct divecomputer *dc = userdata;
 	struct sample *sample;
@@ -252,7 +251,10 @@ sample_cb(dc_sample_type_t type, dc_sample_value_t value, void *userdata)
 	switch (type) {
 	case DC_SAMPLE_TIME:
 		nsensor = 0;
-		mm = 0;
+
+		// The previous sample gets some sticky values
+		// that may have been around from before, even
+		// if there was no new data
 		if (sample) {
 			sample->in_deco = in_deco;
 			sample->ndl.seconds = ndl;
@@ -260,11 +262,12 @@ sample_cb(dc_sample_type_t type, dc_sample_value_t value, void *userdata)
 			sample->stopdepth.mm = stopdepth;
 			sample->setpoint.mbar = po2;
 			sample->cns = cns;
-			mm = sample->depth.mm;
 		}
+		// Create a new sample.
+		// Mark depth as negative
 		sample = prepare_sample(dc);
 		sample->time.seconds = value.time;
-		sample->depth.mm = mm;
+		sample->depth.mm = -1;
 		finish_sample(dc);
 		break;
 	case DC_SAMPLE_DEPTH:
-- 
2.6.0



More information about the subsurface mailing list