(I re-added some cuts by Phil to answer both posts in one) > On Sunday 22 June 2008, Phil Dibowitz wrote: > > On Sunday 22 June 2008, Stephen Warren wrote: > > > A "pulse" is usually a low->high->low transition, whereas these docs > > > talk about pulses and spaces. There should be differentiation between > > > pulses (mark followed by space), marks (high time), and spaces (low > > > time)
I actually wasn't really happy about the naming either initially, but missed to change it to something more appropriate, and then got used to it... I have changed it now in libIRremotes to ir_signal and ir_signal_length and will adapt libconcord accordingly, also making the documentation clearer, as you suggested below. > > > In the documentation for get_key_name, it may be worth mentioning that > > > valid index values start at 1 anything wrong with "index starting at 1 for '1'st entry"? > > > and increment by 1 until NULL is returned. Well, probably some things just become too obvious when you have been working on them for several days... > > > I really don't like the static buffer used by get_key_name. It's nasty > > > that a client can only use this API on a single data blob at a time. > > > > So the question is - do we want to support more than one file AT THE > > SAME TIME? Given the current API you can in fact go through more than > > one file at a time. So being pragmatic, it's not so bad. > > Thus I've gone back and forth a few times, and can't quite decide. I currently don't see any use for being able to handle multiple files in parallel - maybe sequential, in case someone comes up with a concordance daemon lurking 24/7 in the background - but parallel? Multiple parallel connections to Logitech, or collecting a bunch of downloaded files and procesing them in parallel? It's all still driven by the software on the Logitech servers, anyway... > > > that the API internally maintains non-obvious implicit state. I'd > > > prefer something more like the API below, where the XMLBuffer is > > > dynamically allocated per iterator, but the type/content is still > > > opaque to the client: > > > > > > void *keyiter_create(uint8_t *data, uint32_t size); > > > char *keyiter_next(void* keyiter); > > > void keyiter_destroy(void *keyiter); > > > void keyiter_key_destroy(char *); > > > > > > No reversing or random access allowed either; should be much simpler. > > > > So I guess the idea with this feature is being able to go "oh, damn, I > > hit the wrong button on the other remote and don't want to have to go > > back and redo the whole sequence." I like that. To which Stephen replied: > At least in congruity, I will make the client application use > get_key_name to retrieve a list of all known key names into a Python > list, and then iterate back/forth within that. Well, concordance could do that as well, so what about: char **get_keynames(uint8_t *data, uint32_t size); void destroy_keynames(char **names); and be done with the XML data in one call? > > > Please document that the memory pointed at by get_key_name's return > > > value needs to be free'd by the client. libconcord needs an API for the > > > client to call to do this. > > > encode_for_posting needs to define who owns the memory returned, and > > > how to destroy it. I started adding deallocators in libIRremotes, and will do so for libconcord. I'm not sure about the internals of python - will deallocators be of any use there, and when should they be called? > > > post_new_code and encode_for_posting check all the output/pointer > > > parameters against being NULL, but learn_from_remote doesn't. Guess I thought anybody so stupid to call learn_from_remote(NULL,NULL,NULL) should deserve a SIGSEGV... well, adding a check won't hurt, and may be useful when interfacing to some other language.. > > > I'm not convinced that the Python bindings changes for "verbose" are > > > the correct way to go; they don't handle exceptions very well. > > > > I didn't read through those changes, but I would say that seems > > reasonable to be able to enable debug in a production version so you can > > tell your users to do this when they report some problem you can't > > reproduce. At least it was helpful to me in some stage of the develpment to see what was going on and coming out - I also don't see any big difference in the exception handling - except maybe for the case that 'print' in the wrapper itself throws an exception (is that likely?). > > > It'd be best if you removed the "verbose" part from the Python bindings as you like it..There's probably also still to much 'verbose' stuff in my proposal for congruity (e.g. it prints geometry data when you zoom the IR signal plot), and probably nobody is actually interested in the way the signal is encoded for posting to Logitech either...some information needed while coding is surely useless once you've got the code right, so what should a verbose mode show? An enthusiast may enjoy to see the exact mark/space durations and even feed them into some other decoding tool, while a plain user will just think 'what the ....?' And-probably going to fix a few of these things in libconcord tomorrow-reas ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://sourceforge.net/services/buy/index.php _______________________________________________ concordance-devel mailing list concordance-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/concordance-devel
