[PATCH 1/2] Split up fixup_dive_dc() into multiple smaller independent functions
Linus Torvalds
torvalds at linux-foundation.org
Fri Apr 1 13:35:55 PDT 2016
From: Linus Torvalds <torvalds at linux-foundation.org>
Date: Fri, 1 Apr 2016 14:32:56 -0500
Subject: [PATCH 1/2] Split up fixup_dive_dc() into multiple smaller independent functions
fixup_dive_dc() is called for each dive computer when we add a new dive.
It does various housekeeping functions, cleaning up the sample data, and
fixing up dive details as a result of the sample data.
The function has grown to be a monster over time, and particularly the
central "walk every sample" loop has become an unreadable mess.
And the thing is, this isn't even all that performance-critical: it's
only done once per dive and dc, and there is no reason to have a single
illegible and complex loop.
So split up that loop into several smaller pieces that each will loop
individually over the sample data, and do just one thing. So now we
have separate functions for
- fixing up the depth samples with interpolation
- fixing up dive temperature data
- correcting the cylinder pressure sensor index
- cleaning up the actual sample pressures
Yes, this way we walk the samples multiple times, but the end result is
that the code is much easier to understand. There should be no actual
behavioral differences from this cleanup, except for the fact that since
the code is much more understandable, this cleanup also fixed a bug:
In the temperature fixup, we would fix up the overall dive temperatures
based on the dive computer temperatures. But we would then fix up the
overall dive computer temperature based on the sample temperature
*afterwards*, which wouldn't then be reflected in the overall dive
temperatures.
There was another non-symptomatic bug that became obvious when doing
this cleanup: the code used to calculate a 'depthtime' over the dive
that was never actually used. That's a historical artifact of old code
that had become dead when the average depth calculations were moved to a
function of their own earlier.
This is preparatory for fixing the overall cylinder pressure stats,
which are currently wrong for dives with multiple dive computers: we
currently take the starting cylinder pressure from the *first* dive
computer that has cylinder pressure information, but we take the ending
cylinder pressure from the *last* dive computer with cylinder pressure
information.
This does not fix that bug, but without this cleanup fixing that would
be a nightmare due to the previous complicated "do everything in one
single loop" model.
Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
---
subsurface-core/dive.c | 189 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 126 insertions(+), 63 deletions(-)
diff --git a/subsurface-core/dive.c b/subsurface-core/dive.c
index 8fc9e993be03..6ee002e1f8f1 100644
--- a/subsurface-core/dive.c
+++ b/subsurface-core/dive.c
@@ -1262,47 +1262,105 @@ static int interpolate_depth(struct divecomputer *dc, int idx, int lastdepth, in
return interpolate(lastdepth, nextdepth, now-lasttime, nexttime-lasttime);
}
-static void fixup_dive_dc(struct dive *dive, struct divecomputer *dc)
+static void fixup_dc_depths(struct dive *dive, struct divecomputer *dc)
{
- int i, j;
- double depthtime = 0;
- int lasttime = 0;
- int lastindex = -1;
+ int i;
int maxdepth = dc->maxdepth.mm;
- int mintemp = 0;
- int lastdepth = 0;
- int lasttemp = 0;
- int lastpressure = 0, lasto2pressure = 0;
- int pressure_delta[MAX_CYLINDERS] = { INT_MAX, };
- int first_cylinder;
-
- /* Add device information to table */
- if (dc->deviceid && (dc->serial || dc->fw_version))
- create_device_node(dc->model, dc->deviceid, dc->serial, dc->fw_version, "");
-
- /* Fixup duration and mean depth */
- fixup_dc_duration(dc);
- update_min_max_temperatures(dive, dc->watertemp);
+ int lasttime = 0, lastdepth = 0;
- /* make sure we know for which tank the pressure values are intended */
- first_cylinder = explicit_first_cylinder(dive, dc);
for (i = 0; i < dc->samples; i++) {
struct sample *sample = dc->sample + i;
int time = sample->time.seconds;
int depth = sample->depth.mm;
- int temp = sample->temperature.mkelvin;
- int pressure = sample->cylinderpressure.mbar;
- int o2_pressure = sample->o2cylinderpressure.mbar;
- int index;
if (depth < 0) {
depth = interpolate_depth(dc, i, lastdepth, lasttime, time);
sample->depth.mm = depth;
}
- /* if we have an explicit first cylinder */
- if (sample->sensor == 0 && first_cylinder != 0)
+ if (depth > SURFACE_THRESHOLD) {
+ if (depth > maxdepth)
+ maxdepth = depth;
+ }
+
+ lastdepth = depth;
+ lasttime = time;
+ if (sample->cns > dive->maxcns)
+ dive->maxcns = sample->cns;
+ }
+
+ update_depth(&dc->maxdepth, maxdepth);
+ if (maxdepth > dive->maxdepth.mm)
+ dive->maxdepth.mm = maxdepth;
+}
+
+static void fixup_dc_temp(struct dive *dive, struct divecomputer *dc)
+{
+ int i;
+ int mintemp = 0, lasttemp = 0;
+
+ for (i = 0; i < dc->samples; i++) {
+ struct sample *sample = dc->sample + i;
+ int temp = sample->temperature.mkelvin;
+
+ if (temp) {
+ /*
+ * If we have consecutive identical
+ * temperature readings, throw away
+ * the redundant ones.
+ */
+ if (lasttemp == temp)
+ sample->temperature.mkelvin = 0;
+ else
+ lasttemp = temp;
+
+ if (!mintemp || temp < mintemp)
+ mintemp = temp;
+ }
+
+ update_min_max_temperatures(dive, sample->temperature);
+ }
+ update_temperature(&dc->watertemp, mintemp);
+ update_min_max_temperatures(dive, dc->watertemp);
+}
+
+/*
+ * Fix up cylinder sensor information in the samples if we have
+ * an explicit first cylinder
+ */
+static void fixup_dc_cylinder_index(struct dive *dive, struct divecomputer *dc)
+{
+ int i;
+ int first_cylinder = explicit_first_cylinder(dive, dc);
+
+ if (!first_cylinder)
+ return;
+
+ for (i = 0; i < dc->samples; i++) {
+ struct sample *sample = dc->sample + i;
+
+ if (sample->sensor == 0)
sample->sensor = first_cylinder;
+ }
+}
+
+/*
+ * Simplify dc pressure information:
+ * (a) Remove redundant pressure information
+ * (b) Remove linearly interpolated pressure data
+ */
+static void simplify_dc_pressures(struct dive *dive, struct divecomputer *dc)
+{
+ int i, j;
+ int lastindex = -1;
+ int lastpressure = 0, lasto2pressure = 0;
+ int pressure_delta[MAX_CYLINDERS] = { INT_MAX, };
+
+ for (i = 0; i < dc->samples; i++) {
+ struct sample *sample = dc->sample + i;
+ int pressure = sample->cylinderpressure.mbar;
+ int o2_pressure = sample->o2cylinderpressure.mbar;
+ int index;
index = sample->sensor;
@@ -1330,38 +1388,6 @@ static void fixup_dive_dc(struct dive *dive, struct divecomputer *dc)
lastindex = index;
lastpressure = pressure;
lasto2pressure = o2_pressure;
-
- if (depth > SURFACE_THRESHOLD) {
- if (depth > maxdepth)
- maxdepth = depth;
- }
-
- fixup_pressure(dive, sample, OC_GAS);
- if (dive->dc.divemode == CCR)
- fixup_pressure(dive, sample, OXYGEN);
-
- if (temp) {
- /*
- * If we have consecutive identical
- * temperature readings, throw away
- * the redundant ones.
- */
- if (lasttemp == temp)
- sample->temperature.mkelvin = 0;
- else
- lasttemp = temp;
-
- if (!mintemp || temp < mintemp)
- mintemp = temp;
- }
-
- update_min_max_temperatures(dive, sample->temperature);
-
- depthtime += (time - lasttime) * (lastdepth + depth) / 2;
- lastdepth = depth;
- lasttime = time;
- if (sample->cns > dive->maxcns)
- dive->maxcns = sample->cns;
}
/* if all the samples for a cylinder have pressure data that
@@ -1391,11 +1417,48 @@ static void fixup_dive_dc(struct dive *dive, struct divecomputer *dc)
cyl->sample_end.mbar = 0;
}
}
+}
+
+/*
+ * Check the cylinder pressure sample information and fill in the
+ * overall cylinder pressures from those.
+ */
+static void fixup_dive_pressures(struct dive *dive, struct divecomputer *dc)
+{
+ int i;
+ for (i = 0; i < dc->samples; i++) {
+ struct sample *sample = dc->sample + i;
+ fixup_pressure(dive, sample, OC_GAS);
+ if (dive->dc.divemode == CCR)
+ fixup_pressure(dive, sample, OXYGEN);
+ }
+
+ simplify_dc_pressures(dive, dc);
+}
+
+static void fixup_dive_dc(struct dive *dive, struct divecomputer *dc)
+{
+ int i;
+
+ /* Add device information to table */
+ if (dc->deviceid && (dc->serial || dc->fw_version))
+ create_device_node(dc->model, dc->deviceid, dc->serial, dc->fw_version, "");
+
+ /* Fixup duration and mean depth */
+ fixup_dc_duration(dc);
+
+ /* Fix up sample depth data */
+ fixup_dc_depths(dive, dc);
+
+ /* Fix up dive temperatures based on dive computer samples */
+ fixup_dc_temp(dive, dc);
+
+ /* Fix up cylinder sensor data */
+ fixup_dc_cylinder_index(dive, dc);
+
+ /* Fix up cylinder pressures based on DC info */
+ fixup_dive_pressures(dive, dc);
- update_temperature(&dc->watertemp, mintemp);
- update_depth(&dc->maxdepth, maxdepth);
- if (maxdepth > dive->maxdepth.mm)
- dive->maxdepth.mm = maxdepth;
fixup_dc_events(dc);
}
--
2.8.0.rc4.303.g3931579
More information about the subsurface
mailing list