Next round of api improvements.

[Dirk Hohndel]

Hi Jef,

Thanks for the detailed summary email. That's really helpful.

Overall I think we made great progress here. To keep this email
manageable I'll cut everything out where I would otherwise just say
"excellent - I'm glad that's the direction we are taking" :-)

Jef Driesen <jefdriesen at telenet.be> writes:
> There has been some discussion regarding the exact semantics of the 
> temperature field. On one hand it's important to clearly define the 
> exact semantics, but on the other hand the device may provide one or 
> more temperature temperature values with some different semantics. For 
> example we can define the temperature field to be the minimum 
> temperature, but the device can provide the minimum, maximum or average 
> value, or the temperature before and after the dive, or the temperature 
> at maximum depth, or one or more of the above. How do we support this?
>
> One option is to keep the exact semantics vague, and just supply 
> whatever temperature value the device supplies. This is not ideal, 
> because it leaves applications that care about the exact interpretation 
> in the cold, and it also doesn't work well in case the device provides 
> more than one value.
>
> An alternative option is to move the temperature data into the samples. 
> All the values mentioned above can easily be attached to the correct 
> sample. You still won't get a full temperature profile (only a few data 
> points), but at least their is no confusion about the semantics. For 
> example a temperature value attached to sample at maximum depth, is 
> clearly the temperature at max depth, and so on.
>
> Another suggestion was to use flags to make the exact semantics clear. 
> For example if the default semantics is the minimum temperature, provide 
> a flag to overrule this interpretation with another one (e.g at max 
> depth, etc).
>
> struct temperature {
>      flags;
>      value;
>      ... /* Maybe multiple values here? */
> };
>
> And last but not least we can provide a struct with all possible 
> values:
>
> struct temperature {
>      minimum;
>      maximum;
>      average;
>      ...
> };
>
> and then provide some means to let the application know which values 
> are available (using a bitmap again, or some magic values to indicate 
> "not present").
>
> No decision for the temperature issue has been made yet.

Since I was somewhat outspoken on this before, I'll comment again.

The one solution that I don't like is "let's keep it vague and provide
some value where it is rather unclear what it might mean". That is in
general bad API design - especially as in many cases we do know exactly
what the temperature is supposed to mean.

I still like the idea of the flags where one flag could indeed be
DC_WE_DONT_KNOW_EXACTLY_WHAT_TYPE_OF_TEMP_THIS_IS (ok, maybe a shorter
name would be better). But as an application author, if the backend
KNOWS that this is air temperature before the dive, then I want it to
tell me. And if the backend knows that this is the the minimum water
temperature (vs. the temperature at the deepest spot), then I want to
know that, too. Flags are not the only possible implementation - but
they seem a straight forward one. And usually "simple is good".

> 3b. Sample data
> ================
>
> The basic list doesn't need much discussion:
>
>   * time
>   * depth
>   * temperature
>   * tank pressure
>
> There are also a couple less important ones, but as long as they are 
> reasonable device independent, I see no reason not to support them too:
>
>   * heartrate
>   * airtime
>   * CNS/OTU
>   * ...
>
> Note that this is definitely not a complete list. We'll probably need a 
> few extra types for tec divers (setpoints), and there may be others that 
> I forgot.
>
>
> There has been some discussion regarding the CNS values (but the 
> underlying discussion applies to any other type too). The natural unit 
> for the CNS values is obviously a percentage. But there are devices that 
> don't supply a percentage. For example the Suunto has OLF (Oxygen Limit 
> Fraction), which is hybrid between CNS and OTU (e.g. whichever is 
> highest), and Oceanic has their 02 bar graphs. Although both give some 
> indication of the CNS loading, they are not easily translated to a 
> percentage.
>
> What to do in this case? Just not support it, or again provide some 
> flag with the exact interpretation?

You know what I'll say... I'd love to get the data that is there plus a
flag that helps me make sense of it. The Suunto OLF could be roughly
mapped to percentages (in the application, not in libdivecomputer), I
guess (especially if the app does it's own CNS calculation and then can
show the data from the dive computer as additional data to the user).

> 3c. Events
> ===========
>
> The current proposal is to reserve the events exclusively for things 
> that have no duration (and thus no need for the begin/end flags) and 
> don't need to carry any data values. In that case, we can use a simple 
> bitfield to get all the events for the sample at once.
>
> dc_sample_get_available (sample, &bitmap);
>
> if (bitmap & DC_SAMPLE_EVENTS) {
>      dc_sample_get_events (sample, &events);
>
>      if (events & DC_SAMPLE_EVENT_BOOKMARK) {
>          /* Bookmark event. */
>      }
>      if (events & DC_SAMPLE_EVENT_VIOLATION) {
>          /* Decostop violation event. */
>      }
> }
>
> While this should work well for true events like the decostop 
> violations, bookmarks, etc we'll need another solution for things like 
> gas changes, decompression info, etc. The idea is deliver this info as 
> sample values, rather than events.

Yay! Yay! Yay!
(oops, I said I would remove things where I just wholeheartedly agree,
but this one I am really, really happy about :-) )

> For example the gasswitch event would be changed into an "active 
> gasmix" sample value. The main difference is that you will now get the 
> current gasmix on every sample, and not just the gaschange. For the 
> gasmix sample value, the corresponding data type would be the index of 
> the gasmix in the header (see above).
>
>
> Decompression data can be supported in a similar way. For each sample 
> you can get the deco status, with a data structure like this:
>
> dc_deco_t {
>      enum {NDL, DECOSTOP} type;
>      time; /* Optional */
>      depth; /* Optional */
> };
>
> If you're not in deco, the type will be set to NDL, and the time 
> contains the remaining no decompression limit (if available). If you're 
> in deco, the type will be set to DECOSTOP, and the time and depth 
> contain the time and depth of the next decompression stop (if 
> available). With such a structure we can support both devices that 
> provide just a simple ndl/deco flag, or a some more detailed info.

BTW: I ran into a fun special case when implementing this in Subsurface
(which, coincidentally stores all the ndl/deco info in samples...): The
special case is the safety stop. You have NDL remaining (obviously) and
you have a stop (weak obligation) with a time and depth.  Subsurface
doesn't show a ceiling for such stops but shows detailed information if
your mouse hovers over the profile.

I don't think it's necessary to have an additional time value in the
dc_deco_t (because the NDL at the safety stop is usually close to
infinity and isn't really a critical piece of information at that
stage), but maybe we could add a third value to the enum {NDL, DECOSTOP,
SAFETYSTOP} ?

> We already committed some changes to the master branch to move into 
> that direction. The DC_EVENT_DECOSTOP now contains the depth (in meters) 
> and time (in seconds) as two packed 16bit values, and a new DC_EVENT_NDL 
> was introduced. It's not perfect yet, because for the current api we 
> were limited to the existing data structures (hence the packing), but 
> it's a step in the direction of the model explained here.

Yes - and if I may say so, it's very appreciated by the Subsurface
team. I am happy with the stop-gap implementation in 0.3 and even more
happy with the direction this is going for the next-API.

> Of course we can consider to extend the type with deepstops and safety 
> stops, or add an extra field for TTS (Time To Surface), and things like 
> that. But the main point is that we move away from the event model as 
> much as possible.

Ahh - should have read the whole section before responding above.  

I think safety stop and deep stops could just be different enum values.
TTS would have to be a fourth member of the dc_deco_t structure; I would
still have it in there as the deco obligation is a major component of
TTS...


Thanks, Jef!

/D