[PATCH] Added support for parsing temperature in the dive header

[Jef Driesen]

On 2014-09-02 22:53, Dirk Hohndel wrote:
> On Tue, Sep 02, 2014 at 10:04:01AM +0200, Jef Driesen wrote:
>> [Sorry for the slow response, but I've been busy with non 
>> libdivecomputer
>> related things.]
> 
> How dare you have a real life! :-) :-)
> 
> I actually took the last few days off myself and did a computer free 
> trip
> into the mountains.

That'll be the same for me for the next few days.

>> Remember that adding features ad-hoc, based on what a certain 
>> manufacturer
>> (e.g. suunto, which happens to be the first supported devices) 
>> provides is
>> pretty much how the current libdivecomputer "design" evolved into what 
>> we
>> have now. Linus and you already complained several times about the 
>> current
>> libdivecomputer api. Don't get me wrong, I absolutely agree that many 
>> of
>> those concerns are indeed valid and should be addressed. But for the 
>> time
>> being, I just don't want to repeat those mistakes again. So yes, I'm a 
>> lot
>> more careful when it comes to adding new features now.
> 
> Which I think is a wise choice. Don't get me wrong, I am not advocating 
> to
> add any random thing. I'd like at least a couple of dive computers from
> different vendors to provide the information. Which makes this a tricky
> one as the Uemis with its ass-backwards download approach isn't 
> supported
> by libdivecomputer at all. And I'd like at least a vague "OK, this 
> could
> be useful" check. The Uemis includes rather illdefined values for
> "cold_fact", "work_fact", and "bubble_fact" in every sample. I wouldn't
> suggest supporting something like that in libdivecomputer because even 
> if
> the Uemis was supported... what the heck would you do with these?
> 
>> Also from an application point of view, I think it's more interesting 
>> to
>> have a rather limited set of features that is very well supported by 
>> the
>> majority of the backends, then having a large number of features that 
>> are
>> only supported by a few backends. That's basically why I have been 
>> spending
>> most of my time and energy on improving the existing features, and not
>> adding anything new.
> 
> I think we get at least as many "the vendor application supports this, 
> why
> don't you" email as you do. And then we often go back and look and 
> notice
> that the vendor application is actually making data up that isn't 
> included
> in the download (I think you just helped us in one case where the 
> vendor
> application shows a pressure graph but looking into the sample data it 
> is
> clear that there isn't even enough space there to hide the pressure
> data...) - so it's just interpolated data. Same goes for deco 
> information.
> Sadly very few vendors actually include that in the downloadable data -
> yet their applications show it by simply "re-doing the math".
> 
> But on the flip side - if a computer (or better, a couple of computers) 
> do
> provide something that I can imagine being really useful (heart rate 
> comes
> to mind, compass heading, ndl / deco status), then I'll argue to 
> include
> it even if only a few DCs support it.

That's also how I see it, except that the definition of "useful" may not 
always be the same :-)

>> But okay, maybe I went into the defensive corner too fast. So let's 
>> start
>> all over again, and assume we want to include the tank size too. Can 
>> you
>> explain me how you would use the dc_tankdata_t structure in the 
>> following
>> scenarios:
>> 
>>  * No tank size available (e.g. just begin/end pressure): How do we 
>> indicate
>> that the tank size isn't available?
> 
> I see where you are going. The libdivecomputer design pattern would be 
> to
> return an error when that piece of information isn't availble, yet here 
> we
> have some data available and others not available.

If there is not even begin/end pressure info, then yes returning 
DC_STATUS_UNSUPPORTED is the right thing to do. But for optional data, 
like the tank size, that's not really an option.

> We could fall back to saying "all values 0" means no data available. If 
> we
> have neither compressed nor wet volume, we obviously don't know how big
> the tank is (i.e., while 0°C is a valid temperature, a volume of 0ml is
> NOT a valid tank volume).

That's true, we already have a precedent here. For the deco sample, only 
the type is mandatory. The time and depth fields are optional and set to 
zero when not available.

The only tricky part here is that for floating point values, comparisons 
for equality aren't very reliable. We might get away with it here 
because we assign to zero, but nevertheless it's something to watch out 
for. This is one of the reasons why I like the subsurface integer only 
units, I'll certainly consider adopting this for libdivecomputer too.

>>  * Metric vs imperial tanks (e.g. water vs air capacity): How do we 
>> indicate
>> the difference? For imperial tanks, we'll certainly need the working
>> pressure. Metric tank also have a working pressure, and although it's 
>> not
>> needed for calculating the amount of gas, it may be available too. Do 
>> we
>> convert imperial units to metric units to be consistent with the rest 
>> of the
>> libdivecomputer api, or do we make an exception here?
> 
> With all due respect to my host country, the American way of defining
> cylinder capacity is beyond braindead. Yes, it seems to make intuitive
> sense when you tell someone "this is an 80cuft cylinder" (assuming you
> have wrapped your mind around the non-metric measurements), but of 
> course
> this only makes sense if you actually know the correct working pressure 
> -
> and as I frequently find out on dive boats it is not clear at all to a 
> lot
> of divers that an HP119 and LP95 contain the same amount of gas at the
> same pressure...

:-)

> So I would suggest that we have a member for the wet volume in mL.
> And a different member for the volume at working pressure - also in mL.
> And a third member for the working pressure - in mbar.
> (oops, I am assuming the Subsurface style "we prefer integers" approach 
> -
> you can obviously use doubles and liter and bar - I like integer math
> better, but I'm fine either way).
> 
> Technically one of the three values should of course be redundant - but 
> we
> have also learned the hard way that it actually isn't. Because the
> "marketing size" of many cylinders is incorrect. And apparently almost 
> no
> one takes the compressibility factor into account (at 300bar a tank
> contains only about 270 times as much air as its wet volume).
> So if you have all three, give them. If you have only wet volume (or 
> only
> gas volume and working pressure) set the others to 0.

Won't it be confusing if all three values are allowed to contain valid 
data? How do you know if it's a metric or imperial tank? I assume most 
applications already have logic to deal with this difference anyway. Are 
there any devices that give us the wet volume for imperial tanks? As far 
as I know the cobalt doesn't. As you already mention, this conversion 
may need to take into account the compressibility factor (e.g. ideal vs 
vanderwaals gas law). I think that's something libdivecomputer should 
leave to the application.

Wouldn't it be simpler to have either a metric size (wet volume, and 
optionally working pressure) or imperial size (air volume and working 
pressure), but never both at the same time. And then a third field 
containing the type? Something like this:

struct dc_tank_t {
    unsigned int type; /* metric or imperial */
    double volume; /* either wet or air volume depending on the type, but 
always in liter */
    double workpressure; /* in bar */
    double beginpressure;
    double endpressure;
};

Jef