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