On 2012-11-15 17:56, Mikko Rasa wrote:
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?
It's just a tentative name and can be changed.
- 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.
This is a bad idea. Some devices consume significantly more power when connected to the host system, and therefore the connection should be closed as soon as possible. But if the device object owns the dives, the connection can't be closed until the application is done with the dives.
- 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.
Real errors are rare, but they can happen. For the majority of the devices our knowledge is based on reverse engineering, so there is always a possibility that their are errors in our knowledge. Thus the most common parsing error is that we encounter some data that doesn't match our expectations (e.g. DC_STATUS_DATAFORMAT).
In theory we could catch some of those errors when constructing the dive/sample object, but then the error is pretty much fatal. I mean if the dc_xxx_foreach (or dc_iterator_next) function fails, there is no way to proceed anymore with the downloading or parsing. But in many cases it's still possible to move to the next dive or sample.
Take for example the Suunto data format. The dives are stored in a linked list, so you need zero knowledge of the actual dive format to locate and download the dives. Even if some of the dives will fail to parse correctly (for example when the data format has been changed to support a new feature), that has no impact on the ability to download the dives. If this data format error is detected by the parser while constructing the dive object, we have two options:
1. Fail to construct the dive object. This leaves the foreach() function no other option than to abort the downloading. The result is that a minor problem with just one dive will cause the entire download to fail.
2. Construct the dive object anyway in an error state. The application will still get the dive, and downloading will continue. It may not be able to parse this dive completely, but depending on the severity of the error you may still be able to get some info. For example the raw dive data will always be available. Some of the fields may still be parse-able too. But this only works if the individual accessor functions can return an error.
Note that in the current design this is less of a problem, because downloading the dives and creating a parser are completely decoupled. A failure in creating a parser will never affect the downloading.
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.
- 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.
There are several devices which support it. Uwatec, Oceanic, and probably a few other too. It may not be the most critical piece of information, but I do get requests for it. So while it's not on my must-have list, it's something we can consider adding.
You would be surprised what all those dive computers support. We definitely won't support everything, but we want to make a good trade-off between what we consider important and what is available.
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.
Do you have a suggestion for the api?
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.
I was referring to the dive temperature here, not the sample temperature. It makes indeed no sense to have min/max/avg for the samples.
Jef