Rewrite GPS fix matching code for imports from our webservice

The longer I stared at the existing code the less it made sense.
So instead I rewrote it in a way that seems logical to me. And added a
boatload of debugging output (which needs to be removed, of course).

I tested this against more than a hundred dives and it seemed to always
pick the right fix.

Signed-off-by: Dirk Hohndel <dirk@hohndel.org>

3 files changed, 46 insertions, 10 deletions

diff --git a/dive.c b/dive.c
index 02c1090d9..aadddfd14 100644
--- a/dive.c
+++ b/dive.c
@@ -2807,6 +2807,11 @@ struct dive *find_dive_including(timestamp_t when)
 	return NULL;
 }
 
+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;
+}
+
 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;
diff --git a/dive.h b/dive.h
index 36d9a1200..f07c9d9b2 100644
--- a/dive.h
+++ b/dive.h
@@ -635,6 +635,7 @@ extern const char *get_error_string(void);
 
 extern struct dive *find_dive_including(timestamp_t when);
 extern bool dive_within_time_range(struct dive *dive, timestamp_t when, timestamp_t offset);
+extern bool time_during_dive_with_offset(struct dive *dive, timestamp_t when, timestamp_t offset);
 struct dive *find_dive_n_near(timestamp_t when, int n, timestamp_t offset);
 
 /* Check if two dive computer entries are the exact same dive (-1=no/0=maybe/1=yes) */
diff --git a/qt-ui/subsurfacewebservices.cpp b/qt-ui/subsurfacewebservices.cpp
index 6c75513b7..cfd255b0e 100644
--- a/qt-ui/subsurfacewebservices.cpp
+++ b/qt-ui/subsurfacewebservices.cpp
@@ -62,12 +62,15 @@ static bool merge_locations_into_dives(void)
 	for_each_dive (i, dive) {
 		if (!dive_has_gps_location(dive)) {
 			for (j = tracer; (gpsfix = get_dive_from_table(j, &gps_location_table)) !=NULL; j++) {
-				if (dive_within_time_range (dive, gpsfix->when, SAME_GROUP)) {
+				if (time_during_dive_with_offset(dive, gpsfix->when, SAME_GROUP)) {
+					qDebug() << "processing gpsfix @" << get_dive_date_string(gpsfix->when) << "which is withing six hours of dive from" <<
+						 get_dive_date_string(dive->when) << "until" << get_dive_date_string(dive->when + dive->duration.seconds);
 					/*
 					 * If position is fixed during dive. This is the good one.
 					 * Asign and mark position, and end gps_location loop
 					 */
-					if ((dive->when <= gpsfix->when && gpsfix->when <= dive->when + dive->duration.seconds)) {
+					if (time_during_dive_with_offset(dive, gpsfix->when, 0)) {
+						qDebug() << "gpsfix is during the dive, pick that one";
 						copy_gps_location(gpsfix, dive);
 						changed++;
 						tracer = j;
@@ -76,23 +79,50 @@ static bool merge_locations_into_dives(void)
 						/*
 						 * If it is not, check if there are more position fixes in SAME_GROUP range
 						 */
-						if ((nextgpsfix = get_dive_from_table(j+1,&gps_location_table)) &&
-						    dive_within_time_range (dive, nextgpsfix->when, SAME_GROUP)) {
-							/*
-							 * If distance from gpsfix to end of dive is shorter than distance between
-							 * gpsfix and nextgpsfix, gpsfix is the good one. Asign, mark and end loop.
-							 * If not, simply fail and nextgpsfix will be evaluated in next iteration.
-							 */
-							if ((dive->when + dive->duration.seconds - gpsfix->when) < (nextgpsfix->when - gpsfix->when)) {
+						if ((nextgpsfix = get_dive_from_table(j + 1, &gps_location_table)) &&
+						    time_during_dive_with_offset(dive, nextgpsfix->when, SAME_GROUP)) {
+							qDebug() << "look at the next gps fix @" << get_dive_date_string(nextgpsfix->when);
+							/* first let's test if this one is during the dive */
+							if (time_during_dive_with_offset(dive, nextgpsfix->when, 0)) {
+								qDebug() << "which is during the dive, pick that one";
+								copy_gps_location(nextgpsfix, dive);
+								changed++;
+								tracer = j + 1;
+								break;
+							}
+							/* we know the gps fixes are sorted; if they are both before the dive, ignore the first,
+							 * if theay are both after the dive, take the first,
+							 * if the first is before and the second is after, take the closer one */
+							if (nextgpsfix->when < dive->when) {
+								qDebug() << "which is closer to the start of the dive, do continue with that";
+								continue;
+							} else if (gpsfix->when > dive->when + dive->duration.seconds) {
+								qDebug() << "which is even later after the end of the dive, so pick the previous one";
 								copy_gps_location(gpsfix, dive);
 								changed++;
 								tracer = j;
 								break;
+							} else {
+								/* ok, gpsfix is before, nextgpsfix is after */
+								if (dive->when - gpsfix->when <= nextgpsfix->when - (dive->when + dive->duration.seconds)) {
+									qDebug() << "pick the one before as it's closer to the start";
+									copy_gps_location(gpsfix, dive);
+									changed++;
+									tracer = j;
+									break;
+								} else {
+									qDebug() << "pick the one after as it's closer to the start";
+									copy_gps_location(nextgpsfix, dive);
+									changed++;
+									tracer = j + 1;
+									break;
+								}
 							}
 						/*
 						 * If no more positions in range, the actual is the one. Asign, mark and end loop.
 						 */
 						} else {
+							qDebug() << "which seems to be the best one for this dive, so pick it";
 							copy_gps_location(gpsfix, dive);
 							changed++;
 							tracer = j;