Fwd: Re: [PATCH] CCR Import from CSV: Calculate correct nitrogen and helium gas pressures (VERSION 2)

[Willem Ferguson]

On 03/11/2014 19:22, Dirk Hohndel wrote:
> On Mon, Nov 03, 2014 at 04:52:36PM +0200, Willem Ferguson wrote:
>> diff --git a/profile.c b/profile.c
>> index 4be3a5b..e97ea4c 100644
>> --- a/profile.c
>> +++ b/profile.c
>> @@ -901,7 +901,10 @@ static void calculate_gas_information_new(struct dive *dive, struct plot_info *p
>>   		fo2 = get_o2(&dive->cylinder[cylinderindex].gasmix);
>>   		fhe = get_he(&dive->cylinder[cylinderindex].gasmix);
>>
>> -		fill_pressures(&entry->pressures, amb_pressure, &dive->cylinder[cylinderindex].gasmix, entry->pressures.o2);
>> +		if ((dive->dc.dctype == CCR) && (cylinderindex <= dive->diluent_cylinder_index))
> Hmm... the cylinderindex here is 0 because there are no gas changes and
> that's the default. So this condition works. But I'm not sure this is the
> right way to do this. Shouldn't there be a function instead that says
> something like
>
> if (dive->dc.dctype == CCR && cylinderindex == 0)
> 	cylinderindex = dive->diluent_cylinder_index;
>
> and then we use cylinderindex again?
>
> I may be nit-picking here, but I'd really like to figure out a very clean,
> well documented way to deal with the CCR phase of a dive and no surprises
> when we have different DCs and people switch to bailout...
>
> e.g., imagine a diver who (for whatever reason) has O2 as cylinder 0,
> bailout gas as cylinder 1 and diluent as cylinder 2.
>
> I'm not sure this is realistic, but it would make your code rather hard to
> understand.
>
> And the one thing I have learned in the last few years of working on
> Subsurface is to make as few assumptions as possible about what data we
> get from a dive computer.
>
> /D
> _______________________________________________
> subsurface mailing list
> subsurface at subsurface-divelog.org
> http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Thanks for this feedback. I perfectly understand your point above and I
will implement it. As explained in mail to Robert earlier, your approach
is by far the shortest and most direct. I will also implement functions
based on an attribute of structures of cylinder_t. This will basically
step through the cylinders, find the oxygen cylinder and store the index
in dive->oxygen_cylinder_index. The same for the diluent cylinder. So I
will build on top of the existing patch.
Does this sound reasonable?
Kind regards,
wilem