From 4d7291d4a16a8fa803d115818ad878f43c9a7255 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Mon, 24 Jun 2024 20:47:02 +0200 Subject: [PATCH] core: move merge_dives() functios to struct dive_table These functions have to access other dives in the list to calculate CNS, etc, so let's call them from there. Signed-off-by: Berthold Stoeger --- commands/command_divelist.cpp | 4 +- core/dive.cpp | 192 ++++++++-------------------------- core/dive.h | 12 +-- core/divelist.cpp | 121 +++++++++++++++++++++ core/divelist.h | 8 ++ core/divelog.cpp | 4 +- 6 files changed, 178 insertions(+), 163 deletions(-) diff --git a/commands/command_divelist.cpp b/commands/command_divelist.cpp index 0dcf988de..f4959ad3e 100644 --- a/commands/command_divelist.cpp +++ b/commands/command_divelist.cpp @@ -921,7 +921,7 @@ MergeDives::MergeDives(const QVector &dives) return; } - auto [d, trip, site] = merge_dives(*dives[0], *dives[1], dives[1]->when - dives[0]->when, false); + auto [d, trip, site] = divelog.dives.merge_dives(*dives[0], *dives[1], dives[1]->when - dives[0]->when, false); // Currently, the core code selects the dive -> this is not what we want, as // we manually manage the selection post-command. @@ -931,7 +931,7 @@ MergeDives::MergeDives(const QVector &dives) // Set the preferred dive trip, so that for subsequent merges the better trip can be selected d->divetrip = trip; for (int i = 2; i < dives.count(); ++i) { - auto [d2, trip, site] = merge_dives(*d, *dives[i], dives[i]->when - d->when, false); + auto [d2, trip, site] = divelog.dives.merge_dives(*d, *dives[i], dives[i]->when - d->when, false); d = std::move(d2); // Set the preferred dive trip and site, so that for subsequent merges the better trip and site can be selected d->divetrip = trip; diff --git a/core/dive.cpp b/core/dive.cpp index 528836cb3..d57440d34 100644 --- a/core/dive.cpp +++ b/core/dive.cpp @@ -1086,10 +1086,10 @@ void dive::fixup_no_cylinder() } /* Don't pick a zero for MERGE_MIN() */ -#define MERGE_MAX(res, a, b, n) res->n = std::max(a->n, b->n) -#define MERGE_MIN(res, a, b, n) res->n = (a->n) ? (b->n) ? std::min(a->n, b->n) : (a->n) : (b->n) -#define MERGE_TXT(res, a, b, n, sep) res->n = merge_text(a->n, b->n, sep) -#define MERGE_NONZERO(res, a, b, n) (res)->n = (a)->n ? (a)->n : (b)->n +#define MERGE_MAX(res, a, b, n) res->n = std::max(a.n, b.n) +#define MERGE_MIN(res, a, b, n) res->n = (a.n) ? (b.n) ? std::min(a.n, b.n) : (a.n) : (b.n) +#define MERGE_TXT(res, a, b, n, sep) res->n = merge_text(a.n, b.n, sep) +#define MERGE_NONZERO(res, a, b, n) (res)->n = (a).n ? (a).n : (b).n /* * This is like append_sample(), but if the distance from the last sample @@ -1702,47 +1702,7 @@ static void merge_temperatures(struct dive &res, const struct dive &a, const str temperature_t airtemp_a = un_fixup_airtemp(a); temperature_t airtemp_b = un_fixup_airtemp(b); res.airtemp = airtemp_a.mkelvin ? airtemp_a : airtemp_b; - MERGE_NONZERO(&res, &a, &b, watertemp.mkelvin); -} - -/* - * Pick a trip for a dive - */ -static struct dive_trip *get_preferred_trip(const struct dive *a, const struct dive *b) -{ - dive_trip *atrip, *btrip; - - /* If only one dive has a trip, choose that */ - atrip = a->divetrip; - btrip = b->divetrip; - if (!atrip) - return btrip; - if (!btrip) - return atrip; - - /* Both dives have a trip - prefer the non-autogenerated one */ - if (atrip->autogen && !btrip->autogen) - return btrip; - if (!atrip->autogen && btrip->autogen) - return atrip; - - /* Otherwise, look at the trip data and pick the "better" one */ - if (atrip->location.empty()) - return btrip; - if (btrip->location.empty()) - return atrip; - if (atrip->notes.empty()) - return btrip; - if (btrip->notes.empty()) - return atrip; - - /* - * Ok, so both have location and notes. - * Pick the earlier one. - */ - if (a->when < b->when) - return atrip; - return btrip; + MERGE_NONZERO(&res, a, b, watertemp.mkelvin); } #if CURRENTLY_NOT_USED @@ -1984,10 +1944,10 @@ static int match_dc_dive(const struct dive &a, const struct dive &b) * dives together manually. But this tries to handle the sane * cases. */ -static bool likely_same_dive(const struct dive &a, const struct dive &b) +bool dive::likely_same(const struct dive &b) const { /* don't merge manually added dives with anything */ - if (is_dc_manually_added_dive(&a.dcs[0]) || + if (is_dc_manually_added_dive(&dcs[0]) || is_dc_manually_added_dive(&b.dcs[0])) return 0; @@ -1995,47 +1955,24 @@ static bool likely_same_dive(const struct dive &a, const struct dive &b) * Do some basic sanity testing of the values we * have filled in during 'fixup_dive()' */ - if (!similar(a.maxdepth.mm, b.maxdepth.mm, 1000) || - (a.meandepth.mm && b.meandepth.mm && !similar(a.meandepth.mm, b.meandepth.mm, 1000)) || - !a.duration.seconds || !b.duration.seconds || - !similar(a.duration.seconds, b.duration.seconds, 5 * 60)) + if (!similar(maxdepth.mm, b.maxdepth.mm, 1000) || + (meandepth.mm && b.meandepth.mm && !similar(meandepth.mm, b.meandepth.mm, 1000)) || + !duration.seconds || !b.duration.seconds || + !similar(duration.seconds, b.duration.seconds, 5 * 60)) return 0; /* See if we can get an exact match on the dive computer */ - if (match_dc_dive(a, b)) + if (match_dc_dive(*this, b)) return true; /* * Allow a time difference due to dive computer time * setting etc. Check if they overlap. */ - int fuzz = std::max(a.duration.seconds, b.duration.seconds) / 2; + int fuzz = std::max(duration.seconds, b.duration.seconds) / 2; fuzz = std::max(fuzz, 60); - return (a.when <= b.when + fuzz) && (a.when >= b.when - fuzz); -} - -/* - * This could do a lot more merging. Right now it really only - * merges almost exact duplicates - something that happens easily - * with overlapping dive downloads. - * - * If new dives are merged into the dive table, dive a is supposed to - * be the old dive and dive b is supposed to be the newly imported - * dive. If the flag "prefer_downloaded" is set, data of the latter - * will take priority over the former. - * - * Attn: The dive_site parameter of the dive will be set, but the caller - * still has to register the dive in the dive site! - */ -struct std::unique_ptr try_to_merge(const struct dive &a, const struct dive &b, bool prefer_downloaded) -{ - if (!likely_same_dive(a, b)) - return {}; - - auto [res, trip, site] = merge_dives(a, b, 0, prefer_downloaded); - res->dive_site = site; /* Caller has to call site->add_dive()! */ - return std::move(res); + return (when <= b.when + fuzz) && (when >= b.when - fuzz); } static bool operator==(const sample &a, const sample &b) @@ -2200,38 +2137,9 @@ bool dive::is_logged() const return has_dc_type(*this, false); } -/* - * 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 the same dive computer, we *know* the - * two must have a different start time, and "offset" is the relative - * time difference between the two. - * - * (b) two different dive computers that we might 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 might be different. - * - * If new dives are merged into the dive table, dive a is supposed to - * be the old dive and dive b is supposed to be the newly imported - * dive. If the flag "prefer_downloaded" is set, data of the latter - * will take priority over the former. - * - * The trip the new dive should be associated with (if any) is returned - * in the "trip" output parameter. - * - * The dive site the new dive should be added to (if any) is returned - * in the "dive_site" output parameter. - */ -merge_result merge_dives(const struct dive &a_in, const struct dive &b_in, int offset, bool prefer_downloaded) +std::unique_ptr dive::create_merged_dive(const struct dive &a, const struct dive &b, int offset, bool prefer_downloaded) { - merge_result res = { std::make_unique(), nullptr, nullptr }; + auto res = std::make_unique(); if (offset) { /* @@ -2240,59 +2148,41 @@ merge_result merge_dives(const struct dive &a_in, const struct dive &b_in, int o * 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_in, b_in)) + if (a.likely_same(b)) offset = 0; } - const dive *a = &a_in; - const dive *b = &b_in; - if (is_dc_planner(&a->dcs[0])) - std::swap(a, b); - - res.dive->when = prefer_downloaded ? b->when : a->when; - res.dive->selected = a->selected || b->selected; - res.trip = get_preferred_trip(a, b); - MERGE_TXT(res.dive, a, b, notes, "\n--\n"); - MERGE_TXT(res.dive, a, b, buddy, ", "); - MERGE_TXT(res.dive, a, b, diveguide, ", "); - MERGE_MAX(res.dive, a, b, rating); - MERGE_TXT(res.dive, a, b, suit, ", "); - MERGE_MAX(res.dive, a, b, number); - MERGE_NONZERO(res.dive, a, b, visibility); - MERGE_NONZERO(res.dive, a, b, wavesize); - MERGE_NONZERO(res.dive, a, b, current); - MERGE_NONZERO(res.dive, a, b, surge); - MERGE_NONZERO(res.dive, a, b, chill); - res.dive->pictures = !a->pictures.empty() ? a->pictures : b->pictures; - res.dive->tags = taglist_merge(a->tags, b->tags); + res->when = prefer_downloaded ? b.when : a.when; + res->selected = a.selected || b.selected; + MERGE_TXT(res, a, b, notes, "\n--\n"); + MERGE_TXT(res, a, b, buddy, ", "); + MERGE_TXT(res, a, b, diveguide, ", "); + MERGE_MAX(res, a, b, rating); + MERGE_TXT(res, a, b, suit, ", "); + MERGE_MAX(res, a, b, number); + MERGE_NONZERO(res, a, b, visibility); + MERGE_NONZERO(res, a, b, wavesize); + MERGE_NONZERO(res, a, b, current); + MERGE_NONZERO(res, a, b, surge); + MERGE_NONZERO(res, a, b, chill); + res->pictures = !a.pictures.empty() ? a.pictures : b.pictures; + res->tags = taglist_merge(a.tags, b.tags); /* if we get dives without any gas / cylinder information in an import, make sure * that there is at leatst one entry in the cylinder map for that dive */ - auto cylinders_map_a = std::make_unique(std::max(size_t(1), a->cylinders.size())); - auto cylinders_map_b = std::make_unique(std::max(size_t(1), b->cylinders.size())); - merge_cylinders(*res.dive, *a, *b, cylinders_map_a.get(), cylinders_map_b.get()); - merge_equipment(*res.dive, *a, *b); - merge_temperatures(*res.dive, *a, *b); + auto cylinders_map_a = std::make_unique(std::max(size_t(1), a.cylinders.size())); + auto cylinders_map_b = std::make_unique(std::max(size_t(1), b.cylinders.size())); + merge_cylinders(*res, a, b, cylinders_map_a.get(), cylinders_map_b.get()); + merge_equipment(*res, a, b); + merge_temperatures(*res, a, b); if (prefer_downloaded) { /* If we prefer downloaded, do those first, and get rid of "might be same" computers */ - join_dive_computers(*res.dive, *b, *a, cylinders_map_b.get(), cylinders_map_a.get(), true); - } else if (offset && might_be_same_device(a->dcs[0], b->dcs[0])) { - interleave_dive_computers(*res.dive, *a, *b, cylinders_map_a.get(), cylinders_map_b.get(), offset); + join_dive_computers(*res, b, a, cylinders_map_b.get(), cylinders_map_a.get(), true); + } else if (offset && might_be_same_device(a.dcs[0], b.dcs[0])) { + interleave_dive_computers(*res, a, b, cylinders_map_a.get(), cylinders_map_b.get(), offset); } else { - join_dive_computers(*res.dive, *a, *b, cylinders_map_a.get(), cylinders_map_b.get(), false); + join_dive_computers(*res, a, b, cylinders_map_a.get(), cylinders_map_b.get(), false); } - /* The CNS values will be recalculated from the sample in fixup_dive() */ - res.dive->cns = res.dive->maxcns = 0; - - /* we take the first dive site, unless it's empty */ - res.site = a->dive_site && !a->dive_site->is_empty() ? a->dive_site : b->dive_site; - if (!dive_site_has_gps_location(res.site) && dive_site_has_gps_location(b->dive_site)) { - /* we picked the first dive site and that didn't have GPS data, but the new dive has - * GPS data (that could be a download from a GPS enabled dive computer). - * Keep the dive site, but add the GPS data */ - res.site->location = b->dive_site->location; - } - divelog.dives.fixup_dive(*res.dive); return res; } diff --git a/core/dive.h b/core/dive.h index 6289baab0..0f4fbe8d7 100644 --- a/core/dive.h +++ b/core/dive.h @@ -87,6 +87,7 @@ struct dive { bool is_planned() const; bool is_logged() const; + bool likely_same(const struct dive &b) const; int depth_to_mbar(int depth) const; double depth_to_mbarf(int depth) const; @@ -97,6 +98,9 @@ struct dive { pressure_t calculate_surface_pressure() const; pressure_t un_fixup_surface_pressure() const; + + /* Don't call directly, use dive_table::merge_dives()! */ + static std::unique_ptr create_merged_dive(const struct dive &a, const struct dive &b, int offset, bool prefer_downloaded); }; /* For the top-level list: an entry is either a dive or a trip */ @@ -184,14 +188,6 @@ extern bool dive_or_trip_less_than(struct dive_or_trip a, struct dive_or_trip b) extern int get_dive_salinity(const struct dive *dive); extern int dive_getUniqID(); -struct merge_result { - std::unique_ptr dive; - dive_trip *trip; - dive_site *site; -}; - -extern merge_result merge_dives(const struct dive &a, const struct dive &b, int offset, bool prefer_downloaded); -extern std::unique_ptr try_to_merge(const struct dive &a, const struct dive &b, bool prefer_downloaded); extern void copy_events_until(const struct dive *sd, struct dive *dd, int dcNr, int time); extern void copy_used_cylinders(const struct dive *s, struct dive *d, bool used_only); extern bool is_cylinder_used(const struct dive *dive, int idx); diff --git a/core/divelist.cpp b/core/divelist.cpp index f1af46208..9023d1d97 100644 --- a/core/divelist.cpp +++ b/core/divelist.cpp @@ -1149,3 +1149,124 @@ std::array, 2> dive_table::split_dive_at_time(const struct return {}; return split_dive_at(dive, static_cast(idx), static_cast(idx - 1)); } + +/* + * Pick a trip for a dive + */ +static struct dive_trip *get_preferred_trip(const struct dive *a, const struct dive *b) +{ + dive_trip *atrip, *btrip; + + /* If only one dive has a trip, choose that */ + atrip = a->divetrip; + btrip = b->divetrip; + if (!atrip) + return btrip; + if (!btrip) + return atrip; + + /* Both dives have a trip - prefer the non-autogenerated one */ + if (atrip->autogen && !btrip->autogen) + return btrip; + if (!atrip->autogen && btrip->autogen) + return atrip; + + /* Otherwise, look at the trip data and pick the "better" one */ + if (atrip->location.empty()) + return btrip; + if (btrip->location.empty()) + return atrip; + if (atrip->notes.empty()) + return btrip; + if (btrip->notes.empty()) + return atrip; + + /* + * Ok, so both have location and notes. + * Pick the earlier one. + */ + if (a->when < b->when) + return atrip; + return btrip; +} + +/* + * 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 the same dive computer, we *know* the + * two must have a different start time, and "offset" is the relative + * time difference between the two. + * + * (b) two different dive computers that we might 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 might be different. + * + * If new dives are merged into the dive table, dive a is supposed to + * be the old dive and dive b is supposed to be the newly imported + * dive. If the flag "prefer_downloaded" is set, data of the latter + * will take priority over the former. + * + * The trip the new dive should be associated with (if any) is returned + * in the "trip" output parameter. + * + * The dive site the new dive should be added to (if any) is returned + * in the "dive_site" output parameter. + */ +merge_result dive_table::merge_dives(const struct dive &a_in, const struct dive &b_in, int offset, bool prefer_downloaded) const +{ + merge_result res = { }; + + const dive *a = &a_in; + const dive *b = &b_in; + if (is_dc_planner(&a->dcs[0])) + std::swap(a, b); + + res.dive = dive::create_merged_dive(*a, *b, offset, prefer_downloaded); + + /* The CNS values will be recalculated from the sample in fixup_dive() */ + res.dive->cns = res.dive->maxcns = 0; + + res.trip = get_preferred_trip(a, b); + + /* we take the first dive site, unless it's empty */ + res.site = a->dive_site && !a->dive_site->is_empty() ? a->dive_site : b->dive_site; + if (!dive_site_has_gps_location(res.site) && dive_site_has_gps_location(b->dive_site)) { + /* we picked the first dive site and that didn't have GPS data, but the new dive has + * GPS data (that could be a download from a GPS enabled dive computer). + * Keep the dive site, but add the GPS data */ + res.site->location = b->dive_site->location; + } + fixup_dive(*res.dive); + + return res; +} + +/* + * This could do a lot more merging. Right now it really only + * merges almost exact duplicates - something that happens easily + * with overlapping dive downloads. + * + * If new dives are merged into the dive table, dive a is supposed to + * be the old dive and dive b is supposed to be the newly imported + * dive. If the flag "prefer_downloaded" is set, data of the latter + * will take priority over the former. + * + * Attn: The dive_site parameter of the dive will be set, but the caller + * still has to register the dive in the dive site! + */ +struct std::unique_ptr dive_table::try_to_merge(const struct dive &a, const struct dive &b, bool prefer_downloaded) const +{ + if (!a.likely_same(b)) + return {}; + + auto [res, trip, site] = merge_dives(a, b, 0, prefer_downloaded); + res->dive_site = site; /* Caller has to call site->add_dive()! */ + return std::move(res); +} diff --git a/core/divelist.h b/core/divelist.h index 1e96c7040..fc267b62c 100644 --- a/core/divelist.h +++ b/core/divelist.h @@ -15,6 +15,12 @@ struct deco_state; int comp_dives(const struct dive &a, const struct dive &b); int comp_dives_ptr(const struct dive *a, const struct dive *b); +struct merge_result { + std::unique_ptr dive; + dive_trip *trip; + dive_site *site; +}; + struct dive_table : public sorted_owning_table { dive *get_by_uniq_id(int id) const; void record_dive(std::unique_ptr d); // call fixup_dive() before adding dive to table. @@ -35,6 +41,8 @@ struct dive_table : public sorted_owning_table { std::array, 2> split_divecomputer(const struct dive &src, int num) const; std::array, 2> split_dive(const struct dive &dive) const; std::array, 2> split_dive_at_time(const struct dive &dive, duration_t time) const; + merge_result merge_dives(const struct dive &a_in, const struct dive &b_in, int offset, bool prefer_downloaded) const; + std::unique_ptr try_to_merge(const struct dive &a, const struct dive &b, bool prefer_downloaded) const; private: int calculate_cns(struct dive &dive) const; // Note: writes into dive->cns std::array, 2> split_dive_at(const struct dive &dive, int a, int b) const; diff --git a/core/divelog.cpp b/core/divelog.cpp index bdbdda4d2..fd7c2c7e8 100644 --- a/core/divelog.cpp +++ b/core/divelog.cpp @@ -110,7 +110,7 @@ static void merge_imported_dives(struct dive_table &table) prev->endtime() < dive->when) continue; - auto merged = try_to_merge(*prev, *dive, false); + auto merged = table.try_to_merge(*prev, *dive, false); if (!merged) continue; @@ -175,7 +175,7 @@ static bool try_to_merge_into(struct dive &dive_to_add, struct dive *old_dive, b /* output parameters: */ struct dive_table &dives_to_add, struct std::vector &dives_to_remove) { - auto merged = try_to_merge(*old_dive, dive_to_add, prefer_imported); + auto merged = dives_to_add.try_to_merge(*old_dive, dive_to_add, prefer_imported); if (!merged) return false;