Undo some broken CCR features

[Willem Ferguson]

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 <willemferguson at zoology.up.ac.za>
>>
>>  From 8a4121607a4259fc5a2651eb815a340fd81d5034 Mon Sep 17 00:00:00 2001
>> From: willem ferguson <willemferguson at zoology.up.ac.za>
>> 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 <willemferguson at zoology.up.ac.za>
> 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->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'");
>>   		old->setpoint = sample->setpoint;
> So this stores the three sensors in XML - but they aren't stored in the
> git backend?
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?

I am actively working at improving the XML import/export.
>
>> @@ -355,6 +370,8 @@ static void save_dc(struct membuffer *b, struct dive *dive, struct divecomputer
>>   		for (enum dive_comp_type i = 0; i < NUM_DC_TYPE; i++)
>>   			if (dc->dctype == i)
>>   				show_utf8(b, dctype_text[i], " dctype='", "'", 1);
>> +		if (dc->no_o2sensors)
>> +			put_format(b," no_o2sensors='%d'", dc->no_o2sensors);
>>   	}
>>   	put_format(b, ">\n");
>>   	save_depths(b, dc);
> Ditto.
>
>
> /D
So I assume this patch is not acceptable? Would you like me to re-draft 
the patch in line with you comments?
Kind regards,
willem