[PATCH] Correct bug: setpoint handling of Poseidon CCR dive logs

[Dirk Hohndel]

Linus,

much of this is code that you wrote (the original "when do we zero thing,
when don't we"). I clearly mis-remembered some of this. Would you chime in
to get to the right resolution here, please?

On Thu, Oct 30, 2014 at 08:29:18AM +0200, Willem Ferguson wrote:
> Ok, so here is how I understand the code as it is at present. It all happens
> in
> fixup_dive_dc() in dive.c and in fill_o2_values in profile.c.
> 
> fixup_dive_dc():
> 
> While a particular cylinder is being used (i.e. while the index of the
> cylinder *remains the same*), the duplicate gas pressure values are zeroed
> out. This is pre-existing code, i.e  it was there before any CCR code was
> developed. I only added the zeroing out for diluent gas pressures as well.
> [~line 1138 in dive.c].

I think I remember now. The logic was that many tank sensors don't give a
reading in every sample. And somehow, somewhere this got converted into
samples with no pressure reading ending up with the previous pressure
reading which gave rather "step function" looking plots. 

> The pre-existing code also zeroed out duplicate temperature values [~line
> 1169]. You can see there that the style of the commenting for that
> particular operation is different from my style of commenting.

Same logic I believe.

> Immediately below the temperature value manipulation I added code that
> zeroes out the oxygen-related data. (o2 sensors and o2 setpoint).

And at least for O2 setpoint that's problematic, given that we use this to
distinguish CCR and OC. Maybe that's wrong and we need to fix THAT
elsewhere, but right now that's what we do.

> Now where do all the zeroed data values get re-instated for profile
> calculations? As follows:
> 
> 1) Gas pressures:
> In gaspressures.c the zeroed gas pressure values effectively get replaced.
> This is code that originally comes from profile.c and that code does *much*
> more than just re-instating the zeroed values. But, amongst others,
> interpolated values (using raw data from structures of sample) replace
> zeroed values in the appropriate places in structures of plotdata.

Yes, it does "smart" interpolation (depth based, attempting "constant SAC
rate") and removes linear interpolation (some dive log programs did that
and filled in these completely ridiculous linear interpolations... we
detect that and rip it out).

> 2) Temperature values:
> The temperature values are re-instated in populate_plot_entries() in
> profile.c (~line 591). This is pre-existing code.
> 
> 3) Oxygen values:
> In function fill_o2_values() in profile.c the zeroed sensor data and
> setpoint data are re-instated for plotting. As with the pressure
> calculations mentioned above, this function does more than re-instating
> zeroed oxygen values. It also triggers calculation of po2 values that are
> stored in pressures.o2 contained within each structure of plotdata.

This is the code I'm less familiar with - that's partly yours (Willem),
partly Robert's, I think.

> Towards a plan of action:
> In the underlying data structures the memory space requirements are the
> same, regardless of whether a zero is stored in a location or a measured
> data value. My suggestion is that we do not touch the pressure values in
> fixup_dive_dc but that we remove code  that zeroes the oxygen-related
> values. Similarly, fill_o2_values() in profile.c is reduced to a loop that
> steps through the structures of plotdata, which corrects zero values (that
> might have originated either from the insertion of events along the dive
> profile or from glitches in the logging or download process) and which
> triggers po2 calculation for each structure of plotdata (i.e. each point on
> the dive profile).

That sounds sane.

> I am not sure what to do about the zeroing and re-instatement of temperature
> data but if we were consistent, the zeroing of temperature data should also
> be removed.

Why? Can you explain why the tank pressure handling would stay, but the
temperature handling would change?

/D