On 2017-05-10 15:43, Anton Lundin wrote:
>That said, I think setpoint values are still interesting to see, to
>validate how well the controller did manage to try to keep the o2 close
>to the setpoint.
After looking at the data, I had the impression that the setpoint
value is "unused" because it seems to just contain some "dummy"
value (for example the last used value, or some default value).
I'll illustrate with an example dive from Steve's Petrel (*). This
dive has a fixed setpoint of 0.70 on every sample, but the ppo2
values range from 0.32 to 1.74!
This sounds like a mCCR. Then its up to the diver to press a button
until the ppo2 matches what the diver would like to have.
That makes sense!
(*) I can send you the data if you want to take a look. I don't know
if Steve is okay with sending his dives to a public mailinglist, so
I didn't attach it to this email.
To me that doesn't look like the dive computer is even trying to
keep the ppo2 close to the setpoint. At least not to the setpoint
value that's stored in the sample. Hence my question whether this
value is relevant or not?
I think so.
My guess is that the setpoint is what the computer will continue to use
as its ppo2 value if it looses the connection with the sensors.
Its better to expose the information to the user, and let the user
ignore/delete it if they don't care about it.
My only concern here is that if the info is useless, like those zero
millivolt values when external O2 sensor monitoring is disabled, the we
better don't report them at all. But if that's not the case, then it's
of course fine to keep reporting them.
>The only real comment about the code is that I would have liked to see
>the calibration factor kept as a int, and just change the unit factor
>from .00001 to .000022, between the models.
What would be the advantage of that? That would mean yet some other
field to store the scaling factor, or doing some "if (model ==
PREDATOR)" when calculating the ppo2. Now it's just done once in
advance, making the conversion from millivolt to ppO2 independent of
the model. I'm even tempted to pre-multiply the value with the
100000.0 factor too, to get rid of an extra
I'd store it as a separate calibration factor unit. Anyway, if you
multiply the calibration factor with 2.2 as of now, its better to
include the 1/100000 factor to, rather than having them in two separate
places.
I replaced your patch with your latest version, and updated the rest of
the series. Please have a look. If you are okay with the changes, I'll
merge them to master.
Jef
From 588e7e7ab459b5df181d7600b891adc7458d9902 Mon Sep 17 00:00:00 2001
From: Dirk Hohndel <[email protected]>
Date: Fri, 6 Feb 2015 09:56:21 -0800
Subject: [PATCH 1/5] Predator: don't report PPO2 unless in CC mode
Sending this in OC mode is redundant and might confuse applications that
assume they only get PPO2 data in CC mode.
Signed-off-by: Dirk Hohndel <[email protected]>
---
src/shearwater_predator_parser.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/shearwater_predator_parser.c b/src/shearwater_predator_parser.c
index 039926f..c71c9d7 100644
--- a/src/shearwater_predator_parser.c
+++ b/src/shearwater_predator_parser.c
@@ -422,11 +422,11 @@ shearwater_predator_parser_samples_foreach (dc_parser_t *abstract, dc_sample_cal
// Status flags.
unsigned int status = data[offset + 11];
- // PPO2
- sample.ppo2 = data[offset + 6] / 100.0;
- if (callback) callback (DC_SAMPLE_PPO2, sample, userdata);
-
if ((status & OC) == 0) {
+ // PPO2
+ sample.ppo2 = data[offset + 6] / 100.0;
+ if (callback) callback (DC_SAMPLE_PPO2, sample, userdata);
+
// Setpoint
if (parser->petrel) {
sample.setpoint = data[offset + 18] / 100.0;
--
2.11.0
From d3ca3e87bd92bce7a59818998825652e025e341f Mon Sep 17 00:00:00 2001
From: Anton Lundin <[email protected]>
Date: Wed, 14 Oct 2015 19:49:25 +0200
Subject: [PATCH 2/5] shearwater: Report individual sensor values
This reads the reported mV values from the sensors, and based on the
calibration values converts it into a ppo2 value to report.
Signed-off-by: Anton Lundin <[email protected]>
---
src/shearwater_predator_parser.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/src/shearwater_predator_parser.c b/src/shearwater_predator_parser.c
index c71c9d7..7c36651 100644
--- a/src/shearwater_predator_parser.c
+++ b/src/shearwater_predator_parser.c
@@ -61,6 +61,7 @@ struct shearwater_predator_parser_t {
unsigned int ngasmixes;
unsigned int oxygen[NGASMIXES];
unsigned int helium[NGASMIXES];
+ unsigned int calibration[3];
dc_divemode_t mode;
};
@@ -281,6 +282,18 @@ shearwater_predator_parser_cache (shearwater_predator_parser_t *parser)
offset += parser->samplesize;
}
+ // Cache sensor calibration for later use
+ parser->calibration[0] = array_uint16_be(data + 87);
+ parser->calibration[1] = array_uint16_be(data + 89);
+ parser->calibration[2] = array_uint16_be(data + 91);
+ // The Predator expects the mV output of the cells to be within 30mV
+ // to 70mV in 100% O2 at 1 atmosphere.
+ // If we add 1024 (1000?) to the calibration value, then the sensors
+ // lines up and matches the average.
+ parser->calibration[0] += 1024;
+ parser->calibration[1] += 1024;
+ parser->calibration[2] += 1024;
+
// Cache the data for later use.
parser->headersize = headersize;
parser->footersize = footersize;
@@ -424,8 +437,19 @@ shearwater_predator_parser_samples_foreach (dc_parser_t *abstract, dc_sample_cal
if ((status & OC) == 0) {
// PPO2
+#ifdef SENSOR_AVERAGE
sample.ppo2 = data[offset + 6] / 100.0;
if (callback) callback (DC_SAMPLE_PPO2, sample, userdata);
+#else
+ sample.ppo2 = data[offset + 12] * parser->calibration[0] / 100000.0;
+ if (callback && (data[86] & 0x01)) callback (DC_SAMPLE_PPO2, sample, userdata);
+
+ sample.ppo2 = data[offset + 14] * parser->calibration[1] / 100000.0;
+ if (callback && (data[86] & 0x02)) callback (DC_SAMPLE_PPO2, sample, userdata);
+
+ sample.ppo2 = data[offset + 15] * parser->calibration[2] / 100000.0;
+ if (callback && (data[86] & 0x04)) callback (DC_SAMPLE_PPO2, sample, userdata);
+#endif
// Setpoint
if (parser->petrel) {
--
2.11.0
From 8e71ed3c7b9d7cee961c0f1f2ea85e993397f8f7 Mon Sep 17 00:00:00 2001
From: Jef Driesen <[email protected]>
Date: Thu, 13 Apr 2017 21:26:56 +0200
Subject: [PATCH 3/5] Apply the calibration correction only for the Predator
The calibration values for the Petrel are typically in the range 1600 to
2400, while for Predator they are much smaller, with values in the range
800 to 1400. The consequence is that the calculated ppO2 values are too
low for the Predator. Adding a constant offset of about 1000 changes the
calibration value to be in approximately the same range as the Petrel,
and hence more reasonable ppO2 values. But this correction should only
be applied for the Predator, and not the Petrel.
---
src/parser.c | 4 ++--
src/shearwater_petrel.h | 2 +-
src/shearwater_predator.h | 2 +-
src/shearwater_predator_parser.c | 23 +++++++++++++++--------
4 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/src/parser.c b/src/parser.c
index 98d2c89..9fdb1a9 100644
--- a/src/parser.c
+++ b/src/parser.c
@@ -149,10 +149,10 @@ dc_parser_new_internal (dc_parser_t **out, dc_context_t *context, dc_family_t fa
rc = atomics_cobalt_parser_create (&parser, context);
break;
case DC_FAMILY_SHEARWATER_PREDATOR:
- rc = shearwater_predator_parser_create (&parser, context);
+ rc = shearwater_predator_parser_create (&parser, context, model);
break;
case DC_FAMILY_SHEARWATER_PETREL:
- rc = shearwater_petrel_parser_create (&parser, context);
+ rc = shearwater_petrel_parser_create (&parser, context, model);
break;
case DC_FAMILY_DIVERITE_NITEKQ:
rc = diverite_nitekq_parser_create (&parser, context);
diff --git a/src/shearwater_petrel.h b/src/shearwater_petrel.h
index 8b927d6..c23bb74 100644
--- a/src/shearwater_petrel.h
+++ b/src/shearwater_petrel.h
@@ -34,7 +34,7 @@ dc_status_t
shearwater_petrel_device_open (dc_device_t **device, dc_context_t *context, const char *name);
dc_status_t
-shearwater_petrel_parser_create (dc_parser_t **parser, dc_context_t *context);
+shearwater_petrel_parser_create (dc_parser_t **parser, dc_context_t *context, unsigned int model);
#ifdef __cplusplus
}
diff --git a/src/shearwater_predator.h b/src/shearwater_predator.h
index f21d445..4665f80 100644
--- a/src/shearwater_predator.h
+++ b/src/shearwater_predator.h
@@ -34,7 +34,7 @@ dc_status_t
shearwater_predator_device_open (dc_device_t **device, dc_context_t *context, const char *name);
dc_status_t
-shearwater_predator_parser_create (dc_parser_t **parser, dc_context_t *context);
+shearwater_predator_parser_create (dc_parser_t **parser, dc_context_t *context, unsigned int model);
#ifdef __cplusplus
}
diff --git a/src/shearwater_predator_parser.c b/src/shearwater_predator_parser.c
index 7c36651..58d1704 100644
--- a/src/shearwater_predator_parser.c
+++ b/src/shearwater_predator_parser.c
@@ -48,10 +48,14 @@
#define NGASMIXES 10
+#define PREDATOR 2
+#define PETREL 3
+
typedef struct shearwater_predator_parser_t shearwater_predator_parser_t;
struct shearwater_predator_parser_t {
dc_parser_t base;
+ unsigned int model;
unsigned int petrel;
unsigned int samplesize;
// Cached fields.
@@ -106,7 +110,7 @@ shearwater_predator_find_gasmix (shearwater_predator_parser_t *parser, unsigned
static dc_status_t
-shearwater_common_parser_create (dc_parser_t **out, dc_context_t *context, unsigned int petrel)
+shearwater_common_parser_create (dc_parser_t **out, dc_context_t *context, unsigned int model, unsigned int petrel)
{
shearwater_predator_parser_t *parser = NULL;
const dc_parser_vtable_t *vtable = NULL;
@@ -131,6 +135,7 @@ shearwater_common_parser_create (dc_parser_t **out, dc_context_t *context, unsig
}
// Set the default values.
+ parser->model = model;
parser->petrel = petrel;
parser->samplesize = samplesize;
parser->cached = 0;
@@ -150,16 +155,16 @@ shearwater_common_parser_create (dc_parser_t **out, dc_context_t *context, unsig
dc_status_t
-shearwater_predator_parser_create (dc_parser_t **out, dc_context_t *context)
+shearwater_predator_parser_create (dc_parser_t **out, dc_context_t *context, unsigned int model)
{
- return shearwater_common_parser_create (out, context, 0);
+ return shearwater_common_parser_create (out, context, model, 0);
}
dc_status_t
-shearwater_petrel_parser_create (dc_parser_t **out, dc_context_t *context)
+shearwater_petrel_parser_create (dc_parser_t **out, dc_context_t *context, unsigned int model)
{
- return shearwater_common_parser_create (out, context, 1);
+ return shearwater_common_parser_create (out, context, model, 1);
}
@@ -290,9 +295,11 @@ shearwater_predator_parser_cache (shearwater_predator_parser_t *parser)
// to 70mV in 100% O2 at 1 atmosphere.
// If we add 1024 (1000?) to the calibration value, then the sensors
// lines up and matches the average.
- parser->calibration[0] += 1024;
- parser->calibration[1] += 1024;
- parser->calibration[2] += 1024;
+ if (parser->model == PREDATOR) {
+ parser->calibration[0] += 1024;
+ parser->calibration[1] += 1024;
+ parser->calibration[2] += 1024;
+ }
// Cache the data for later use.
parser->headersize = headersize;
--
2.11.0
From fdc1c078a60748875b3f9c126bca6817c0a9e261 Mon Sep 17 00:00:00 2001
From: Jef Driesen <[email protected]>
Date: Thu, 13 Apr 2017 21:26:56 +0200
Subject: [PATCH 4/5] Replace the constant offset with a scaling factor
Correcting the Predator calibration value with a scaling factor produces
even more reasonable ppO2 values compared to using a constant offset.
The scaling factor of 2.2 is based on a linear regression between the
average ppO2 reported by the dive computer, and the average ppO2
calculated over all (calibrated) sensors using the raw calibration
value.
---
src/shearwater_predator_parser.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/src/shearwater_predator_parser.c b/src/shearwater_predator_parser.c
index 58d1704..3e5fcbf 100644
--- a/src/shearwater_predator_parser.c
+++ b/src/shearwater_predator_parser.c
@@ -65,7 +65,7 @@ struct shearwater_predator_parser_t {
unsigned int ngasmixes;
unsigned int oxygen[NGASMIXES];
unsigned int helium[NGASMIXES];
- unsigned int calibration[3];
+ double calibration[3];
dc_divemode_t mode;
};
@@ -288,17 +288,17 @@ shearwater_predator_parser_cache (shearwater_predator_parser_t *parser)
}
// Cache sensor calibration for later use
- parser->calibration[0] = array_uint16_be(data + 87);
- parser->calibration[1] = array_uint16_be(data + 89);
- parser->calibration[2] = array_uint16_be(data + 91);
+ parser->calibration[0] = array_uint16_be(data + 87) / 100000.0;
+ parser->calibration[1] = array_uint16_be(data + 89) / 100000.0;
+ parser->calibration[2] = array_uint16_be(data + 91) / 100000.0;
// The Predator expects the mV output of the cells to be within 30mV
// to 70mV in 100% O2 at 1 atmosphere.
- // If we add 1024 (1000?) to the calibration value, then the sensors
- // lines up and matches the average.
+ // If the calibration value is scaled with a factor 2.2, then the
+ // sensors lines up and matches the average.
if (parser->model == PREDATOR) {
- parser->calibration[0] += 1024;
- parser->calibration[1] += 1024;
- parser->calibration[2] += 1024;
+ for (size_t i = 0; i < 3; ++i) {
+ parser->calibration[i] *= 2.2;
+ }
}
// Cache the data for later use.
@@ -448,14 +448,15 @@ shearwater_predator_parser_samples_foreach (dc_parser_t *abstract, dc_sample_cal
sample.ppo2 = data[offset + 6] / 100.0;
if (callback) callback (DC_SAMPLE_PPO2, sample, userdata);
#else
- sample.ppo2 = data[offset + 12] * parser->calibration[0] / 100000.0;
+ sample.ppo2 = data[offset + 12] * parser->calibration[0];
if (callback && (data[86] & 0x01)) callback (DC_SAMPLE_PPO2, sample, userdata);
- sample.ppo2 = data[offset + 14] * parser->calibration[1] / 100000.0;
+ sample.ppo2 = data[offset + 14] * parser->calibration[1];
if (callback && (data[86] & 0x02)) callback (DC_SAMPLE_PPO2, sample, userdata);
- sample.ppo2 = data[offset + 15] * parser->calibration[2] / 100000.0;
+ sample.ppo2 = data[offset + 15] * parser->calibration[2];
if (callback && (data[86] & 0x04)) callback (DC_SAMPLE_PPO2, sample, userdata);
+ }
#endif
// Setpoint
--
2.11.0
From 6a938d18c02c1081cbf82ebdd8cc4e316f1b597c Mon Sep 17 00:00:00 2001
From: Jef Driesen <[email protected]>
Date: Mon, 8 May 2017 19:24:32 +0200
Subject: [PATCH 5/5] Report the ppO2 in external O2 sensor mode only
The O2 sensor millivolt values are only valid if external O2 sensor
monitoring is enabled.
Note that the interpretation of the PPO2 status bit appears to be
reversed (0=external and 1=internal).
---
src/shearwater_predator_parser.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/shearwater_predator_parser.c b/src/shearwater_predator_parser.c
index 3e5fcbf..e922220 100644
--- a/src/shearwater_predator_parser.c
+++ b/src/shearwater_predator_parser.c
@@ -448,14 +448,15 @@ shearwater_predator_parser_samples_foreach (dc_parser_t *abstract, dc_sample_cal
sample.ppo2 = data[offset + 6] / 100.0;
if (callback) callback (DC_SAMPLE_PPO2, sample, userdata);
#else
- sample.ppo2 = data[offset + 12] * parser->calibration[0];
- if (callback && (data[86] & 0x01)) callback (DC_SAMPLE_PPO2, sample, userdata);
+ if ((status & PPO2_EXTERNAL) == 0) {
+ sample.ppo2 = data[offset + 12] * parser->calibration[0];
+ if (callback && (data[86] & 0x01)) callback (DC_SAMPLE_PPO2, sample, userdata);
- sample.ppo2 = data[offset + 14] * parser->calibration[1];
- if (callback && (data[86] & 0x02)) callback (DC_SAMPLE_PPO2, sample, userdata);
+ sample.ppo2 = data[offset + 14] * parser->calibration[1];
+ if (callback && (data[86] & 0x02)) callback (DC_SAMPLE_PPO2, sample, userdata);
- sample.ppo2 = data[offset + 15] * parser->calibration[2];
- if (callback && (data[86] & 0x04)) callback (DC_SAMPLE_PPO2, sample, userdata);
+ sample.ppo2 = data[offset + 15] * parser->calibration[2];
+ if (callback && (data[86] & 0x04)) callback (DC_SAMPLE_PPO2, sample, userdata);
}
#endif
--
2.11.0
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
