[PATCH] Make sure all dive computer events are represented in the plot_info data

Linus Torvalds torvalds at linux-foundation.org
Wed Jun 1 13:06:14 PDT 2016


From: Linus Torvalds <torvalds at linux-foundation.org>
Date: Wed, 1 Jun 2016 12:49:32 -0700
Subject: [PATCH] Make sure all dive computer events are represented in the plot_info data

We could create a plot_info data that didn't contain all the time slots
for the events fromt he dive computer, which would terminally confuse
the plotting of the event profile widgets because it couldn't match up
the event with the dive plot data model.

So for example, in DiveEventItem::recalculatePos(), when the code tries
to figure out the spot in the data model, it could fail, and then try to
hide the event (because without the data model information it doesn't
know where it should go).  But that hiding would then not match the
logic in DiveEventItem::shouldBeHidden(), and the event would end up
being shown in the upper left-hand corner of the profile after all.

The reason the plot_info data wouldn't contain the time slots is that
the slots are allocated primarily for the sample data, and then the
events would be added in between sample data in populate_plot_entries().

But since we'd only add the event pointer *between* samples, that would
mean that events after the last samples would not get plot-info points
allocated to them.

That issue was exacerbated by how we also truncate uninteresting samples
at the end when some dive computers end up giving a long stream
(possibly several minutes) of "at the surface" events before they
finally turn off logging.

This makes sure that we take the event timestamps into account for the
"maxtime" calculation, and also that we finish populating the plot_info
data with any final event timestamps.

Now all the events will have a matching plot_info entry.

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

This one actually took me *much* too long to figure out, because I 
initially couldn't for the life of me see how the hell some events ended 
up showing up in the top left-hand corner of the profile. The whole 
interaction with "recalculatePos()" hiding events without actually 
initializing their position, and then the events being shown later anyway 
was very confusing.

The symptom would be that a event marker would show up in the top left 
corner, and while the event name was correct, the time and depth would be 
incorrectly shown as zero in the tooltip even though the event time was 
actually at the very end of the dive.

This problem is not new - I suspect it's been around since the initial Qt 
rewrite. I only noticed it because I was sanity-checking the events after 
the previous patch-series.

But this patch is independent of the surface event patches, and stands on 
its own.

 core/profile.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/core/profile.c b/core/profile.c
index 4e9ee88ee35b..4438154e3fed 100644
--- a/core/profile.c
+++ b/core/profile.c
@@ -472,6 +472,7 @@ struct plot_info calculate_max_limits_new(struct dive *dive, struct divecomputer
 		int i = dc->samples;
 		int lastdepth = 0;
 		struct sample *s = dc->sample;
+		struct event *ev;
 
 		while (--i >= 0) {
 			int depth = s->depth.mm;
@@ -501,6 +502,15 @@ struct plot_info calculate_max_limits_new(struct dive *dive, struct divecomputer
 			lastdepth = depth;
 			s++;
 		}
+
+		/* Make sure we can fit all events */
+		ev = dc->events;
+		while (ev) {
+			if (ev->time.seconds > maxtime)
+				maxtime = ev->time.seconds;
+			ev = ev->next;
+		}
+
 		dc = dc->next;
 		if (dc == NULL && !seen) {
 			dc = given_dc;
@@ -608,7 +618,6 @@ struct plot_data *populate_plot_entries(struct dive *dive, struct divecomputer *
 			ev = ev->next;
 		}
 
-
 		entry->sec = time;
 		entry->depth = depth;
 
@@ -652,6 +661,18 @@ struct plot_data *populate_plot_entries(struct dive *dive, struct divecomputer *
 			break;
 	}
 
+	/* Add any remaining events */
+	while (ev) {
+		struct plot_data *entry = plot_data + idx;
+		int time = ev->time.seconds;
+
+		if (time > lasttime) {
+			INSERT_ENTRY(ev->time.seconds, 0, 0);
+			lasttime = time;
+		}
+		ev = ev->next;
+	}
+
 	/* Add two final surface events */
 	plot_data[idx++].sec = lasttime + 1;
 	plot_data[idx++].sec = lasttime + 2;
-- 
2.9.0.rc0.21.g7777322



More information about the subsurface mailing list