Next round of api improvements.

[Mikko Rasa]

On 15.11.2012 13:16, Jef Driesen wrote:
> Note that this new dc_dive_t interface will also simplify the
> transition from a callback based interface to an iterator based
> interface, because the dc_iterator_next() function can now return the
> dc_dive_t object:
>
> dc_dive_t *dive;
> dc_iterator_t *iterator;
> dc_device_iterator (device,&iterator);
> while (dc_iterator_next (iterator,&dive) == DC_STATUS_SUCCESS) {
>      /* Contents of the callback function goes here. */
> }
> dc_iterator_free(iterator);

dc_device_iterator sounds like it would iterate devices.  Perhaps it 
could be called dc_device_iterate_dives, or dc_device_dive_iterator, or 
some variant thereof?

> 1. Object ownership, lifetime and memory management.
> =====================================================
>
> How should we deal with the ownership of the new objects?
>
> With the current api, the application has to call dc_parser_new()
> function explicitly, and their is no doubt that the application is the
> owner of the returned object. However, because the new dive and sample
> objects will be allocated internally, their ownership is not immediately
> obvious. It all depends on the internal implementation. There are a
> couple of options, each with their own advantages and disadvantages:

What about utilizing the object hierarchy?  Make the device object own 
the dives, and each dive object own its samples.  This gives the 
application control over their lifetime, but doesn't require freeing 
everything separately.

The downside is a larger memory footprint while parsing, since all dives 
would be in memory simultaneously.  I don't think it's an issue, but if 
it is, you could provide a dc_device_discard_dives function to get rid 
of unwanted dives.

On the other hand, this would save some work when iterating over the 
same dives a second time.

> 2. Querying available sample values?
> =====================================
>
> I think the simplest solution is to use the accessor function itself to
> probe for the presence of the info. You just call the accessor function
> and if the info isn't available, a special error code like UNSUPPORTED
> is returned:
>
> if (dc_sample_get_depth (sample,&depth) == DC_STATUS_SUCCESS) {
>      /* Depth retrieved successfully. */
> } else {
>      /* No depth available (or an error occurred). */
> }

Are there any legitimate error conditions other than that piece of data 
not being present in the sample?  I try to avoid out parameters due to 
the greater amount of scaffolding required in calling code when compared 
to using the return value, but then again I mostly write C++ which 
provides out-of-band error handling with exceptions.

> But then the caller somehow has to know the maximum value for n, and
> the api also has to communicate back the actual number of values
> returned. The second part is easy with an extra output parameter, but I
> don't know a nice solution for the first problem (other than a second
> dc_sample_get_pressure_count function).

A typical solution is to allow passing a null pointer for the array, in 
which case no data is written to it but the count is still returned.

> Alternatives for the probing approach outlined above, are an iterator
> like interface to iterator over the available sample values, or a
> function to retrieve a bitmap with the available sample values.

I went with the bitmap approach in a similar scenario.  It works well 
enough and produces more compact code.

> 3. Supported dive and sample data
> ==================================
>
>    * airtime

Does some computer actually report this?  If tank pressure is also 
reported, airtime can be calculated from that.  In my experience it's 
not too stable over the course of the dive, so it would be of little 
value, but perhaps others disagree.

> For the events, I'm not sure yet how the querying (see previous item)
> should be done. The normal samples are continuous values, and will
> usually appear at least every X sample, or not at all. But the events
> are discrete in nature and will only appear occasionally. Should this be
> done differently in the api?

I'd add a separate API for iterating events.  They're different enough 
from normal samples that trying to force them into one API is only going 
to cause headache down the road.

> We'll also have to agree on a data structure and the exact semantics
> for each of those pieces of data. Some won't need much discussion, but
> others will. For example take the temperature. A device can record a
> single value which could be the minimum, maximum or average temperature.
> It can have all three, or a full temperature profile. How do we deal
> with that? Things are not always as easy as they may look.

Minimum, maximum and average values belong in the dive, not in the 
samples.  I dunno what to do if it's not known which of those a 
particular computer provides, though.

-- 
Mikko