stuck in deco calculations

Sun Sep 20 12:12:03 PDT 2015

On Sat, Sep 19, 2015 at 10:46 PM, Dirk Hohndel <dirk at hohndel.org> wrote:
>
> A silly patch like this prevents the hang, but I really want to understand
> why we end up with two consecutive entries that have non-monotonous time
> stamps...

So we've done something slightly odd.

Your XML file has these samples:

  <sample time='33:15 min' depth='0.517 m' pressure='94.32 bar' />^M
  <sample time='33:30 min' depth='0.557 m' pressure='94.596 bar' />^M

and those are correct in the sample array:

(gdb) p dc->sample[132]
$20 = {time = {seconds = 1995}, stoptime = {seconds = 0}, ndl =
{seconds = 11940}, tts = {seconds = 0}, rbt = {seconds = 0}, depth =
{mm = 517}, stopdepth = {mm = 0}, temperature = {mkelvin = 0},
  cylinderpressure = {mbar = 94320}, o2cylinderpressure = {mbar = 0},
setpoint = {mbar = 0}, o2sensor = {{mbar = 0}, {mbar = 0}, {mbar =
0}}, bearing = {degrees = 0}, sensor = 0 '\000',
  cns = 0 '\000', heartbeat = 0 '\000', sac = {mliter = 0}, in_deco =
false, manually_entered = false}
(gdb) p dc->sample[133]
$21 = {time = {seconds = 2010}, stoptime = {seconds = 0}, ndl =
{seconds = 11940}, tts = {seconds = 0}, rbt = {seconds = 0}, depth =
{mm = 557}, stopdepth = {mm = 0}, temperature = {mkelvin = 0},
  cylinderpressure = {mbar = 94596}, o2cylinderpressure = {mbar = 0},
setpoint = {mbar = 0}, o2sensor = {{mbar = 0}, {mbar = 0}, {mbar =
0}}, bearing = {degrees = 0}, sensor = 0 '\000',
  cns = 0 '\000', heartbeat = 0 '\000', sac = {mliter = 0}, in_deco =
false, manually_entered = false}

but the plot-info (which expands the samples to have a maximum of 10s
intervals, and inserts events etc) has

  (gdb) p pi->entry[267]
  $32 = {in_deco = 0, cylinderindex = 0, sec = 1995, pressure =
{94320, 0}, o2cylinderpressure = {0, 0}, temperature = 291483, depth =
517, ceiling = 0, ceilings = {0 <repeats 16 times>}, percentages = {
  (gdb) p pi->entry[268]
  $33 = {in_deco = 0, cylinderindex = 0, sec = 2005, pressure =
{94320, 0}, o2cylinderpressure = {0, 0}, temperature = 291483, depth =
544, ceiling = 0, ceilings = {0 <repeats 16 times>}, percentages = {
  (gdb) p pi->entry[269]
  $34 = {in_deco = 0, cylinderindex = 0, sec = 1996, pressure =
{94320, 0}, o2cylinderpressure = {0, 0}, temperature = 0, depth = 0,
ceiling = 0, ceilings = {0 <repeats 16 times>}, percentages = {
  (gdb) p pi->entry[270]
  $35 = {in_deco = 0, cylinderindex = 0, sec = 1997, pressure =
{94320, 0}, o2cylinderpressure = {0, 0}, temperature = 0, depth = 0,
ceiling = 0, ceilings = {0 <repeats 16 times>}, percentages = {

at this point. Note how "2010" (33:30) is missing entirely - the 2005
one is the "ten seconds after 1995" (33:15 + 10 = 33:25).

And "pi->nr" is 271 so the above is the very end of the plot-info array.

Note how the *sample* array is bigger, and dc->samples is 137, so
those samples 132/133 are *not* at the end of the sample array. So the
two last events look like the two surface events we populate (and I
think that explains the "+1", but where are the rest of the events?

So it looks like "populate_plot_entries()" has done something wrong.

Looking further, it looks like "pi->maxtime" is 2008 (33:28min). And
that seems to be what screws "populate_plot_entries()" up: it depends
on maxtime for deciding on number of allocations, and it also has

                if (time > maxtime)
                        break;

and stops populating the plot-info entries there. And because the
interpolated entries (ie entry 2005) are inserted specially, we hadn't
updated "lasttime", so time went backwards.

So this bug is due to several oddities:

 - INSERT_ENTRY() doesn't update lasttime, so if the last entry is an
INSERT_ENTRY, we get confused at the final surface events we insert

   And no, we mustn't update lasttime in INSERT_ENTRY, because we
actually want to have it match "lastdepth".

 - the (time > maxtime) breakout thus results in us crating incorrect
entries at the end.

 - but that "time > maxtime" breakout doesn't make sense to begin with

 - HOWEVER -  we *do* use maxtime to estimate the size of the
allocation, so ....

And the reason "maxtime" is smaller than the last offset is that the
end of that dive has depths that are shallower than SURFACE_THRESHOLD.

Anyway, the quick fix is to move that exit condition (the "if (time >
maxtime) break;') because that really does screw things up in the
place it is now. Moving it to the *end* of the for-loop guarantees
that we've fixed up "lasttime" and "lastdepth" correctly, but it also
means that we may have one entry that is past maxtime in the plot
info, so change the plot-info allocation to have one more slop entry.

So try the attached patch, and if it works for you, commit it with my
sign-off (and try to munge the above explanation into a commit
message).

                        Linus
-------------- next part --------------
 profile.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/profile.c b/profile.c
index b676dc5b002f..cad768728b74 100644
--- a/profile.c
+++ b/profile.c
@@ -551,9 +551,11 @@ struct plot_data *populate_plot_entries(struct dive *dive, struct divecomputer *
 	 * but samples could be more dense than that (so add in dc->samples). We also
 	 * need to have one for every event (so count events and add that) and
 	 * additionally we want two surface events around the whole thing (thus the
-	 * additional 4).
+	 * additional 4).  There is also one extra space for a final entry
+	 * that has time > maxtime (because there can be surface samples
+	 * past "maxtime" in the original sample data)
 	 */
-	nr = dc->samples + 5 + maxtime / 10 + count_events(dc);
+	nr = dc->samples + 6 + maxtime / 10 + count_events(dc);
 	plot_data = calloc(nr, sizeof(struct plot_data));
 	pi->entry = plot_data;
 	if (!plot_data)
@@ -604,8 +606,6 @@ struct plot_data *populate_plot_entries(struct dive *dive, struct divecomputer *
 			ev = ev->next;
 		}
 
-		if (time > maxtime)
-			break;
 
 		entry->sec = time;
 		entry->depth = depth;
@@ -645,6 +645,9 @@ struct plot_data *populate_plot_entries(struct dive *dive, struct divecomputer *
 		lasttime = time;
 		lastdepth = depth;
 		idx++;
+
+		if (time > maxtime)
+			break;
 	}
 
 	/* Add two final surface events */
