Andreas Schulz wrote: > TODO: > ===== > While I managed to adapt the python bindings on my own, I would appreciate > any support to adapt the PERL bindings as well.
Perl bindings are a bitch. I'll apply the patch without them and do the
binding support myself.... but do you think you can do me a favor and flesh
out the perl driver to use these functions? (test.pl) That would save me time.
> Any volunteers for testing on a Macintosh?
My mac died... I may be able to use one at work... I'll get it tested before
a release, but our mac support isn't solid enough for me to hold up applying
the patch though.
Thanks for the detailed comments - awesome!
I have some minor style comments below nothing huge. The only big question I
have really crosses between patch 1/2 and 2/2:
Why aren't you using a callback in learn_from_remote() (which would, I
assume pass through to LearnIR)? I realize one wasn't there before, but I
think we can all agree the code that was there before was crappy. =)
You sorta fake it up in concordance.c, but LearnIR() has a lot of knowledge
of progress, it seems and a better progress meter could be given to the user.
(I haven't finished reviewing path 2/2 yet, that's coming shortly).
===============================================
> -#include <string.h>
> +#include <string>
>> fixed VC++ warning about deprecated include
I can't reproduce it now, but I seem to remember this doing weird things in
linux for old c-string functions. Have you tested this on Linux?
+ /*
+ * to be really paranoid, we would have to narrow the search range
+ * further down to next <PARAMETER>...</PARAMETER>, but IMHO it
+ * should be safe to assume that Logitech always sends sane files:
+ */
Spacing issue there. Well, two. The 4th line is inconsistent (but right),
and the 2nd, 3rd and 4th are wrong.
I noticed you use a do-while instead of a while-do. Is there a reason? If
they're the same upon entrance to the function, should you still run?
Do-while should only be used in that scenario (despite it's regular use in
our code).
+ std::list<string> key_list;
Just do a:
using namespace std;
instead.
Normally I wouldn't nitpick on this, but since you'll be changing this
anyway would you mind switching these:
return 0;
+ /* C++ should take care of key_name and key_list */
Put the comment before the return?
+ delete [] ir_signal; /* allocated by new[] -> delete[] */
Your inconsistent here - within the same line as well as with your previous
code. This should be "delete[] ir_signal"
libconcord/remote.cpp
+ if (err != 0) { return err; }
Please don't do this - multilines. I spent a long time cleaning this stuff up.
libconcord/bindings/python/libconcord.py
Stephen - can you please do a code review on this? I'm actually learning
python, but I'm not yet qualified to do a code review.
--
Phil Dibowitz [EMAIL PROTECTED]
Open Source software and tech docs Insanity 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
------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________ concordance-devel mailing list concordance-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/concordance-devel
