[PATCH] Try to sanely download multiple concurrent cylinder pressures

Linus Torvalds torvalds at linux-foundation.org
Mon Jul 24 12:09:31 PDT 2017


From: Linus Torvalds <torvalds at linux-foundation.org>
Date: Mon, 24 Jul 2017 11:55:47 -0700
Subject: [PATCH] Try to sanely download multiple concurrent cylinder pressures

This tries to sanely handle the case of a dive computer reporting
multiple cylinder pressures concurrently.

NOTE! There are various "interesting" situations that this whole issue
brings up:

 - some dive computers may report more cylinder pressures than we have
   slots for.

   Currently we will drop such pressures on the floor if they come for
   the same sample, but if they end up being spread across multiple
   samples we will end up re-using the slots with different sensor
   indexes.

   That kind of slot re-use may or may not end up confusing other
   subsurface logic - for example, make things believe there was a
   cylidner change event.

 - some dive computers might send only one sample at a time, but switch
   *which* sample they send on a gas switch event.  If they also report
   the correct sensor number, we'll now start reporting that pressure in
   the second slot.

   This should all be fine, and is the RightThing(tm) to do, but is
   different from what we used to do when we only ever used a single
   slot.

 - When people actually use multiple sensors, our old save format will
   start to need fixing.  Right now our save format comes from the CCR
   model where the second sensor was always the Oxygen sensor.

   We save that pressure fine (except we save it as "o2pressure" - just
   an odd historical naming artifact), but we do *not* save the actual
   sensor index, because in our traditional format that was always
   implicit in the data ("it's the oxygen cylinder").

so while this code hopefully makes our libdivecomputer download do the
right thing, there *will* be further fallout from having multiple
cylinder pressure sensors.  We're not done yet.

Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
---

Please note the NOTE part above. I have basically reached the point where 
I no longer can even test that I'm doing the right thing, because I simply 
don't have any dive computers with multiple pressure sensors. 

I have been able to test CCR data, and some fake XML multi-sensor data, 
but it's not "real". 

I think we'll need to create some test data with Dirk, but we only have 
two Suunto transmitters between us to begin with, and mine is already off 
for the recall.

We do have three transmitters total for the Shearwater, I think, but that 
case is afaik limited to only tracking two pressures at a time, so we 
can't see that "more than two cylinder pressures" case at all right now. 
Although maybe together with tank switch events we could get something.

Anyway - what I'm trying to say is that this patch really looks like the 
right thing to do, but it doesn't fully solve the known issues that people 
*will* hit with multiple transmitters. 

It probably works fairly well in practice, though. Even the "hardcoded to 
O2 cylinder" save format thing should just work in practice because if you 
have two sensors and two tanks, the default mapping will just happen to 
work.

So this *may* actually make peoples sidemount cases work. 

And if you really do have three sensors (for example, two for sidemount, 
one for deco bottle), I will ask you to send me a libdivecomputer logfile 
and dump file of your dive, and maybe I can recreate it and actually make 
MAX_SENSORS be 4 or something.

 core/dive.c            |  7 +++++++
 core/dive.h            |  5 +++--
 core/libdivecomputer.c | 51 ++++++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/core/dive.c b/core/dive.c
index 6406242b..8270c547 100644
--- a/core/dive.c
+++ b/core/dive.c
@@ -648,6 +648,13 @@ struct sample *prepare_sample(struct divecomputer *dc)
 		}
 		sample = dc->sample + nr;
 		memset(sample, 0, sizeof(*sample));
+
+		// Copy the sensor numbers - but not the pressure values
+		// from the previous sample if any.
+		if (nr) {
+			sample->sensor[0] = sample[-1].sensor[0];
+			sample->sensor[1] = sample[-1].sensor[1];
+		}
 		return sample;
 	}
 	return NULL;
diff --git a/core/dive.h b/core/dive.h
index 2d1b761f..df0c4107 100644
--- a/core/dive.h
+++ b/core/dive.h
@@ -184,6 +184,7 @@ static inline int interpolate(int a, int b, int part, int whole)
 void get_gas_string(const struct gasmix *gasmix, char *text, int len);
 const char *gasname(const struct gasmix *gasmix);
 
+#define MAX_SENSORS 2
 struct sample                         // BASE TYPE BYTES  UNITS    RANGE      DESCRIPTION
 {                                     // --------- -----  -----    -----      -----------
 	duration_t time;               // uint32_t   4  seconds  (0-68 yrs)   elapsed dive time up to this sample
@@ -194,11 +195,11 @@ struct sample                         // BASE TYPE BYTES  UNITS    RANGE      DE
 	depth_t depth;                 // int32_t    4    mm     (0-2000 km)  dive depth of this sample
 	depth_t stopdepth;             // int32_t    4    mm     (0-2000 km)  depth of next deco stop
 	temperature_t temperature;     // int32_t    4  mdegrK   (0-2 MdegK)  ambient temperature
-	pressure_t pressure[2];        // int32_t    4    mbar   (0-2 Mbar)   cylinder pressures (main and CCR o2)
+	pressure_t pressure[MAX_SENSORS]; // int32_t    4    mbar   (0-2 Mbar)   cylinder pressures (main and CCR o2)
 	o2pressure_t setpoint;         // uint16_t   2    mbar   (0-65 bar)   O2 partial pressure (will be setpoint)
 	o2pressure_t o2sensor[3];      // uint16_t   6    mbar   (0-65 bar)   Up to 3 PO2 sensor values (rebreather)
 	bearing_t bearing;             // int16_t    2  degrees  (-32k to 32k deg) compass bearing
-	uint8_t sensor[2];             // uint8_t    1  sensorID (0-255)      ID of cylinder pressure sensor
+	uint8_t sensor[MAX_SENSORS];   // uint8_t    1  sensorID (0-255)      ID of cylinder pressure sensor
 	uint8_t cns;                   // uint8_t    1     %     (0-255 %)    cns% accumulated
 	uint8_t heartbeat;             // uint8_t    1  beats/m  (0-255)      heart rate measurement
 	volume_t sac;                  //            4  ml/min                predefined SAC
diff --git a/core/libdivecomputer.c b/core/libdivecomputer.c
index c935ecbc..819db022 100644
--- a/core/libdivecomputer.c
+++ b/core/libdivecomputer.c
@@ -307,6 +307,47 @@ static void handle_gasmix(struct divecomputer *dc, struct sample *sample, int id
 	current_gas_index = idx;
 }
 
+/*
+ * Adding a cylinder pressure sample field is not quite as trivial as it
+ * perhaps should be.
+ *
+ * We try to keep the same sensor index for the same sensor, so that even
+ * if the dive computer doesn't give pressure information for every sample,
+ * we don't move pressure information around between the different sensor
+ * indexes.
+ *
+ * The "prepare_sample()" function will always copy the sensor indices
+ * from the previous sample, so the indexes are pre-populated (but the
+ * pressures obviously are not)
+ */
+static void add_sample_pressure(struct sample *sample, int sensor, int mbar)
+{
+	int idx;
+
+	if (!mbar)
+		return;
+
+	/* Do we already have a slot for this sensor */
+	for (idx = 0; idx < MAX_SENSORS; idx++) {
+		if (sensor != sample->sensor[idx])
+			continue;
+		sample->pressure[idx].mbar = mbar;
+		return;
+	}
+
+	/* Pick the first unused index if we couldn't reuse one */
+	for (idx = 0; idx < MAX_SENSORS; idx++) {
+		if (sample->pressure[idx].mbar)
+			continue;
+		sample->sensor[idx] = sensor;
+		sample->pressure[idx].mbar = mbar;
+		return;
+	}
+
+	/* We do not have enough slots for the pressure samples. */
+	/* Should we warn the user about dropping pressure data? */
+}
+
 void
 sample_cb(dc_sample_type_t type, dc_sample_value_t value, void *userdata)
 {
@@ -352,15 +393,9 @@ sample_cb(dc_sample_type_t type, dc_sample_value_t value, void *userdata)
 	case DC_SAMPLE_DEPTH:
 		sample->depth.mm = lrint(value.depth * 1000);
 		break;
-	case DC_SAMPLE_PRESSURE: {
-		int sensoridx = 0;
-		/* Do we already have a pressure reading? */
-		if (sample->pressure[0].mbar)
-			sensoridx = 1;
-		sample->sensor[sensoridx] = value.pressure.tank;
-		sample->pressure[sensoridx].mbar = lrint(value.pressure.value * 1000);
+	case DC_SAMPLE_PRESSURE:
+		add_sample_pressure(sample, value.pressure.tank, lrint(value.pressure.value * 1000));
 		break;
-	}
 	case DC_SAMPLE_GASMIX:
 		handle_gasmix(dc, sample, value.gasmix);
 		break;
-- 
2.13.1.518.g0d864c4df



More information about the subsurface mailing list