Fix subtle bug in dive site import from V2 file

Assume your V2 file contains two locations with different name but
identical GPS location. "Blue Corner" and "Blue Corner, Palau". And you
have many dives there.

When reading a V2 file the GPS is read first and a dive site is looked up
based on that. Let's assume the lookup by gps finds "Blue Corner, Palau".

Now every time we get the "Blue Corner" site in the V2 file we look  up
the GPS, get "Blue Corner, Palau" as dive site, then read the name "Blue
Corner" and say "oops, different site" and create a new one. Resulting in
several dive sites named "Blue Corner" with identical GPS but different
UUID (as we add the dive time into the SHA for the deterministic UUID).

With this commit, if we have a dive site that matches the GPS but has a
different name, we check if we happen to have an exact match for the dive
site information from the XML file (and use it, if we have it) before
creating a new dive site.

Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This commit is contained in:
Dirk Hohndel 2015-08-30 10:10:07 -07:00
parent d15a1db428
commit e0587cb6df
3 changed files with 33 additions and 9 deletions

View file

@ -36,6 +36,21 @@ uint32_t get_dive_site_uuid_by_gps(degrees_t latitude, degrees_t longitude, stru
return 0; return 0;
} }
/* to avoid a bug where we have two dive sites with different name and the same GPS coordinates
* and first get the gps coordinates (reading a V2 file) and happen to get back "the other" name,
* this function allows us to verify if a very specific name/GPS combination already exists */
uint32_t get_dive_site_uuid_by_gps_and_name(char *name, degrees_t latitude, degrees_t longitude)
{
int i;
struct dive_site *ds;
for_each_dive_site (i, ds) {
if (ds->latitude.udeg == latitude.udeg && ds->longitude.udeg == longitude.udeg && same_string(ds->name, name))
return ds->uuid;
}
return 0;
}
// Calculate the distance in meters between two coordinates. // Calculate the distance in meters between two coordinates.
unsigned int get_distance(degrees_t lat1, degrees_t lon1, degrees_t lat2, degrees_t lon2) unsigned int get_distance(degrees_t lat1, degrees_t lon1, degrees_t lat2, degrees_t lon2)
{ {

View file

@ -59,6 +59,7 @@ uint32_t create_dive_site_from_current_dive(const char *name);
uint32_t create_dive_site_with_gps(const char *name, degrees_t latitude, degrees_t longitude, timestamp_t divetime); uint32_t create_dive_site_with_gps(const char *name, degrees_t latitude, degrees_t longitude, timestamp_t divetime);
uint32_t get_dive_site_uuid_by_name(const char *name, struct dive_site **dsp); uint32_t get_dive_site_uuid_by_name(const char *name, struct dive_site **dsp);
uint32_t get_dive_site_uuid_by_gps(degrees_t latitude, degrees_t longitude, struct dive_site **dsp); uint32_t get_dive_site_uuid_by_gps(degrees_t latitude, degrees_t longitude, struct dive_site **dsp);
uint32_t get_dive_site_uuid_by_gps_and_name(char *name, degrees_t latitude, degrees_t longitude);
uint32_t get_dive_site_uuid_by_gps_proximity(degrees_t latitude, degrees_t longitude, int distance, struct dive_site **dsp); uint32_t get_dive_site_uuid_by_gps_proximity(degrees_t latitude, degrees_t longitude, int distance, struct dive_site **dsp);
bool dive_site_is_empty(struct dive_site *ds); bool dive_site_is_empty(struct dive_site *ds);
void copy_dive_site(struct dive_site *orig, struct dive_site *copy); void copy_dive_site(struct dive_site *orig, struct dive_site *copy);

View file

@ -1247,17 +1247,25 @@ static void add_dive_site(char *ds_name, struct dive *dive)
ds->name = copy_string(buffer); ds->name = copy_string(buffer);
} else if (!same_string(ds->name, buffer)) { } else if (!same_string(ds->name, buffer)) {
// if it's not the same name, it's not the same dive site // if it's not the same name, it's not the same dive site
dive->dive_site_uuid = create_dive_site(buffer, dive->when); // but wait, we could have gotten this one based on GPS coords and could
struct dive_site *newds = get_dive_site_by_uuid(dive->dive_site_uuid); // have had two different names for the same site... so let's search the other
if (cur_latitude.udeg || cur_longitude.udeg) { // way around
// we started this uuid with GPS data, so lets use those uint32_t exact_match_uuid = get_dive_site_uuid_by_gps_and_name(buffer, ds->latitude, ds->longitude);
newds->latitude = cur_latitude; if (exact_match_uuid) {
newds->longitude = cur_longitude; dive->dive_site_uuid = exact_match_uuid;
} else { } else {
newds->latitude = ds->latitude; dive->dive_site_uuid = create_dive_site(buffer, dive->when);
newds->longitude = ds->longitude; struct dive_site *newds = get_dive_site_by_uuid(dive->dive_site_uuid);
if (cur_latitude.udeg || cur_longitude.udeg) {
// we started this uuid with GPS data, so lets use those
newds->latitude = cur_latitude;
newds->longitude = cur_longitude;
} else {
newds->latitude = ds->latitude;
newds->longitude = ds->longitude;
}
newds->notes = add_to_string(newds->notes, translate("gettextFromC", "additional name for site: %s\n"), ds->name);
} }
newds->notes = add_to_string(newds->notes, translate("gettextFromC", "additional name for site: %s\n"), ds->name);
} else { } else {
// add the existing dive site to the current dive // add the existing dive site to the current dive
dive->dive_site_uuid = uuid; dive->dive_site_uuid = uuid;