Re: [concordance-devel] libconcord new full patch (R e: Next try for the big IR learning patch. .)
On Tuesday 24 June 2008, Stephen Warren wrote: > On Tue, June 24, 2008 1:04 am, Andreas Schulz wrote: > In the delete_* functions, I'd use a more descriptive name for the > parameter than "garbage", e.g.: > > void delete_key_names(char **key_names); > ... > So, perhaps simply say this instead: > > * Memory allocated for the strings must be freed by the caller > > * via destroy_key_names() when no longer needed. done. > int get_key_names(uint8_t *data, uint32_t size, > char *** key_names, uint32_t *key_names_length); I had considered this, but somehow the triple '***' made me a little nervous to rewrite everything once again. Well, eventually I did now, and the API now really looks much better. > One could make a similar change to encode_for_posting, but since that's > always returning just one string, I'm not worried about that so much; your > call. Changed that also - adds consistency to the API to make all functions return status and write to parameters. The low-level function in web.cpp already worked that way anyway... I got the C parts mostly done - just allow for a few days for testing and building also for Windows (and splitting the patch into the learnig and the irremotes part...). > I would rename post_new_code's "code" parameter to "encoded", to hint that > it comes from encode_for_posting. > > Thanks for implementing this guys, and being open to API changes. You're welcome... that's the good thing about writing open source software - no boss sitting behind your back and counting the hours you need to get the job done... - 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
Re: [concordance-devel] libconcord new full patch (Re: Next try for the big IR learning patch..)
Stephen Warren wrote: > On Mon, June 23, 2008 4:53 pm, Phil Dibowitz wrote: >> Because you didn't ask for an LH type. You asked for a standard type. >> Strings, ints, etc. I think are standard. > > The type of an object has absolutely nothing to do with how it was > allocated; to return a pointer, code could malloc() memory, return a > pointer to a static (global) object, mmap() something, and I'm sure there > are more examples. Well, I was going to say "libc functions return char*s all the time"... but I can't find an example where it's not a pointer to a static string or something else similar that doesn't involve cleanup. Fair enough. -- Phil Dibowitz [EMAIL PROTECTED] Open Source software and tech docsInsanity Palace of Metallica http://www.phildev.net/ http://www.ipom.com/ "Never write it in C if you can do it in 'awk'; Never do it in 'awk' if 'sed' can handle it; Never use 'sed' when 'tr' can do the job; Never invoke 'tr' when 'cat' is sufficient; Avoid using 'cat' whenever possible" -- Taylor's Laws of Programming signature.asc Description: OpenPGP digital signature - 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
Re: [concordance-devel] libconcord new full patch (Re: Next try for the big IR learning patch..)
On Tue, June 24, 2008 1:04 am, Andreas Schulz wrote:
> So, maybe we can agree on the following interface:
That should work. Just a few mostly small comments:
In the delete_* functions, I'd use a more descriptive name for the
parameter than "garbage", e.g.:
void delete_key_names(char **key_names);
In the description for get_key_names:
> * Memory allocated for the strings must be freed by the caller
> * via destroy_key_names() when not needed any longer, i.e.
> * after all codes have been posted via post_new_code.
I would avoid specifically mentioning deleting the memory after
post_new_code, because some clients will probably do this:
char ** key_names = get_key_names(...);
duplicate key_names into some internal structure (e.g. Python strings in a
list)
delete_key_names(key_names);
process the internal structure
The internal structure could be created by duplicating all the strings and
indexing them in some different way. Note: The key names are obviously
required to call post_new_code later, but there's no requirement it be the
same actual string (memory location holding it, that is), but simply that
the content of the string must be the same.
So, perhaps simply say this instead:
> * Memory allocated for the strings must be freed by the caller
> * via destroy_key_names() when no longer needed.
Same for other "allocating" functions.
In get_key_names, rather than returning an array with a NULL entry to
indicate the end, how about this:
int get_key_names(uint8_t *data, uint32_t size,
char *** key_names, uint32_t *key_names_length);
and called/used like this:
char **key_names;
uint32_t key_names_length;
int err = get_key_names(d, s, &key_names, &key_names_length);
if (err != SUCCESS) {
// handle error here
return;
}
for (int i = 0; i < key_names_length; ++i) {
printf("Key name %d is '%s'\n", i, key_names[i]);
}
This:
a) Can allow more meaningful error status
b) Is explicit about the array length up front
One could make a similar change to encode_for_posting, but since that's
always returning just one string, I'm not worried about that so much; your
call.
I would rename post_new_code's "code" parameter to "encoded", to hint that
it comes from encode_for_posting.
Thanks for implementing this guys, and being open to API changes.
-
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
Re: [concordance-devel] libconcord new full patch (Re: Next try for the big IR learning patch..)
On Tuesday 24 June 2008, Phil Dibowitz wrote: > Andreas Schulz wrote: > > As I just recognized, better make that delete_*, to be consistent > > with delete_blob.. > > Delete blob always deletes the same thing - in your case you have lots > of different things.. Sure, by 'delete_*' I meant of course to rename each of them, not all to one, giving: > ... > void delete_key_names(char **garbage); > void delete_ir_signal(uint32_t *garbage); > void delete_post_string(char *garbage); - 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
Re: [concordance-devel] libconcord new full patch (Re: Next try for the big IR learning patch..)
Andreas Schulz wrote: > On Tuesday 24 June 2008, Andreas Schulz wrote: >> So, maybe we can agree on the following interface: > ... >> void destroy_key_names(char **garbage); > ... >> void destroy_ir_signal(uint32_t *garbage); > ... >> void destroy_post_string(char *garbage); > > As I just recognized, better make that delete_*, to be consistent > with delete_blob.. Delete blob always deletes the same thing - in your case you have lots of different things.. -- Phil Dibowitz [EMAIL PROTECTED] Open Source software and tech docsInsanity Palace of Metallica http://www.phildev.net/ http://www.ipom.com/ "Never write it in C if you can do it in 'awk'; Never do it in 'awk' if 'sed' can handle it; Never use 'sed' when 'tr' can do the job; Never invoke 'tr' when 'cat' is sufficient; Avoid using 'cat' whenever possible" -- Taylor's Laws of Programming signature.asc Description: OpenPGP digital signature - 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
Re: [concordance-devel] libconcord new full patch (R e: Next try for the big IR learning patch. .)
On Tuesday 24 June 2008, Andreas Schulz wrote: > So, maybe we can agree on the following interface: ... > void destroy_key_names(char **garbage); ... > void destroy_ir_signal(uint32_t *garbage); ... > void destroy_post_string(char *garbage); As I just recognized, better make that delete_*, to be consistent with delete_blob.. Andreas - 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
Re: [concordance-devel] libconcord new full patch (R e: Next try for the big IR learning patch. .)
So, maybe we can agree on the following interface:
/*
* IR-stuff
* ===
* Data structure information:
*
* carrier_clock: in Hz, usually ~36000..4
* ir_signal: IR mark/space durations (alternating) in microsconds
* ir_signal_length : total number of mark/space durations in ir_signal
* ir_signal will start with a mark and end with a space duration,
* hence ir_signal_length will always be an even number.
*
* They are usually filled in by calling learn_from_remote(...),
* to learn IR signals from an existing other remote, but may also
* be set by the application, e.g. be derived from Pilips Pronto Hex
* codes or RC5/NEC/... command codes (separate conversion library required).
*
* encoded posting format : IR code data converted to Logitech
* posting string format, returned by encode_for_posting.
* Having the encoding separate from the posting keeps the
* parameter list of post_new_code() tidy and allows the
* application to display the encoded signal when desired.
*/
/*
* Scans the contents of the received LearnIR.EZTut file
* (read into *data[size]) for the key names to be learned.
*
* Returns an array of strings containing the key names,
* terminated by a NULL pointer: ("a", "b", "c", NULL),
* or NULL in case of any error while scanning.
*
* Memory allocated for the strings must be freed by the caller
* via destroy_key_names() when not needed any longer, i.e.
* after all codes have been posted via post_new_code.
*/
char **get_key_names(uint8_t *data, uint32_t size);
void destroy_key_names(char **garbage);
/*
* Fills ir_data with IR code learned from other remote
* via Harmony IR receiver.
*
* Returns 0 for success, error code for failure.
*
* Memory allocated for ir_signal must be freed by the caller
* via destroy_ir_signal() when not needed any longer,
* i.e. after calling encode_for_posting
*/
int learn_from_remote(uint32_t *carrier_clock, uint32_t **ir_signal,
uint32_t *ir_signal_length);
void destroy_ir_signal(uint32_t *garbage);
/*
* Encodes IR code in to Logitech posting string format.
*
* Returns encoded string or NULL in case of failure.
*
* Memory allocated for the string must be freed by the caller
* via destroy_post_string() when not needed any longer,
* i.e. after calling post_new_code
*/
char *encode_for_posting(uint32_t carrier_clock, uint32_t *ir_signal,
uint32_t ir_signal_length);
void destroy_post_string(char *garbage);
/*
* Posts encoded IR-code with key_name and additional
* information from XML data[size] to Logitech.
*
* Returns 0 for success, error code for failure.
*/
int post_new_code(uint8_t *data, uint32_t size, char *key_name, char *code);
-
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
