Re: Undo some broken CCR features
On 18/11/2014 10:41, Dirk Hohndel wrote: It is the variable that is used for plotting in the dive profile. entry->o2pressure is calculated before pressures.o2. And what, may I ask, is pressures.o2? pressures is a structure containing the partial pressures for O2, N2 and He that Robert created to form the basis of the ceiling and deco calculations. I think it is a pretty useful concept. entry->o2pressure is the original variable used before most of these development took place and, as far as I can gather, is still the one used by Qt for plotting. After version 4.3 we should look at closer integration between between entry->o2pressure and pressures, allowing us to eliminate that particular parameter in the call to fill_pressures(). So my suggestion in this case is: for 4.3 do not mess with something that works because this will probably require some action on the Qt side to get things 100% consistent, utilising pressures for plot data. Right now there are bigger fish to fry. Kind regards, willem ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Undo some broken CCR features
On Tue, Nov 18, 2014 at 08:41:26AM +, Dirk Hohndel wrote: > > So I assume this patch is not acceptable? Would you like me to re-draft the > > patch in line with you comments? > > No, I wanted to see your comments to understand what's going on. > I still want to understand what pressures.o2 does... > > I'll take the patch and hope that you'll send patches on top of it to get > CCR closer to ready for Beta 2 BTW, as a general rule. If I look at the patch, it does four things. While this may seem silly, I would have MUCH preferred four patches that each do one thing. Each with their own commit message. Etc. But as I said, I took what you sent (but I rewrote the commit message to be less... personal). /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Undo some broken CCR features
On Tue, Nov 18, 2014 at 08:19:43AM +0200, Willem Ferguson wrote: > >The first one is what you wrote in the email, the second one is the commit > >message that you gave when committing the patch. Why do you do that twice > >instead of just sending the commit? > > In future I will just send the commit. Thanks > >BTW: I wanted to comment on this one before. This is SO UGLY. > >if you have braces on one side of the else, have them on both sides. > > This is not my code Interesting. Let's look at git blame to find out who I need to yell at :-) Miika! That's just terrible... > >>diff --git a/profile.c b/profile.c > >>index 63023b5..8e43c21 100644 > >>--- a/profile.c > >>+++ b/profile.c > >>@@ -905,7 +905,7 @@ static void calculate_gas_information_new(struct dive > >>*dive, struct plot_info *p > >>amb_pressure = depth_to_mbar(entry->depth, dive) / 1000.0; > >>- fill_pressures(&entry->pressures, amb_pressure, > >>&dive->cylinder[cylinderindex].gasmix, entry->pressures.o2, > >>dive->dc.dctype, entry->sac); > >>+ fill_pressures(&entry->pressures, amb_pressure, > >>&dive->cylinder[cylinderindex].gasmix, entry->o2pressure, dive->dc.dctype, > >>entry->sac); > >>fn2 = (int) (1000.0 * entry->pressures.n2 / amb_pressure); > >>fhe = (int) (1000.0 * entry->pressures.he / amb_pressure); > >hmm, so what IS the difference between entry->pressures.o2 and > >enry->o2pressure? > entry->o2pressure holds the consensus PO2 after considering the data from > the different oxygen sensors. OK > It is the variable that is used for plotting > in the dive profile. entry->o2pressure is calculated before pressures.o2. And what, may I ask, is pressures.o2? > The call above to fill_pressures() is the one that actually computes > pressures.o2. I assume one should not use pressures.o2 as a parameter in a > call to a function that calculates pressures.o2?? Valid point. > The XML import/export is not perfect yet. The export removes the cylinder > start and end values supplied by the DC because cylinder->sample_start and > cylinder->sample_end are zeroed somewhere beforehand, so they are not > written. The import also has some problems with cylinder pressures and > figuring out start/end pressures. My suggestion is let's get the XML side > working reliably, then it is a simple matter to transfer to the git back > end. Does this make sense? Sure. We're running out of time for 4.3, though > I am actively working at improving the XML import/export. Excellent. > So I assume this patch is not acceptable? Would you like me to re-draft the > patch in line with you comments? No, I wanted to see your comments to understand what's going on. I still want to understand what pressures.o2 does... I'll take the patch and hope that you'll send patches on top of it to get CCR closer to ready for Beta 2 /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Undo some broken CCR features
On 17/11/2014 22:58, Dirk Hohndel wrote:
A few questions...
a - why are there two different commit messages?
On Mon, Nov 17, 2014 at 09:19:20PM +0200, Willem Ferguson wrote:
Undo some features that were broken in Robert's patch 0d7c192e:
1) Calculate correct partial pressure of oxygen to be plotted on
dive profile, taking into account the oxygen sensor data.
Currently, erroneously, OC PO2 values are shown, due to an
erroneous calling parameter to fill_pressures().
2) Read start and end cylinder pressured correctly. In Robert's
patch, some wrong assignments were done in file.c. This is
now corrected and the correct cylinder pressures are shown in
the equipment tab.
3) Write correct cylinder pressures to XML. Currently the data for
the two cylinder are written to XML the wrong way round
(diluent pressures = oxygen and vice versa).
Expand XML output:
1) Write oxygen sensor data to XML
2) Write no_of_02sensors to XML
Signed-off-by: willem ferguson
From 8a4121607a4259fc5a2651eb815a340fd81d5034 Mon Sep 17 00:00:00 2001
From: willem ferguson
Date: Mon, 17 Nov 2014 21:04:36 +0200
Subject: [PATCH] Undo some features that were broken in Robert's patch
0d7c192e:
1) Calculate correct partial pressure of oxygen to be plotted on
dive profile, taking into account the oxygen sensor data.
Currently, erroneously, OC PO2 values are shown, due to an
erroneous calling parameter to fill_pressures().
2) Read start and end cylinder pressured correctly. In Robert's
patch, some wrong assignments wer done in file.c. This is
now corrected and the correct cylinder pressures are shown in
the equipment tab.
Expand XML output:
1) Write oxygen sensor data to XML
2) Write no_of_02sensors to XML
Signed-off-by: willem ferguson
The first one is what you wrote in the email, the second one is the commit
message that you gave when committing the patch. Why do you do that twice
instead of just sending the commit?
In future I will just send the commit.
diff --git a/file.c b/file.c
index 97095fd..4fe4d35 100644
--- a/file.c
+++ b/file.c
@@ -597,7 +597,7 @@ int parse_txt_file(const char *filename, const char *csv)
case 13:
add_sample_data(sample,
POSEIDON_O2CYLINDER, value);
if (!o2cylinder_pressure) {
-
dive->cylinder[1].sample_start.mbar = value * 1000;
+
dive->cylinder[0].sample_start.mbar = value * 1000;
o2cylinder_pressure =
value;
} else
o2cylinder_pressure =
value;
BTW: I wanted to comment on this one before. This is SO UGLY.
if you have braces on one side of the else, have them on both sides.
This is not my code
diff --git a/profile.c b/profile.c
index 63023b5..8e43c21 100644
--- a/profile.c
+++ b/profile.c
@@ -905,7 +905,7 @@ static void calculate_gas_information_new(struct dive
*dive, struct plot_info *p
amb_pressure = depth_to_mbar(entry->depth, dive) / 1000.0;
- fill_pressures(&entry->pressures, amb_pressure, &dive->cylinder[cylinderindex].gasmix, entry->pressures.o2, dive->dc.dctype, entry->sac);
+ fill_pressures(&entry->pressures, amb_pressure,
&dive->cylinder[cylinderindex].gasmix, entry->o2pressure, dive->dc.dctype, entry->sac);
fn2 = (int) (1000.0 * entry->pressures.n2 / amb_pressure);
fhe = (int) (1000.0 * entry->pressures.he / amb_pressure);
hmm, so what IS the difference between entry->pressures.o2 and
enry->o2pressure?
entry->o2pressure holds the consensus PO2 after considering the data
from the different oxygen sensors. It is the variable that is used for
plotting in the dive profile. entry->o2pressure is calculated before
pressures.o2. The call above to fill_pressures() is the one that
actually computes pressures.o2. I assume one should not use pressures.o2
as a parameter in a call to a function that calculates pressures.o2??
diff --git a/save-xml.c b/save-xml.c
index 52582db..cf7ccfe 100644
--- a/save-xml.c
+++ b/save-xml.c
@@ -254,6 +254,21 @@ static void save_sample(struct membuffer *b, struct sample
*sample, struct sampl
old->cns = sample->cns;
}
+ if ((sample->o2sensor[0].mbar) && (sample->o2sensor[0].mbar != old->o2sensor[0].mbar)) {
+ put_milli(b, " sensor1='", sample->o2sensor[0].mbar, " bar'");
+ old->o2sensor[0] = sample->o2sensor[0];
+ }
+
+ if ((sample->o2sensor[1].mbar) && (sample->o2sensor[1].mbar !=
old->o2sensor[1].mbar)) {
+ put_milli(b, " sensor2='", sample->o2sensor[1].mbar, " bar'");
+ old->o2sensor[1] = sample->o2sensor[1];
+ }
+
+ if ((sample->o
Re: Undo some broken CCR features
A few questions...
a - why are there two different commit messages?
On Mon, Nov 17, 2014 at 09:19:20PM +0200, Willem Ferguson wrote:
> Undo some features that were broken in Robert's patch 0d7c192e:
>
> 1) Calculate correct partial pressure of oxygen to be plotted on
>dive profile, taking into account the oxygen sensor data.
>Currently, erroneously, OC PO2 values are shown, due to an
>erroneous calling parameter to fill_pressures().
> 2) Read start and end cylinder pressured correctly. In Robert's
>patch, some wrong assignments were done in file.c. This is
>now corrected and the correct cylinder pressures are shown in
>the equipment tab.
> 3) Write correct cylinder pressures to XML. Currently the data for
>the two cylinder are written to XML the wrong way round
> (diluent pressures = oxygen and vice versa).
>
> Expand XML output:
> 1) Write oxygen sensor data to XML
> 2) Write no_of_02sensors to XML
>
> Signed-off-by: willem ferguson
>
> From 8a4121607a4259fc5a2651eb815a340fd81d5034 Mon Sep 17 00:00:00 2001
> From: willem ferguson
> Date: Mon, 17 Nov 2014 21:04:36 +0200
> Subject: [PATCH] Undo some features that were broken in Robert's patch
> 0d7c192e:
>
> 1) Calculate correct partial pressure of oxygen to be plotted on
>dive profile, taking into account the oxygen sensor data.
>Currently, erroneously, OC PO2 values are shown, due to an
>erroneous calling parameter to fill_pressures().
> 2) Read start and end cylinder pressured correctly. In Robert's
>patch, some wrong assignments wer done in file.c. This is
>now corrected and the correct cylinder pressures are shown in
>the equipment tab.
>
> Expand XML output:
> 1) Write oxygen sensor data to XML
> 2) Write no_of_02sensors to XML
>
> Signed-off-by: willem ferguson
The first one is what you wrote in the email, the second one is the commit
message that you gave when committing the patch. Why do you do that twice
instead of just sending the commit?
> diff --git a/file.c b/file.c
> index 97095fd..4fe4d35 100644
> --- a/file.c
> +++ b/file.c
> @@ -597,7 +597,7 @@ int parse_txt_file(const char *filename, const char *csv)
> case 13:
> add_sample_data(sample,
> POSEIDON_O2CYLINDER, value);
> if (!o2cylinder_pressure) {
> -
> dive->cylinder[1].sample_start.mbar = value * 1000;
> +
> dive->cylinder[0].sample_start.mbar = value * 1000;
> o2cylinder_pressure =
> value;
> } else
> o2cylinder_pressure =
> value;
BTW: I wanted to comment on this one before. This is SO UGLY.
if you have braces on one side of the else, have them on both sides.
> diff --git a/profile.c b/profile.c
> index 63023b5..8e43c21 100644
> --- a/profile.c
> +++ b/profile.c
> @@ -905,7 +905,7 @@ static void calculate_gas_information_new(struct dive
> *dive, struct plot_info *p
>
> amb_pressure = depth_to_mbar(entry->depth, dive) / 1000.0;
>
> - fill_pressures(&entry->pressures, amb_pressure,
> &dive->cylinder[cylinderindex].gasmix, entry->pressures.o2, dive->dc.dctype,
> entry->sac);
> + fill_pressures(&entry->pressures, amb_pressure,
> &dive->cylinder[cylinderindex].gasmix, entry->o2pressure, dive->dc.dctype,
> entry->sac);
> fn2 = (int) (1000.0 * entry->pressures.n2 / amb_pressure);
> fhe = (int) (1000.0 * entry->pressures.he / amb_pressure);
>
hmm, so what IS the difference between entry->pressures.o2 and
enry->o2pressure?
> diff --git a/save-xml.c b/save-xml.c
> index 52582db..cf7ccfe 100644
> --- a/save-xml.c
> +++ b/save-xml.c
> @@ -254,6 +254,21 @@ static void save_sample(struct membuffer *b, struct
> sample *sample, struct sampl
> old->cns = sample->cns;
> }
>
> + if ((sample->o2sensor[0].mbar) && (sample->o2sensor[0].mbar !=
> old->o2sensor[0].mbar)) {
> + put_milli(b, " sensor1='", sample->o2sensor[0].mbar, " bar'");
> + old->o2sensor[0] = sample->o2sensor[0];
> + }
> +
> + if ((sample->o2sensor[1].mbar) && (sample->o2sensor[1].mbar !=
> old->o2sensor[1].mbar)) {
> + put_milli(b, " sensor2='", sample->o2sensor[1].mbar, " bar'");
> + old->o2sensor[1] = sample->o2sensor[1];
> + }
> +
> + if ((sample->o2sensor[2].mbar) && (sample->o2sensor[2].mbar !=
> old->o2sensor[2].mbar)) {
> + put_milli(b, " sensor3='", sample->o2sensor[2].mbar, " bar'");
> + old->o2sensor[2] = sample->o2sensor[2];
> + }
> +
> if (sample->setpoint.mbar != old->setpoint.mbar) {
> put_milli(b, " po2='", sample->setpoint.mbar, " bar'");
>
