From 701a7813a349e658a9348f3db6763fe6d6f28dbf Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sat, 14 Dec 2024 18:36:22 +0100 Subject: [PATCH] core: refactor per_cylinder_mean_depth() This function had a horrendous interface: The caller would have to allocate two arrays of the correct size to be filled with data. The callee couldn't even check the size, because the data was passed as raw pointers. Instead, use std::vector<>, construct everything in the called function and do size-sanity check in the calling function. Use depth_t and duration_t instead of plain integers to represent mean depth and time. Signed-off-by: Berthold Stoeger --- core/dive.cpp | 82 ++++++++++--------- core/dive.h | 7 +- .../tab-widgets/TabDiveInformation.cpp | 14 ++-- 3 files changed, 54 insertions(+), 49 deletions(-) diff --git a/core/dive.cpp b/core/dive.cpp index 8c7b23932..721b4f2f5 100644 --- a/core/dive.cpp +++ b/core/dive.cpp @@ -367,89 +367,93 @@ static bool has_unknown_used_cylinders(const struct dive &dive, const struct div return num > 0; } -void per_cylinder_mean_depth(const struct dive *dive, struct divecomputer *dc, int *mean, int *duration) +std::vector dive::per_cylinder_mean_depth_and_duration(int dc_nr) const { - int num_used_cylinders; + const struct divecomputer *dc = get_dc(dc_nr); - if (dive->cylinders.empty()) - return; + std::vector res; + if (cylinders.empty()) + return res; - for (size_t i = 0; i < dive->cylinders.size(); i++) - mean[i] = duration[i] = 0; - if (!dc) - return; + res.resize(cylinders.size()); /* * There is no point in doing per-cylinder information * if we don't actually know about the usage of all the * used cylinders. */ - auto used_cylinders = std::make_unique(dive->cylinders.size()); - num_used_cylinders = get_cylinder_used(dive, used_cylinders.get()); - if (has_unknown_used_cylinders(*dive, dc, used_cylinders.get(), num_used_cylinders)) { + auto used_cylinders = std::make_unique(cylinders.size()); + int num_used_cylinders = get_cylinder_used(this, used_cylinders.get()); + if (has_unknown_used_cylinders(*this, dc, used_cylinders.get(), num_used_cylinders)) { /* * If we had more than one used cylinder, but * do not know usage of them, we simply cannot * account mean depth to them. */ if (num_used_cylinders > 1) - return; + return res; /* * For a single cylinder, use the overall mean * and duration */ - for (size_t i = 0; i < dive->cylinders.size(); i++) { + for (size_t i = 0; i < cylinders.size(); i++) { if (used_cylinders[i]) { - mean[i] = dc->meandepth.mm; - duration[i] = dc->duration.seconds; + res[i].depth = dc->meandepth; + res[i].duration = dc->duration; } } - return; + return res; } - if (dc->samples.empty()) - fake_dc(dc); - gasmix_loop loop(*dive, *dc); - std::vector depthtime(dive->cylinders.size(), 0); - int lasttime = 0; - int lastdepth = 0; + // TODO: Wow: the old code was super sketchy: It would "fake" a dc here of all places if there were no samples. + // Let's comment this out for now and fix the root-cause if this ever gives a problem. + //if (dc->samples.empty()) + // fake_dc(dc); + + // No samples - something is wrong. Give up. + if (dc->samples.empty()) + return res; + + gasmix_loop loop(*this, *dc); + std::vector depthtime(cylinders.size(), 0); // mm × seconds + duration_t lasttime; + depth_t lastdepth; int last_cylinder_index = -1; std::pair gaschange_event; for (auto it = dc->samples.begin(); it != dc->samples.end(); ++it) { - int32_t time = it->time.seconds; - int depth = it->depth.mm; - /* Make sure to move the event past 'lasttime' */ - gaschange_event = loop.cylinder_index_at(lasttime); + gaschange_event = loop.cylinder_index_at(lasttime.seconds); /* Do we need to fake a midway sample? */ if (last_cylinder_index >= 0 && last_cylinder_index != gaschange_event.first) { - int newdepth = interpolate(lastdepth, depth, gaschange_event.second - lasttime, time - lasttime); - if (newdepth > SURFACE_THRESHOLD || lastdepth > SURFACE_THRESHOLD) { - duration[gaschange_event.first] += gaschange_event.second - lasttime; - depthtime[gaschange_event.first] += (gaschange_event.second - lasttime) * (newdepth + lastdepth) / 2; + depth_t newdepth = interpolate(lastdepth, it->depth, gaschange_event.second - lasttime.seconds, (it->time - lasttime).seconds); + if (newdepth.mm > SURFACE_THRESHOLD || lastdepth.mm > SURFACE_THRESHOLD) { + res[gaschange_event.first].duration.seconds += gaschange_event.second - lasttime.seconds; + depthtime[gaschange_event.first] += (gaschange_event.second - lasttime.seconds) * (newdepth.mm + lastdepth.mm) / 2; } - lasttime = gaschange_event.second; + lasttime.seconds = gaschange_event.second; lastdepth = newdepth; } /* We ignore segments at the surface */ - if (depth > SURFACE_THRESHOLD || lastdepth > SURFACE_THRESHOLD) { - duration[gaschange_event.first] += time - lasttime; - depthtime[gaschange_event.first] += (time - lasttime) * (depth + lastdepth) / 2; + if (it->depth.mm > SURFACE_THRESHOLD || lastdepth.mm > SURFACE_THRESHOLD) { + res[gaschange_event.first].duration += it->time - lasttime; + depthtime[gaschange_event.first] += (it->time - lasttime).seconds * (it->depth + lastdepth).mm / 2; } - lastdepth = depth; - lasttime = time; + lastdepth = it->depth; + lasttime = it->time; last_cylinder_index = gaschange_event.first; } - for (size_t i = 0; i < dive->cylinders.size(); i++) { - if (duration[i]) - mean[i] = (depthtime[i] + duration[i] / 2) / duration[i]; + for (size_t i = 0; i < cylinders.size(); i++) { + if (res[i].duration.seconds) + res[i].depth.mm = (depthtime[i] + res[i].duration.seconds / 2) / res[i].duration.seconds; } + + return res; } static void update_min_max_temperatures(struct dive &dive, temperature_t temperature) diff --git a/core/dive.h b/core/dive.h index c2fc174d5..03a2bbd57 100644 --- a/core/dive.h +++ b/core/dive.h @@ -130,6 +130,12 @@ struct dive { fraction_t best_o2(depth_t depth, bool in_planner) const; fraction_t best_he(depth_t depth, bool o2narcotic, fraction_t fo2) const; + struct depth_duration { + depth_t depth; + duration_t duration; + }; + std::vector per_cylinder_mean_depth_and_duration(int dc_nr) const; + bool dive_has_gps_location() const; location_t get_gps_location() const; @@ -193,7 +199,6 @@ extern void copy_events_until(const struct dive *sd, struct dive *dd, int dcNr, extern void copy_used_cylinders(const struct dive *s, struct dive *d, bool used_only); extern void add_gas_switch_event(struct dive *dive, struct divecomputer *dc, int time, int idx); extern struct event create_gas_switch_event(struct dive *dive, struct divecomputer *dc, int seconds, int idx); -extern void per_cylinder_mean_depth(const struct dive *dive, struct divecomputer *dc, int *mean, int *duration); extern bool cylinder_with_sensor_sample(const struct dive *dive, int cylinder_id); extern void update_setpoint_events(const struct dive *dive, struct divecomputer *dc); diff --git a/desktop-widgets/tab-widgets/TabDiveInformation.cpp b/desktop-widgets/tab-widgets/TabDiveInformation.cpp index 64cc74607..b4ce8576d 100644 --- a/desktop-widgets/tab-widgets/TabDiveInformation.cpp +++ b/desktop-widgets/tab-widgets/TabDiveInformation.cpp @@ -126,15 +126,12 @@ void TabDiveInformation::updateProfile() std::vector gases = get_gas_used(currentDive); QString volumes; - std::vector mean(currentDive->cylinders.size()), duration(currentDive->cylinders.size()); - struct divecomputer *currentdc = parent.getCurrentDC(); - if (currentdc && !currentDive->cylinders.empty()) - per_cylinder_mean_depth(currentDive, currentdc, mean.data(), duration.data()); + auto mean = currentDive->per_cylinder_mean_depth_and_duration(parent.currentDC); volume_t sac; QString gaslist, SACs, separator; for (size_t i = 0; i < currentDive->cylinders.size(); i++) { - if (!currentDive->is_cylinder_used(i)) + if (!currentDive->is_cylinder_used(i) || i >= mean.size()) continue; gaslist.append(separator); volumes.append(separator); SACs.append(separator); separator = "\n"; @@ -143,9 +140,8 @@ void TabDiveInformation::updateProfile() if (!gases[i].mliter) continue; volumes.append(get_volume_string(gases[i], true)); - if (duration[i]) { - depth_t mean_depth = { .mm = mean[i] }; // Will be removed in upcoming commit - sac.mliter = lrint(gases[i].mliter / (currentDive->depth_to_atm(mean_depth) * duration[i] / 60)); + if (mean[i].duration.seconds > 0) { + sac.mliter = lrint(gases[i].mliter / (currentDive->depth_to_atm(mean[i].depth) * mean[i].duration.seconds / 60)); SACs.append(get_volume_string(sac, true).append(tr("/min"))); } } @@ -155,7 +151,7 @@ void TabDiveInformation::updateProfile() ui->diveTimeText->setText(get_dive_duration_string(currentDive->duration.seconds, tr("h"), tr("min"), tr("sec"), " ", currentDive->dcs[0].divemode == FREEDIVE)); - ui->sacText->setText(!currentDive->cylinders.empty() && mean[0] && currentDive->dcs[0].divemode != CCR ? std::move(SACs) : QString()); + ui->sacText->setText(!currentDive->cylinders.empty() && mean[0].depth.mm > 0 && currentDive->dcs[0].divemode != CCR ? std::move(SACs) : QString()); if (currentDive->surface_pressure.mbar == 0) { ui->atmPressVal->clear(); // If no atm pressure for dive then clear text box