RFC: support for deco algorith and parameter fields

[Dirk Hohndel]

On Thu, Oct 16, 2014 at 05:09:06PM +0200, Jef Driesen wrote:
> 
> 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.

Definitely

>  +	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?

Hmm. I think these are all different.

- DC_DECO_ALG_NONE would be if the computer is in gauge or freedive or
  apnoe mode, right?
- DC_DECO_ALG_UNKNOWN if it does deco calculation, but we don't know the
  algoright
- DC_STATUS_UNSUPPORTED means the backend doesn't have this function
  implemented

> +	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_BUHLMAN ?
> 
> +	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,

Yes, as I explained in the other email, this one has a parameter as well -
more or less the GFhigh for a Buhlmann algorithm that is run in parallel
to VPM (and the lower ceiling is picked).

> and DSAT is some Pelagic thing? Are there any others we are missing?

Reading the Aeris A300CS documentation it states that the "proprietary
DSAT algorithm [...] has been used in all prior Aeris dive computers".

> +	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?

Maybe it would be smart to return UNKNOWN for the oldest Suuntos. I know
that they have been advertizing RGBM since at least the Vyper generation.

And yes, we should use this for all RGBM - the "Suunto fused" was meant as
an example in the comment.

> You forgot the ALG_ prefix in the name here.

So you want DC_DECO_ALG_BUHLMANN, etc? Just making sure this is all
consistent (and I double checked, it's two 'n').

> >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?

VPMGFS also has one GF. In the other mail the sat/desat factor was
mentioned. I don't know what else is out there.

> 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.

Yes. In the patch series I have separate fields for GFlow and GFhigh. That
may be overkill. But combining them to a dc_decoparams_gf_t looks good to
me. And since the GFS is in fact a GFhigh, we could use the same structure
for VPMGFS and just leave the GFlow 0.

/D