[PATCH] Change in logic while aplying gps fixes to dives

[Dirk Hohndel]

I'm very uncomfortable with such a drastic change this late in the game.
What we have may be broken, but it seems to work for a lot of people and
has received some amount of testing...

Can you re-submit this after 4.2?

I'm sure a thorough analysis and a bunch of testing could sway me to
include this, but frankly I won't have time for that in the next couple of
days ;-(

/D

On Mon, Jul 14, 2014 at 10:11:47AM +0200, Salvador Cuñat wrote:
> We were actually searching dives which match the dowloaded position
> fixes. So we're also trying to take into account if the fix is automatic
> or no based on a limited amount of predefined strings (bad idea, as the
> user can change in companion app settings the predefined string).
> This way, in actual implementation, if program concludes that a fix has
> been manually got or, simply, the user is unlucky enough to have all the
> position fixes out of the dive time, find_dive_n_near() function will pair
> fix and dive in an ordered way (1st fix -> 1st dive; 2nd fix -> 2nd dive ...)
> which is probably erroneous, except for manual position fixes.
> BTW actual implementation is unable to pair the same gps fix with more than
> one dive (which is most likely to happen).
> 
> The patch changes the logic:
> 
> - Search positions for defined dives (instead of dives for defined positions)
>   without care if position has manually or  automatically been set.
> - It makes two assumptions:
>    a.- If the position fix has been taken during the dive time, is correct.
>    b.- If not during diving time, the correct one is the nearest fix
>       before the dive begins (also the usual case if manually fixed from the
>       smartphone just before jump into the water). But will work too if there
>       is only one fix *in range* after the dive (another usual case).
> - Finally, as copy_gps_location() in dive.h is used only here, let it take
>   care of naming the dive if user hasn't named it yet.
> 
> Reported-by: Marc Merlin <marc at merlins.org>
> Signed-off-by: Salvador Cuñat <salvador.cunat at gmail.com>
> ---
>  dive.h                          |    4 ++
>  qt-ui/subsurfacewebservices.cpp |   93 ++++++++++++++++-----------------------
>  2 files changed, 43 insertions(+), 54 deletions(-)
> 
> diff --git a/dive.h b/dive.h
> index 09a4dda..bb22a3f 100644
> --- a/dive.h
> +++ b/dive.h
> @@ -322,7 +322,11 @@ static inline void copy_gps_location(struct dive *from, struct dive *to)
>  	if (from && to) {
>  		to->latitude.udeg = from->latitude.udeg;
>  		to->longitude.udeg = from->longitude.udeg;
> +		if (!to->location) {
> +			to->location = strdup(from->location);
> +		}
>  	}
> +
>  }
>  
>  static inline int get_surface_pressure_in_mbar(const struct dive *dive, bool non_null)
> diff --git a/qt-ui/subsurfacewebservices.cpp b/qt-ui/subsurfacewebservices.cpp
> index 27a56ec..fe1a5cb 100644
> --- a/qt-ui/subsurfacewebservices.cpp
> +++ b/qt-ui/subsurfacewebservices.cpp
> @@ -31,69 +31,54 @@
>  struct dive_table gps_location_table;
>  static bool merge_locations_into_dives(void);
>  
> -static bool is_automatic_fix(struct dive *gpsfix)
> -{
> -	if (gpsfix && gpsfix->location &&
> -	    (!strcmp(gpsfix->location, "automatic fix") ||
> -	     !strcmp(gpsfix->location, "Auto-created dive")))
> -		return true;
> -	return false;
> -}
> -
>  #define SAME_GROUP 6 * 3600 // six hours
>  //TODO: C Code. static functions are not good if we plan to have a test for them.
>  static bool merge_locations_into_dives(void)
>  {
> -	int i, nr = 0, changed = 0;
> -	struct dive *gpsfix, *last_named_fix = NULL, *dive;
> -
> -	sort_table(&gps_location_table);
> -
> -	for_each_gps_location (i, gpsfix) {
> -		if (is_automatic_fix(gpsfix) && (dive = find_dive_including(gpsfix->when))) {
> -			if (dive && !dive_has_gps_location(dive)) {
> -#if DEBUG_WEBSERVICE
> -				struct tm tm;
> -				utc_mkdate(gpsfix->when, &tm);
> -				printf("found dive named %s @ %04d-%02d-%02d %02d:%02d:%02d\n",
> -				       gpsfix->location,
> -				       tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
> -				       tm.tm_hour, tm.tm_min, tm.tm_sec);
> -#endif
> -				changed++;
> -				copy_gps_location(gpsfix, dive);
> -			}
> -		} else {
> -			if (last_named_fix && dive_within_time_range(last_named_fix, gpsfix->when, SAME_GROUP)) {
> -				nr++;
> -			} else {
> -				nr = 1;
> -				last_named_fix = gpsfix;
> -			}
> -			dive = find_dive_n_near(gpsfix->when, nr, SAME_GROUP);
> -			if (dive) {
> -				if (!dive_has_gps_location(dive)) {
> -					copy_gps_location(gpsfix, dive);
> -					changed++;
> -				}
> -				if (!dive->location) {
> -					dive->location = strdup(gpsfix->location);
> -					changed++;
> +	int i, j;
> +	struct dive *gpsfix, *nextgpsfix, *dive;
> +
> +	sort_table(&dive_table);
> +
> +	for_each_dive (i, dive) {
> +		if (!dive_has_gps_location(dive)) {
> +			sort_table(&gps_location_table);
> +			for_each_gps_location (j, gpsfix) {
> +				if (dive_within_time_range (dive, gpsfix->when, SAME_GROUP)) {
> +					/*
> +					 * If position is fixed during dive. This is the good one.
> +					 * Asign and end for_each_gps loop
> +					 */
> +					if ((dive->when <= gpsfix->when && gpsfix->when <= dive->when + dive->duration.seconds)) {
> +						if (!dive_has_gps_location(dive)) {
> +							copy_gps_location(gpsfix,dive);
> +							break;
> +						}
> +					} else {
> +						/*
> +						 * If it is not, check if there are more position fixes.
> +						 * If none this is the last (or the only one) and so the good one.
> +						 * If there is at least one but later than end of dive, the actual fix
> +						 * is OK, asign it and end for_each_gps loop.
> +						 * If next fix is closer or into the dive, it will be evaluated in next
> +						 * loop iteration
> +						 */
> +						if (nextgpsfix = get_gps_location(j+1,&gps_location_table)) {
> +							if ((dive->when + dive->duration.seconds - gpsfix->when) < (nextgpsfix->when - gpsfix->when)) {
> +								copy_gps_location(gpsfix,dive);
> +								break;
> +							}
> +						} else {
> +							copy_gps_location(gpsfix,dive);
> +							break;
> +						}
> +					}
>  				}
> -			} else {
> -				struct tm tm;
> -				utc_mkdate(gpsfix->when, &tm);
> -#if DEBUG_WEBSERVICE
> -				printf("didn't find dive matching gps fix named %s @ %04d-%02d-%02d %02d:%02d:%02d\n",
> -				       gpsfix->location,
> -				       tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
> -				       tm.tm_hour, tm.tm_min, tm.tm_sec);
> -#endif
>  			}
>  		}
>  	}
> -	return changed > 0;
>  }
> +
>  //TODO: C-code.
>  static void clear_table(struct dive_table *table)
>  {
> -- 
> 1.7.10.4
>