Re: [concordance-devel] Suggestion for some cleanup in libconcord.cpp
On Tuesday 01 April 2008, Phil Dibowitz wrote: > Applied. I made a tweak and renamed read_from_file to be _read_from_file, > but other than that, I applied it as-is. > > Though I think I may re-org that file a bit. Feel free as you like - it's just my 2p to keep things going and tidy (same for the bugfix). Sorry for the missing underscore - since it's even illegal in the code I've been paid for the last 15 years, it never came to my mind to start an identifier like that (though I just had to learn the hard way yesterday with _DEBUG)... - Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ___ concordance-devel mailing list concordance-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/concordance-devel
Re: [concordance-devel] Suggestion for some cleanup in libconcord.cpp
Andreas Schulz wrote: > On Monday 31 March 2008, Phil Dibowitz wrote: >> Now that Stephen's patch has been merged, did you want to re-submit your >> patch against the latest CVS? In theory it should make your patch a bit >> smaller... > > Yup - piece of cake now, with all that \0 termination or not stuff gone. > Makes some functions almost redundant. > I compiled the patch and did a few tests with up- and downloading files > from/to the remote that looked ok. Applied. I made a tweak and renamed read_from_file to be _read_from_file, but other than that, I applied it as-is. Though I think I may re-org that file a bit. -- 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://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace___ concordance-devel mailing list concordance-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/concordance-devel
Re: [concordance-devel] Suggestion for some cleanup in libconcord.cpp
Andreas Schulz wrote: > On Wednesday 26 March 2008, Phil Dibowitz wrote: >> Your patch looks very solid. However, your changing things that Stephen's >> API patch is ALSO going to change (in fact, his patch will make your patch >> simpler, you won't have to deal with that silly static-size vs dynamic-size >> thing). >> Can you wait until his patch is applied and then fix up yours accordingly? > > No problem, it will be a few days anyway before I will have time to dig > into the CVS again - I'll see what's left then... Andreas, Now that Stephen's patch has been merged, did you want to re-submit your patch against the latest CVS? In theory it should make your patch a bit smaller... -- 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://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace___ concordance-devel mailing list concordance-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/concordance-devel
Re: [concordance-devel] Suggestion for some cleanup in libconcord.cpp
Andreas Schulz wrote: > No problem, it will be a few days anyway before I will have time to dig > into the CVS again - I'll see what's left then... Cool! Stephen's patch should be merged in the next few days... >> Also, in learn_ir_commands, if reading the file fails, you just go into the >> generic LearnIR code. This is the wrong approach. If the user gave us a >> file, we should fail if the file wasn't able to be opened. > > I just had a look a the patch to verify, but AFAIK that's just what my code > does; it should do (2) only when file_name is null, otherwise if err != 0, > it should proceed from (1) to (3) and just return err (you may have noticed > that I prefer to code single exit points). You're right - I mis-read the patch... ignore my comment. :) -- 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://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace___ concordance-devel mailing list concordance-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/concordance-devel
Re: [concordance-devel] Suggestion for some cleanup in libconcord.cpp
On Wednesday 26 March 2008, Phil Dibowitz wrote:
> Your patch looks very solid. However, your changing things that Stephen's
> API patch is ALSO going to change (in fact, his patch will make your patch
> simpler, you won't have to deal with that silly static-size vs dynamic-size
> thing).
> Can you wait until his patch is applied and then fix up yours accordingly?
No problem, it will be a few days anyway before I will have time to dig
into the CVS again - I'll see what's left then...
> Also, in learn_ir_commands, if reading the file fails, you just go into the
> generic LearnIR code. This is the wrong approach. If the user gave us a
> file, we should fail if the file wasn't able to be opened.
I just had a look a the patch to verify, but AFAIK that's just what my code
does; it should do (2) only when file_name is null, otherwise if err != 0,
it should proceed from (1) to (3) and just return err (you may have noticed
that I prefer to code single exit points).
if (file_name) { ...
err = read_from_file(file_name, &x, &size);
if ( err == 0 ) {..
rmt->LearnIR(&ls); ..
if (post) Post(x, "POSTOPTIONS", ri, true, &ls, &keyname);
} // (1)
} else {
rmt->LearnIR(); // (2)
}
return err; // (3)
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://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
___
concordance-devel mailing list
concordance-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/concordance-devel
Re: [concordance-devel] Suggestion for some cleanup in libconcord.cpp
Andreas Schulz wrote: > Oops, wrong sender mail address - trying again... > > Hi, it's me again - the guy from the sourceforge forum with > the plan to teach the Harmonys Philips Pronto hex codes... Andreas, Your patch looks very solid. However, your changing things that Stephen's API patch is ALSO going to change (in fact, his patch will make your patch simpler, you won't have to deal with that silly static-size vs dynamic-size thing). Can you wait until his patch is applied and then fix up yours accordingly? Also, in learn_ir_commands, if reading the file fails, you just go into the generic LearnIR code. This is the wrong approach. If the user gave us a file, we should fail if the file wasn't able to be opened. Thanks! -- 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://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace___ concordance-devel mailing list concordance-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/concordance-devel
Re: [concordance-devel] Suggestion for some cleanup in libconcord.cpp
Andreas Schulz wrote: > The patch attached is compiled (LINUX) and tested by writing > back dumped firmware and configuration as well as a single IR > learn on my H-785 (concordance -i -v see below). > Patch attached gzipped to avoid line breaks in mail text. Just attaching will also prevent that, and helps to ensure more people read the patch. But I don't mind gzip'd if you prefer that. I'll have a look at it soon - gotta run to the airport in a minute. -- 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 - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/___ concordance-devel mailing list concordance-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/concordance-devel
[concordance-devel] Suggestion for some cleanup in libconcord.cpp
Oops, wrong sender mail address - trying again...
Hi, it's me again - the guy from the sourceforge forum with
the plan to teach the Harmonys Philips Pronto hex codes...
While looking at the IR learning code, I could not help
noticing that libconcord.cpp is cluttered all the same file
reading ops over and over again, so I thought it might be a
good idea to make a common function for all this repeating
open-read-close stuff.
The patch attached is compiled (LINUX) and tested by writing
back dumped firmware and configuration as well as a single IR
learn on my H-785 (concordance -i -v see below).
Patch attached gzipped to avoid line breaks in mail text.
BTW.: I'm trying in parallel to get used to VisualStudio 9.0
Express for WinXP, and found as first that it complained
about a line in remote_z.cpp:
> int CRemoteZ_TCP::Write(uint8_t typ, uint8_t cmd, uint32_t len,
> uint8_t *data)
>
> pkt[0] = service_type << 4 | (cmd >> 8) & 0x0F;
- stating that with (uint8_t)cmd >> 8, there will not much be left,
which sounds reasonable to me - probably something to be fixed?
Andreas
=== ./concordance/concordance -i -v
Concordance 0.13+CVS
Copyright 2007 Kevin Timmerman and Phil Dibowitz
This software is distributed under the GPLv3.
Requesting Identity: 100% done
Model: Logitech Harmony 785 (Corona)
Skin: 45
Firmware Version: 4.2
Firmware Type: 0
Hardware Version: 0.8
External Flash: 2 MiB - 01:49 AMD Am29LV160BB
Architecture: 8
Protocol: 8
Manufacturer: Harmony Remote 0-4.2.2
Product: Harmony Remote 0-4.2.2
IRL, ORL, FRL: 64, 64, 0
USB VID: 046D
USB PID: C110
USB Ver: 082D
Serial Number: {----}
{----}
{----}
Config Flash Used: 35% (690 of 1920 KiB)
---
libconcord.cpp.get_from_file.patch.gz
Description: GNU Zip compressed data
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/___
concordance-devel mailing list
concordance-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/concordance-devel
