On 2014-10-16 15:24, Dirk Hohndel wrote:
as "promised", here's another little feature that I frequently get asked about by some tech divers: show which algorithm / which paramters where used on a dive. There are quite a few dive computers for which we can return at least some viable data (I almost apologize for the last patch in the series... technically it is correct, and I think adding it makes it simpler for consumers of libdivecomputer to use this API).
Nice work!
I think this is nicely complementary with another improvement that I have been thinking about: a new DC_FIELD_DIVEMODE, to differentiate between freedive, gauge, open circuit and closed circuit modes.
I only had time for a quick look at the moment, so just a few quick comments for now:
+typedef enum dc_deco_alg_t {
I'm not really in love with the "deco_alg" abbreviation. But "deco_algorithm" becomes pretty long. Maybe just "algorithm", or "decomodel"? Other suggestions are welcome too.
+ DC_DECO_ALG_UNKNOWN,
I wonder if we should rename this to NONE? It looks like you used this for gauge/freedive mode. But in that case, the decompression algorithm is basically disabled. That's not the same as an unknown algorithm. Maybe the DC_STATUS_UNSUPPORTED status code is a better choice to indicate an unknown algorithm?
+ DC_DECO_ALG_BZH, /* Buehlmann ZH, no GF */ + DC_DECO_ALG_BZHGF, /* Buehlmann ZH, with GF */
The BZH name sounds a bit cryptic to me. How about naming it BUHLMANN instead?
+ DC_DECO_ALG_VPMGFS, /* VPM with GF surface */ + DC_DECO_ALG_DSAT, /* Aeris DSAT */
I've never really heard of these before. Apparently VPMGFS is a hybrid between VPM and Buhlmann GF used by Shearwater, and DSAT is some Pelagic thing? Are there any others we are missing?
+ DC_DECO_RGBM, /* Suunto fused RGBM */
I assume you mean all variants of RGBM, and not just the Suunto variant? I believe Atomics also uses some RGBM variant. I don't think the older Suunto's (eon and solution) use RGBM. Most likely they use Buhlmann, although I'm not sure. Did RGBM even exist already at that time?
You forgot the ALG_ prefix in the name here.
I'm calling this an RFC because I want feedback if this is the right API... this was straight forward to do, but I wonder if you would prefer a complex data structure and a single API entry point:
DC_FIELD_DECO_INFO
and
struct dc_deco_information_t { unsigned int algo; unsigned int param1; /* e.g. GFhigh or GFS */ unsigned int param2; /* e.g. GFlow */ unsigned int param3; /* currently unused */ };
I'm not really sure what would be the best solution here. Are there any other algorithms besides the GF that have parameters that make sense to support? One concern is that with an integrated structure, we can't add new parameters if that's ever necessary. I'm not really in love with placeholder fields either. While with a separate DC_FIELD_DECOPARAMS type, we can introduce different data structures for each deco algorithm. For example:
struct dc_decoparams_gf_t { unsigned int low; /* e.g. GFhigh or GFS */ unsigned int high; /* e.g. GFlow */ };
Of course an application will first have to check the algorithm, to know which structure to use, but I think that's not unreasonable. If you care about the params, you probably need the algo anyway.
What do you think?
Jef