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

[Jef Driesen]

On 09-09-14 21:27, Dirk Hohndel wrote:
> On Tue, Sep 09, 2014 at 01:04:29PM +0200, Jef Driesen wrote:
>>> 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;
>>> };
>
> How is that simpler than having all three values and figuring it out based
> on that?

There is certainly a bit of personal preference involved, but I find it more 
intuitive and descriptive to read something like this:

typedef enum dc_tanksize_t {
    DC_TANKSIZE_NONE,
    DC_TANKSIZE_METRIC,
    DC_TANKSIZE_IMPERIAL,
} dc_tanksize_t;

if (tank.type == DC_TANKSIZE_METRIC) {
    /* Wet volume */
} else if (tank.type == DC_TANKSIZE_IMPERIAL) {
    /* Air volume */
} else {
    /* No tank size available */
}

compared to checking the contents of the fields for the magic value zero. It 
also avoids the problem of the floating point comparisons (see below). And as a 
bonus, it also very clear that it's either metric or imperial, but never both. 
Allowing both is only confusing if you ask me.

>> I would love to get some feedback on this!
>>
>> Although the discussion on whether libdivecomputer should calculate missing
>> header fields from the samples or not, is an interesting one, it's not a
>> priority at the moment. Initially libdivecomputer will only implement the
>> new tank and temperature fields for devices where such info is available in
>> the header, especially those that do not even have sample data. But I really
>> would like to have this included in the next v0.5 release!
>
> I would LOVE if you would switch these new fields to be integers.
> And I think it would be perfectly fine to have wet_volume, air_volume and
> working pressure and simply test against 0 to see which one is set. And if
> for some odd device both are set, have the app figure out what it wants to
> do.

I have nothing against integers, but everywhere else in the libdivecomputer api 
we already use floating point values. So at the moment I have a preference 
towards maintaining consistency and sticking to floating points. The other 
reason is that storing the wet volume (in liters) in an integer doesn't provide 
enough precision. The cobalt for example stores the data with a precision of 
1/10 liter. We could resort to using milliliters instead, but that would be even 
more alien to the current libdivecomputer api.

If we decide to switch to integers eventually, then it should be done 
everywhere, and not just for these new fields. But that's definitely out of 
scope for the next release.

Jef