[PATCH] Be much more careful about merging dives
Linus Torvalds
torvalds at linux-foundation.org
Tue Sep 22 12:43:30 PDT 2015
From: Linus Torvalds <torvalds at linux-foundation.org>
Date: Tue, 22 Sep 2015 12:32:27 -0700
Subject: [PATCH] Be much more careful about merging dives
This patch changes the dive merging to be much more careful about
things, because it turns out that we had several small oddities that
caused big merge issues.
The oddities are:
- the dive "duration" is actually how long we spend under water.
But that means that when we do "dive->when + dive.duration.seconds"
to calculate the end of the dive, that is nonsensical if you came up
to the surface in the middle of a dive.
Now, normally you don't see profiles like that, but once you start
merging dives together, it can go from "small detail" to "dominant
factor".
- We have two different cases of merging: the automatic "merge new dive
computer download if it looks like the same dive" (which always has a
merge offset of 0, since we merge it as a new dive computer) and the
"merge two different dives into one longer dive.
The code assumed that it could look at the "downloaded" flag for the
dive to check one or the other, but that doesn't really work.
Reading a dive from an XML file isn't any different from downloading
it.
So we need to change the logic to determine what kind of merge it is
to actually check the passed-in time offset.
With this, Stuart Vernon's test-case of eight dives with short surface
intervals in between end up merging correctly into one dive.
Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
Reported-by: Stuart Vernon <stuartv at force2.net>
---
NOTE! This fixes merging with current subsurface, but the merged dive
displays incorrectly because the dive plot isn't updated. You have to
switch to another dive and back to get the plot to update.
I didn't fix that, because that is a totally different bug, and I'm not
sure what the right way to signal replotting is. Dirk?
dive.c | 114 ++++++++++++++++++++++++++++++++++++++++---------
dive.h | 2 +
qt-ui/divelistview.cpp | 5 ++-
3 files changed, 99 insertions(+), 22 deletions(-)
diff --git a/dive.c b/dive.c
index 10ce6385948f..681db92f24b3 100644
--- a/dive.c
+++ b/dive.c
@@ -2743,28 +2743,48 @@ int count_dives_with_suit(const char *suit)
return counter;
}
-
+/*
+ * Merging two dives can be subtle, because there's two different ways
+ * of merging:
+ *
+ * (a) two distinctly _different_ dives that have the same dive computer
+ * are merged into one longer dive, because the user asked for it
+ * in the divelist.
+ *
+ * Because this case is with teh same dive computer, we *know* the
+ * two must have a different start time, and "offset" is the relative
+ * time difference between the two.
+ *
+ * (a) two different dive computers that we migth want to merge into
+ * one single dive with multiple dive computers.
+ *
+ * This is the "try_to_merge()" case, which will have offset == 0,
+ * even if the dive times migth be different.
+ */
struct dive *merge_dives(struct dive *a, struct dive *b, int offset, bool prefer_downloaded)
{
struct dive *res = alloc_dive();
struct dive *dl = NULL;
- /* Aim for newly downloaded dives to be 'b' (keep old dive data first) */
- if (a->downloaded && !b->downloaded) {
- struct dive *tmp = a;
- a = b;
- b = tmp;
+ if (offset) {
+ /*
+ * If "likely_same_dive()" returns true, that means that
+ * it is *not* the same dive computer, and we do not want
+ * to try to turn it into a single longer dive. So we'd
+ * join them as two separate dive computers at zero offset.
+ */
+ if (likely_same_dive(a, b))
+ offset = 0;
+ } else {
+ /* Aim for newly downloaded dives to be 'b' (keep old dive data first) */
+ if (a->downloaded && !b->downloaded) {
+ struct dive *tmp = a;
+ a = b;
+ b = tmp;
+ }
+ if (prefer_downloaded && b->downloaded)
+ dl = b;
}
- if (prefer_downloaded && b->downloaded)
- dl = b;
-
- /*
- * Did the user ask us to merge dives in the dive list?
- * We may want to just join the dive computers, not try to
- * interleave them at some offset.
- */
- if (offset && likely_same_dive(a, b))
- offset = 0;
res->when = dl ? dl->when : a->when;
res->selected = a->selected || b->selected;
@@ -2793,6 +2813,56 @@ struct dive *merge_dives(struct dive *a, struct dive *b, int offset, bool prefer
return res;
}
+/*
+ * "dc_maxtime()" is how much total time this dive computer
+ * has for this dive. Note that it can differ from "duration"
+ * if there are surface events in the middle.
+ *
+ * Still, we do ignore all but the last surface samples from the
+ * end, because some divecomputers just generate lots of them.
+ */
+static inline int dc_totaltime(const struct divecomputer *dc)
+{
+ int time = dc->duration.seconds;
+ int nr = dc->samples;
+
+ while (nr--) {
+ struct sample *s = dc->sample + nr;
+ time = s->time.seconds;
+ if (s->depth.mm >= SURFACE_THRESHOLD)
+ break;
+ }
+ return time;
+}
+
+/*
+ * The end of a dive is actually not trivial, because "duration"
+ * is not the duration until the end, but the time we spend under
+ * water, which can be very different if there are surface events
+ * during the dive.
+ *
+ * So walk the dive computers, looking for the longest actual
+ * time in the samples (and just default to the dive duration if
+ * there are no samples).
+ */
+static inline int dive_totaltime(const struct dive *dive)
+{
+ int time = dive->duration.seconds;
+ struct divecomputer *dc;
+
+ for_each_dc(dive, dc) {
+ int dc_time = dc_totaltime(dc);
+ if (dc_time > time)
+ time = dc_time;
+ }
+ return time;
+}
+
+timestamp_t dive_endtime(const struct dive *dive)
+{
+ return dive->when + dive_totaltime(dive);
+}
+
struct dive *find_dive_including(timestamp_t when)
{
int i;
@@ -2802,7 +2872,7 @@ struct dive *find_dive_including(timestamp_t when)
* also we always use the duration from the first divecomputer
* could this ever be a problem? */
for_each_dive (i, dive) {
- if (dive->when <= when && when <= dive->when + dive->duration.seconds)
+ if (dive->when <= when && when <= dive_endtime(dive))
return dive;
}
return NULL;
@@ -2810,12 +2880,16 @@ struct dive *find_dive_including(timestamp_t when)
bool time_during_dive_with_offset(struct dive *dive, timestamp_t when, timestamp_t offset)
{
- return dive->when - offset <= when && when <= dive->when + dive->duration.seconds + offset;
+ timestamp_t start = dive->when;
+ timestamp_t end = dive_endtime(dive);
+ return start - offset <= when && when <= end + offset;
}
bool dive_within_time_range(struct dive *dive, timestamp_t when, timestamp_t offset)
{
- return when - offset <= dive->when && dive->when + dive->duration.seconds <= when + offset;
+ timestamp_t start = dive->when;
+ timestamp_t end = dive_endtime(dive);
+ return when - offset <= start && end <= when + offset;
}
/* find the n-th dive that is part of a group of dives within the offset around 'when'.
@@ -2970,7 +3044,7 @@ bool dive_check_picture_time(struct dive *d, int shift_time, timestamp_t timesta
offset_t offset;
if (timestamp) {
offset.seconds = timestamp - d->when + shift_time;
- if (offset.seconds > -D30MIN && offset.seconds < (int)d->duration.seconds + D30MIN) {
+ if (offset.seconds > -D30MIN && offset.seconds < dive_totaltime(d) + D30MIN) {
// this picture belongs to this dive
return true;
}
diff --git a/dive.h b/dive.h
index 4eb44cfc9c8d..3bcc7b0e2dcc 100644
--- a/dive.h
+++ b/dive.h
@@ -562,6 +562,8 @@ static inline struct divecomputer *get_dive_dc(struct dive *dive, int nr)
return dc;
}
+extern timestamp_t dive_endtime(const struct dive *dive);
+
extern void make_first_dc(void);
extern int count_divecomputers(void);
extern void delete_current_divecomputer(void);
diff --git a/qt-ui/divelistview.cpp b/qt-ui/divelistview.cpp
index 2ee5f5dcda03..39eab9c7ad01 100644
--- a/qt-ui/divelistview.cpp
+++ b/qt-ui/divelistview.cpp
@@ -22,6 +22,7 @@
#include "divelistview.h"
#include "divepicturemodel.h"
#include "metrics.h"
+#include "helpers.h"
// # Date Rtg Dpth Dur Tmp Wght Suit Cyl Gas SAC OTU CNS Loc
static int defaultWidth[] = { 70, 140, 90, 50, 50, 50, 50, 70, 50, 50, 70, 50, 50, 500};
@@ -575,12 +576,12 @@ static bool can_merge(const struct dive *a, const struct dive *b, enum asked_use
if (a->when > b->when)
return false;
/* Don't merge dives if there's more than half an hour between them */
- if (a->when + a->duration.seconds + 30 * 60 < b->when) {
+ if (dive_endtime(a) + 30 * 60 < b->when) {
if (*have_asked == NOTYET) {
if (QMessageBox::warning(MainWindow::instance(),
MainWindow::instance()->tr("Warning"),
MainWindow::instance()->tr("Trying to merge dives with %1min interval in between").arg(
- (b->when - a->when - a->duration.seconds) / 60),
+ (b->when - dive_endtime(a)) / 60),
QMessageBox::Ok | QMessageBox::Cancel) == QMessageBox::Cancel) {
*have_asked = DONTMERGE;
return false;
--
2.6.0.rc1.16.g1962994
More information about the subsurface
mailing list