Enumeration of supported models

[Jef Driesen]

Hi,

I have been prototyping the device enumeration code (item #14 on the 
roadmap). I have a couple of questions for which I would like to get 
some feedback from users of the api. It's mainly about the naming of 
functions and data structures.

> typedef dc_model_t {
>        const char *manufacturer;
>        const char *name;
>        device_type_t type;
>        unsigned int model;
> } dc_model_t;

I'm not that satisfied with the name "dc_model_t". I think it's too 
easy to confuse with the model code. Does anyone have some suggestions 
for a better name? The only alternative I came up with is the more 
general "dc_descriptor_t".

A similar question for the "device_type_t" and "parser_type_t" enums. 
As explained before (see item #10 on the roadmap), I would like to merge 
both into a single enum. A simple change, but I'm still looking after a 
good general name for the new enum. Currently I have two suggestions: 
"dc_backend_t" or "dc_family_t". The dc_backend_t name feels a bit 
heavyweight to my taste, because I usually use the term backend to refer 
to the device/parser handle itself, rather than its type. Any other 
suggestions?

And last but not least, how about the "manufacturer" and "name" fields? 
Everywhere else I'm using the term "vendor" instead of "manufacturer" 
(e.g. the vendor extensions in the events). Which name would you prefer 
here? And would you keep the "name" field, or rename it into "product". 
The full name of the device is in fact the concatenation of the 
manufacturer and the product name (e.g. Suunto Vyper). The reason for 
using two separate fields is to allow easy grouping by manufacturer in a 
GUI application. I assume most applications will use such grouping 
rather than showing one huge list to choose from, so having the two 
separated makes sense I think.

> One possibility would be an iterator style api, similar to this:
>
> dc_iterator_t *iterator = dc_get_supported_models ();

For retrieving the iterator, which name is preferred?

dc_xxx_get_iterator (xxx, &iterator)
dc_xxx_iterator (xxx, &iterator)
dc_xxx_iterate (xxx, &iterator)

I usually consider a getter (or setter) function as a lightweight 
function that just accesses some internal field of an opaque object. But 
depending on the underlying implementation this function could perform 
some more heavyweight action. Imagine we use the iterator concept for 
downloading dives too. There are several devices that maintain an index 
with the available dives. This index needs to be downloaded first to be 
able to locate the actual dive profiles. Downloading this index could be 
done when getting the iterator, or it could be delayed until you 
actually try to download the first dive. In the first case getting the 
iterator will perform some heavyweight I/O, while in the second case 
it's only creating a placeholder and the real work is done in the 
dc_iterator_next() function.

For functions that perform real work, I usually use a verb. For example 
the dc_device_read function. But in this case, it's not really the 
dc_xxx_iterate() function that will be doing the enumeration. The 
dc_xxx_iterator() function would fall somewhere in between the other two 
options.

> while (dc_iterator_next (iterator)) {
>      dc_model_t *model = dc_iterator_current (iterator);
>
>      /* Do stuff here */
>      dc_model_get_name (model);
>      dc_model_get_manufacturer (model);
>      ...
> }

For the iterator loop, an alternative would be to integrate the 
dc_iterator_current() functionality directly into the 
dc_iterator_next(), like this:

while (dc_iterator_next (it, &item)) {
    ...
}

The benefit would be a little less overhead, because there is only one 
function call left. I don't see any disadvantages. Of course you can't 
iterate without fetching the item, but why would you need to do that in 
the first place?

And last but not least, which status code should the dc_iterator_next() 
return to indicate: (a) an item has been returned, (b) the iterator has 
reached past the last item (e.g. an EOF condition).

Either we introduce some new status code for the EOF condition:

while ((status = dc_iterator_next (it, &item)) == DC_STATUS_SUCCESS) {
    ...
}
if (status != DC_STATUS_DONE) {
    /* Handle errors */
}

Or we use the new status code to indicate a data item has been 
retrieved:

while ((status = dc_iterator_next (it, &item)) == DC_STATUS_DATA) {
    ...
}
if (status != DC_STATUS_SUCCESS) {
    /* Handle errors */
}

Remember DC_STATUS_SUCCESS is equal to zero, and error codes are 
negative. So I think it makes sense to use a positive value (e.g. the 
number one) for this new status code, so it is clearly distinct from the 
real errors.

Jef