Re: [CCR PATCH] reorganise po2 calculations

[Robert Helling]

On 14.10.2014, at 11:01, Robert Helling  wrote:

Ah, one more thing although not urgent:

> I have some more concerns:
> 


Currently, we have an enum in struct dive computer that tells us if a dive is 
CCR. But strictly speaking, a dive can be both CCR and OC, for example when 
using bailout. So the property being CCR should not be a property of a dive but 
rather of a Moment in time (i.e. a sample or a plot entry).

Best
Robert

-- 
.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oO
Robert C. Helling Elite Master Course Theoretical and Mathematical Physics
  Scientific Coordinator
  Ludwig Maximilians Universitaet Muenchen, Dept. Physik
  Phone: +49 89 2180-4523  Theresienstr. 39, rm. B339
  http://www.atdotde.de

Enhance your privacy, use cryptography! My PGP keys have fingerprints
A9D1 A01D 13A5 31FA 6515  BB44 0820 367C 36BC 0C1Dand
DCED 37B6 251C 7861 270D  5613 95C7 9D32 9A8D 9B8F






signature.asc
Description: Message signed with OpenPGP using GPGMail
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Re: [CCR PATCH] reorganise po2 calculations

[Willem Ferguson]

Robert,

On 14/10/2014 11:01, Robert Helling wrote:
1) The planning of CCR is broken (after reenabling it). It was working 
before we started to mess with this CCR sensor code (needs looking 
into at some point, but I don’t have that right now, but might have 
soon. Also: Not critical as currently disabled anyway)


2) The code of fill_o2_values() seems fishy to me: Why is that called 
for OC dives, anyway and if it is, why are the last lines excited for 
OC???
Thank you for spotting this. I have just submitted a patch to improve 
this situation. As indicated in the patch message, the po2 values have 
not been explicitly initialised at the point when this code executes. 
po2 is initialised here for both CCR and OC.




3) Possibly unrelated: I looked into the xml of the Poseidon MkIV 
reference dive. That has a number of missing pressures and some values 
are NaX. There seems to be something broken as well.


I hope you are talking about the inconsistent pressure data at the start 
of the log. If not, please indicate because I have looked but cannot 
find this lower down in the log. It takes the dive computer 3-10 seconds 
to start reporting all parameters. The log starts when the very first 
value is reported (in this specific case, depth and o2sensor1), even 
though other parameters (o2sensor2, other pressures, temp, etc) have not 
been reported yet. Therefore the very first few log records may contain 
undefined fields. This is inherent in the way the MKVI logs dives and 
will be cleaned up when the log import code is finalised and integrated 
within the Subsurface code.

Kind regards,
willem

___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Re: [CCR PATCH] reorganise po2 calculations

[Robert Helling]

On 14.10.2014, at 07:08, Dirk Hohndel  wrote:Robert,I took the patch with tiny cleanups as it looked sane to me, but I think Ithe last couple of days have shown that you are much better than me inspotting issues in that code :-)Would you please take a look? Willem is leaving tonight and I want to makesure we get this figured out in the next 12 hours or so.Hi everybody,this looks pretty good. I took the liberty to factor out some calculations of the voting logic to simplify it a bit, took care of an assertion failing when the pO2=ambient_pressure (and then the inert gases were not set) and did some more clean up, see the patch attached.Looking at the computed ceilings for my OC dives, I cannot spot any problems, they all look reasonable to me. I don’t know however for CCR dives. In particular, you should check that the ceiling of the Poseidon MkIV dive looks good (I cannot judge).I have some more concerns:1) The planning of CCR is broken (after reenabling it). It was working before we started to mess with this CCR sensor code (needs looking into at some point, but I don’t have that right now, but might have soon. Also: Not critical as currently disabled anyway)2) The code of fill_o2_values() seems fishy to me: Why is that called for OC dives, anyway and if it is, why are the last lines excited for OC???3) Possibly unrelated: I looked into the xml of the Poseidon MkIV reference dive. That has a number of missing pressures and some values are NaX. There seems to be something broken as well.

0001-Reshuffle-CCR-voting-logic-and-minor-clean-ups.patch
Description: Binary data

-- .oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oORobert C. Helling     Elite Master Course Theoretical and Mathematical Physics                      Scientific Coordinator                      Ludwig Maximilians Universitaet Muenchen, Dept. Physik                      Phone: +49 89 2180-4523  Theresienstr. 39, rm. B339                      http://www.atdotde.deEnhance your privacy, use cryptography! My PGP keys have fingerprintsA9D1 A01D 13A5 31FA 6515  BB44 0820 367C 36BC 0C1D    andDCED 37B6 251C 7861 270D  5613 95C7 9D32 9A8D 9B8F



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Re: [CCR PATCH] reorganise po2 calculations

[Dirk Hohndel]

Robert,

I took the patch with tiny cleanups as it looked sane to me, but I think I
the last couple of days have shown that you are much better than me in
spotting issues in that code :-)

Would you please take a look? Willem is leaving tonight and I want to make
sure we get this figured out in the next 12 hours or so.

Thanks

/D

On Mon, Oct 13, 2014 at 10:24:36PM +0200, Willem Ferguson wrote:
> CCR patch. Reorganise the oxygen partial pressure calculations.
> 
> This patch responds to the side effects that the CCR code has had with
> respect to ceilings in OC dives and dive plans. Dive ceilings are now
> calculated apparently correctly. The following were performed:
> 1) remove the oxygen sensor and setpoint fields from the gas_pressures
> structure.
> 2) Re-insert setpoint and oxygen sensor fields in the plot_data structure.
> 3) Remove the algorithm that reads the o2 sensor data and calculates the
> pressures.po2
>value from function fill_pressures() in dive.c and save it as a separate
> function
>calc_ccr_po2() in profile.c.
> 4) Activate calc_ccr_po2 from function fill_pressures() in profile.c.
> 5) Move the relative position of the call to fill_pressures() within the
>function create_polt_info_new() in profile.c.
> 
> Signed-off-by: willem ferguson 
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

[CCR PATCH] reorganise po2 calculations

[Willem Ferguson]

CCR patch. Reorganise the oxygen partial pressure calculations.

This patch responds to the side effects that the CCR code has had with
respect to ceilings in OC dives and dive plans. Dive ceilings are now
calculated apparently correctly. The following were performed:
1) remove the oxygen sensor and setpoint fields from the gas_pressures 
structure.

2) Re-insert setpoint and oxygen sensor fields in the plot_data structure.
3) Remove the algorithm that reads the o2 sensor data and calculates the 
pressures.po2
   value from function fill_pressures() in dive.c and save it as a 
separate function

   calc_ccr_po2() in profile.c.
4) Activate calc_ccr_po2 from function fill_pressures() in profile.c.
5) Move the relative position of the call to fill_pressures() within the
   function create_polt_info_new() in profile.c.

Signed-off-by: willem ferguson 

I have carefully attended to whitespace issues. I hope this is ok.

>From d1c99e22270b9a2a6c303b688656264bd46bf248 Mon Sep 17 00:00:00 2001
From: willem ferguson 
Date: Mon, 13 Oct 2014 21:19:21 +0200
Subject: [PATCH] CCR patch. Reorganise the oxygen partial pressure
 calculations.

This patch responds to the side effects that the CCR code has had with
respect to ceilings in OC dives and dive plans. Dive ceilings are now
calculated, apparently correctly. The following were performed:
1) remove the oxygen sensor and setpoint fields from the gas_pressures structure.
2) Re-insert setpoint and oxygen sensor fields in the plot_data structure.
3) Remove the algorithm that reads the o2 sensor data and calculates the pressures.po2
   value from function fill_pressures() in dive.c and save it as a separate function
   calc_ccr_po2() in profile.c.
4) Activate calc_ccr_po2 from function fill_pressures() in profile.c.
5) Move the relative position of the call to fill_pressures() within the
   function create_polt_info_new() in profile.c.

Signed-off-by: willem ferguson 
---
 deco.c|   2 +-
 dive.c| 101 +---
 dive.h|   9 ++--
 profile.c | 155 +-
 4 files changed, 137 insertions(+), 130 deletions(-)

diff --git a/deco.c b/deco.c
index 42580d3..8dcfb7e 100644
--- a/deco.c
+++ b/deco.c
@@ -190,7 +190,7 @@ double add_segment(double pressure, const struct gasmix *gasmix, int period_in_s
 	int fo2 = get_o2(gasmix), fhe = get_he(gasmix);
 	struct gas_pressures pressures;
 
-	fill_pressures(&pressures, pressure, gasmix, (double) ccpo2 / 1000.0, &(dive->dc));
+	fill_pressures(&pressures, pressure, gasmix, (double) ccpo2 / 1000.0);
 
 	if (buehlmann_config.gf_low_at_maxdepth && pressure > gf_low_pressure_this_dive)
 		gf_low_pressure_this_dive = pressure;
diff --git a/dive.c b/dive.c
index 4d906e5..e6ee2e2 100644
--- a/dive.c
+++ b/dive.c
@@ -1527,105 +1527,26 @@ int gasmix_distance(const struct gasmix *a, const struct gasmix *b)
 
 /* fill_pressures(): Compute partial gas pressures in bar from gasmix and ambient pressures, possibly for OC or CCR, to be
  *  extended to PSCT. This function does the calculations of gass pressures applicable to a single point on the dive profile.
- * The structure "pressures" is used to obtain data and to return calculated gas pressures to the calling software.
+ * The structure "pressures" is used to return calculated gas pressures to the calling software.
  * Call parameters:	po2 = po2 value applicable to the record in calling function
  *			amb_pressure = ambient pressure applicable to the record in calling function
  *			*pressures = structure for communicating o2 sensor values from and gas pressures to the calling function.
  *			*mix = structure containing cylinder gas mixture information.
-			*dc = pointer to divecomputer structure.
- * This function called by: calculate_gas_information_new() in profile.c; add_segment() in deco.c.
+* This function called by: calculate_gas_information_new() in profile.c; add_segment() in deco.c.
  */
-extern void fill_pressures(struct gas_pressures *pressures, const double amb_pressure, const struct gasmix *mix, double po2, const struct divecomputer *dc)
-{
-	double sensor_j, sump, midp, minp, maxp;
-	double diff_limit = 100; // The limit beyond which O2 sensor differences are considered significant (default = 100 mbar)
-	int num_of_diffs;	 // The number of unacceptable differences among the ogygen sensor partial pressure measurements
-	int i, j, np;
-	bool maxdif = false, mindif = false;
-
-	if (dc->dctype == CCR) { // for CCR rebreathers..
-		// Estimate the most reliable PO2, given the different oxygen partial pressure values from the O2 sensors
-		switch (dc->no_o2sensors) {
-		case 2: { // For 2 sensors: take the mean value of the two partial pressure sensors.
-			np = 0;
-			sump = 0;		  // This calculation allows that for some samples (especially at start of dive) with
-			for (j = 0; j < 2; j++) { // an inactive sensor, the sensor(s) with zero values are not used.
-sensor_j = pressures->sensor[j