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