Hi Jef,
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).
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 */ };
/D
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
On Thu Oct 16 2014 16:09:06 GMT+0100 (IST), Jef Driesen wrote: [...]
+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.
While 'algorithm' is used quite often, the 'model' is more correct, IMHO. Quite often a simple change to deco model parameters causes its name change i.e. ZHL-16B vs. ZHL-16C. Calling them two different algorithms does not make sense.
[...]
- 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?
If I am not mistaken Buhlmann was calling his models ZHL-XY, i.e. ZHL-16C. There are Buhlmann models with different number of tissue compatments than 16...
[...]
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 */ };
You are basically opening a can of worms. :) Manufacturers like to add their own parameters it seems. Look at OSTC custom functions where ZHL-16C is extended with desaturation/saturation multipliers ( note: non-GF version). Also there are parameters which I find hard to classify. Is calculating your deco 1m deeper than you are part of deco model extension or a safety feature? And these are probably specific to a dive computer manufacturer.
Maybe the best structure would be simple list of (name, value) pairs for all dive computer configration options (name being a string)?
Regards,
Artur
On Thu, Oct 16, 2014 at 03:43:11PM +0000, wrob311@gmail.com wrote:
On Thu Oct 16 2014 16:09:06 GMT+0100 (IST), Jef Driesen wrote: [...]
+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.
While 'algorithm' is used quite often, the 'model' is more correct, IMHO. Quite often a simple change to deco model parameters causes its name change i.e. ZHL-16B vs. ZHL-16C. Calling them two different algorithms does not make sense.
Yeah, I like dc_deco_model_t
- 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?
If I am not mistaken Buhlmann was calling his models ZHL-XY, i.e. ZHL-16C. There are Buhlmann models with different number of tissue compatments than 16...
I thought about that - but given how incredibly sloppy many of the implementations are (often due to limited processing power), it seems that even if the vendor were to tell you exactly which model they use (as some do), there is very limited benefit in knowing that. More importantly, I believe in most cases we don't know which exact model has been implemented. Some don't even tell you if they use gradient factors (and which ones).
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 */ };
You are basically opening a can of worms. :) Manufacturers like to add their own parameters it seems. Look at OSTC custom functions where ZHL-16C is extended with desaturation/saturation multipliers ( note: non-GF version).
Both versions, I believe.
Also there are parameters which I find hard to classify. Is calculating your deco 1m deeper than you are part of deco model extension or a safety feature? And these are probably specific to a dive computer manufacturer.
And most of those are not really well specified or documented.
I get specific requests from tech divers to show the gradient factor used by their dive computer on specific dives - I think that's because Subsurface shows the GF we use for our own deco calculations and people want to be able to compare. I have not received any other requests for paramters. I simply made this somewhat more generic when I saw in the Shearwater documentation that they have the fused VPM-GSF where they basically take the lower ceiling of VPM or Buehlmann with that parameter as surface GF (so basically GFhigh).
Maybe the best structure would be simple list of (name, value) pairs for all dive computer configration options (name being a string)?
Interesting. What exactly would that include? Metric vs imperial? 12 or 24h? yyyy-mm-dd or mm/dd/yy? Just trying to understand how broad you take "all configuration options".
/D
On Thu, Oct 16, 2014 at 7:06 PM, Dirk Hohndel dirk@hohndel.org wrote:
On Thu, Oct 16, 2014 at 03:43:11PM +0000, wrob311@gmail.com wrote:
[...]
Maybe the best structure would be simple list of (name, value) pairs for all dive computer configration options (name being a string)?
Interesting. What exactly would that include? Metric vs imperial? 12 or 24h? yyyy-mm-dd or mm/dd/yy? Just trying to understand how broad you take "all configuration options".
My experience with dive computers is quite limited of course, but I would simply fetch values as they are. Do not interpret them as something more meaningful than (name, value) pair, just display to the user.
Of course, you could argue that you want just to take deco model into account or few options, but I believe once you give people one option, then they will request more of them.
And then, as OSTC case shows with options being changed between various versions of firmware releases, maintaining the whole thing on API level will be simply horrible.
Regards,
w
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
On 2014-10-16 20:16, Dirk Hohndel wrote:
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
So'll add that to my todo list for v0.5 too, along with the DC_FIELD_TEMPERATURE and DC_FIELD_TANK stuff.
- 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?
Yes
- 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
The DC_STATUS_UNSUPPORTED is used in case a feature is not supported by the dive computer, or not implemented by the backend. The line between those two is very thin. If it's not implemented by the backend, that could be due to the fact that it's not supported by the dive computer, or because we don't know yet where and how it's stored. But usually we don't know that difference in advance.
The same logic applies here. Of course every dive computer will use some deco model (unless in gauge/freedive mode), but for libdivecomputer unsupported and unknown are basically the same thing.
- 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 ?
Yes, something like that.
- 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".
I see no reasons not to include those two, I was just wondering how they relate to the others. DSAT (Diving Science and Technology) appears to be a PADI related company, but that's all I can find about it.
- 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.
That's also what I found. According to wikipedia, RGBM was introduced in 1999, but the solution and eon range are much older. I checked some manuals, but the algorithm isn't mentioned. There are references to tissue groups and half times, so that makes me believe they use some variant of the Buhlmann model. But yes, unknown/unsupported is probably more correct.
http://www.suunto.com/en-US/Support/Suunto-rgbm-dive-algorithms/ http://en.wikipedia.org/wiki/Suunto#Diving_computers_and_instruments
And yes, we should use this for all RGBM - the "Suunto fused" was meant as an example in the comment.
I already assumed that, but the "Suunto fused RGBM" comment might be confusing. Giving the full "Reduced gradient bubble model" name is maybe more descriptive?
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').
The actual prefix will depend on how we name the enum, but yes we should be consistent and use the same prefix for all enum values.
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.
I guess only the GF's are a (defacto) standard. Everything is more or less some manufacturer specific tweak of the algorithm. So I think it's not unreasonable if we don't support those. If you really need to have that much detail, you should probably stick to the manufacturer's application.
For Buhlmann we could maybe also include the number of tissues and the A, B or C variant for the coefficients. It seems VPM also has a few variants, like VPM-A and VPM-B. But as you mention in your other email, this might already be overkill, because in most cases we don't have this kind of detailed info.
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.
I'm fine with that too, otherwise I wouldn't have proposed it :-)
Jef
On Fri Oct 17 2014 09:15:07 GMT+0100 (IST), Jef Driesen wrote:
On 2014-10-16 20:16, Dirk Hohndel wrote:
On Thu, Oct 16, 2014 at 05:09:06PM +0200, Jef Driesen wrote:
[...]
VPMGFS also has one GF. In the other mail the sat/desat factor was mentioned. I don't know what else is out there.
I guess only the GF's are a (defacto) standard. Everything is more or less some manufacturer specific tweak of the algorithm. So I think it's not unreasonable if we don't support those.
It is very interesting to see the last sentence, because similar opinion is quite often seen on this mailing list and leads to a conclusion that some other design has to be chosen because there is no info from manufacturer or some reverse enigineering is needed to support something. And indeed, this project has reverse engineering imprinted in its bones. But maybe sometimes it is OK to simply tell people - if you get info from manufacturer then we will support this piece of information, otherwise you have to live with this or buy something else.
So I would support VPM variants fully. To know if it was VPM-B or VPM-B/E is as important as to know if it was VPM-GFS (both B/E and GFS introduce more conservatism or simply fix original VPM problems). If it is unknown then stick unknown and let the users to do their job with manufacturer support call line.
Regards,
w
On Fri, Oct 17, 2014 at 10:15:07AM +0200, Jef Driesen wrote:
- 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
The DC_STATUS_UNSUPPORTED is used in case a feature is not supported by the dive computer, or not implemented by the backend. The line between those two is very thin. If it's not implemented by the backend, that could be due to the fact that it's not supported by the dive computer, or because we don't know yet where and how it's stored. But usually we don't know that difference in advance.
The same logic applies here. Of course every dive computer will use some deco model (unless in gauge/freedive mode), but for libdivecomputer unsupported and unknown are basically the same thing.
I won't push too hard on this one. I disagree with you, but that doesn't mean that I'm right.
Now, Linus is sitting next to me on the lounge (we're connecting in Atlanta on our way home... don't ask) and came up with an embarrassingly simple solution to all this. And I hate to say it, but I think he's right.
Given that every vendor messes with their implementation, invents their own secret sauce and no one really really correctly implements the (often not well defined) algorithms... how about we just return a string?
On the OSTC one could return "Bühlmann ZHL-16c GF 30/70 sat 110 desat 90" On the Aeris one cour do "Bühlmann ZH (params unknown)" etc
This avoids the complication with the parameters and basically allows us to give back what we know, without having to try to invent too much structure around it.
What do you think?
/D
On Fri, Oct 17, 2014 at 1:25 PM, Dirk Hohndel dirk@hohndel.org wrote:
Given that every vendor messes with their implementation, invents their own secret sauce and no one really really correctly implements the (often not well defined) algorithms... how about we just return a string?
Btw, strings is what the EON Steel returns natively. So it will ave things like
DeviceLog.Device.Name: "EON Steel" DeviceLog.Device.SerialNumber: "#p1437019" DeviceLog.Device.Info.SW: 1.0.3 DeviceLog.Device.Info.HW: "71.2.0" DeviceLog.Device.Info.BatteryAtStart: "Charge: 73%, Voltage: 3.959V" DeviceLog.Header.Diving.Algorithm: "Suunto Fused RGBM" DeviceLog.Header.Diving.Gases.Gas.TransmitterID: "1437137031"
that are fundamentally strings (they are returned as utf8, even the ones that are mostly numerical).
(Also, interestingly, it saves the firmware version per dive: that "1.0.3" version is the pre-release one we were diving with, but I have since updated the device to the final 1.0.7 firmware, but the dives are reported with the firmware-at-the-time)
I would really prefer to see libdivecomputer use more plain *strings*. We talked about this long long ago about the serial number too: trying to encode things as numbers is fragile as hell, and it's also very complicated because you need to specify (and often over-specify) all the details. A string, in contrast, doesn't need much specification at all, and allows for obvious extensions (ie maybe the device is doing something that is very similar to Bühlmann, but has some vendor extensions - a string can just *say* so, while some crazy enumeration model will inevitably either be overly complicated, or won't be able to express the fact that it's not just Bühlmann, it's some vendor variation there-of).
Strings are good. The UNIX people taught people that long long ago. Back in the early sixties, it was ASCII. These days it's utf8, but aside from that minor change, the basic rule has remained the same. Freeform text really is very very powerful.
This is *especially* true for random data that is really informational, and not to be "plotted". Yes, you might want to plot the deco data, but if we plot that in subsurface, it will be with *our* algorithms (currently just Bühlmann 16 with gf, maybe that will change in the future), and at best that will be just something that we can try to match against whatever the divecomputer vendor claimed they did.
Linus
-
Artur Wroblewski -
Dirk Hohndel -
Jef Driesen -
Linus Torvalds -
wrob311＠gmail.com