On 07/22/2010 03:37 AM, Phil Dibowitz wrote:
On Wed, Jul 21, 2010 at 07:38:15PM -0600, Stephen Warren wrote:
Attached is a patch to support the Harmony 700.
OK, this patch was smaller than I expected. It all looks good - just a few
minor things I'd like to see fixed.
Updated patch attached.
Note: The new prep_config/finish_config functions are technically
required to match the Windows software. However, during my testing, I
forgot to update congruity to call those functions, and everything
worked as expected. Should I just rip those new functions out? That
would significantly reduce the size of the patch, and remove the need to
release a new congruity version too.
I have a suspicion we'll need this ability anyway, so lets keep them.
Perhaps it's worth adding a comment in the code to this effect though?
Yup, I added a comment.
+int prep_config()
+{
+ int err = 0;
+
+ if (ri.architecture != 14) {
+ return 0;
+ }
+
+ if ((err = rmt->PrepConfig())) {
+ return LC_ERROR;
+ }
+
+ return 0;
+}
When we prep/finish firmware we do the calls directly in libconcord.cpp.
Ultimately what you do in CRemote:: is just low level stuff, and
then we build on those here.
So I'd probably say it's best to put that stuff here...
However, I don't feel strongly, and if you want to push it down into the
CRemote object, then, I'd ask you to make the {prep,finish}_firmware match.
I went with moving the prep/finish_firmware into remote.cpp; it seems to
me that remote/remote_z might end up with different implementations (and
potentially even more variants based on remote model), and putting these
transparently behind the virtual class interface made sense to me.
@@ -117,7 +117,7 @@ int CRemote::GetIdentity(TRemoteInfo&ri
ri.architecture = rx_len< 6 ? 2 : rsp[5]>> 4;
ri.fw_type = rx_len< 6 ? 0 : rsp[5]& 0x0F;
ri.skin = rx_len< 6 ? 2 : rsp[6];
- ri.protocol = rx_len< 7 ? 0 : rsp[7];
+ ri.protocol = rx_len< 7 ? 0 : ((rx_len< 8) ? rsp[7] :
ri.architecture);
::wince::
Can you please break this up into a series of if's? The double-tertiary
operator is just evil.
Fixed.
- if ((err = (ri.architecture == 2)
- // The old 745 stores the serial number in EEPROM
- ? ReadMiscByte(FLASH_EEPROM_ADDR, FLASH_SIZE,
- COMMAND_MISC_EEPROM, rsp)
- // All newer models store it in Flash
- : ReadFlash(FLASH_SERIAL_ADDR, 48, rsp, ri.protocol))) {
+ switch (ri.arch->serial_location) {
+ case SERIAL_LOCATION_EEPROM:
+ err = ReadMiscByte(ri.arch->serial_address, SERIAL_SIZE,
+ COMMAND_MISC_EEPROM, rsp);
+ break;
+ case SERIAL_LOCATION_FLASH:
+ err = ReadFlash(ri.arch->serial_address, SERIAL_SIZE, rsp,
+ ri.protocol);
+ break;
+ default:
+ debug("Invalid TArchInfo\n");
+ return LC_ERROR_READ;
+ }
+ if (err) {
Nice :)
-static const TArchInfo ArchList[11]={
+10 for reformatting this.
Oh, and I added the new functions to the Python bindings too, which I
forgot before.
? install
? concordance/.concordance.c.swp
? concordance/.deps
? concordance/.libs
? concordance/Makefile
? concordance/Makefile.in
? concordance/aclocal.m4
? concordance/autom4te.cache
? concordance/concordance
? concordance/config.h
? concordance/config.h.in
? concordance/config.log
? concordance/config.status
? concordance/configure
? concordance/libtool
? concordance/ltmain.sh
? concordance/stamp-h1
? consnoop/consnoop
? libconcord/.deps
? libconcord/.libconcord.cpp.swp
? libconcord/.libconcord.h.swp
? libconcord/.libs
? libconcord/.remote.cpp.swp
? libconcord/.remote.h.swp
? libconcord/.remote_z.cpp.swp
? libconcord/Makefile
? libconcord/Makefile.in
? libconcord/aclocal.m4
? libconcord/autom4te.cache
? libconcord/binaryfile.lo
? libconcord/config.guess
? libconcord/config.h
? libconcord/config.h.in
? libconcord/config.log
? libconcord/config.status
? libconcord/config.sub
? libconcord/configure
? libconcord/depcomp
? libconcord/install-sh
? libconcord/libconcord.la
? libconcord/libconcord.lo
? libconcord/libtool
? libconcord/libusbhid.lo
? libconcord/ltmain.sh
? libconcord/missing
? libconcord/remote.lo
? libconcord/remote_z.lo
? libconcord/stamp-h1
? libconcord/usblan.lo
? libconcord/web.lo
? libconcord/bindings/python/.libconcord.py.swp
? libconcord/bindings/python/libconcord.pyc
Index: concordance/concordance.c
===
RCS file: /cvsroot/concordance/concordance/concordance/concordance.c,v
retrieving revision 1.39
diff -u -p -r1.39 concordance.c
--- concordance/concordance.c 5 Jul 2009 13:46:56 - 1.39
+++ concordance/concordance.c 23 Jul 2010 00:24:10 -
@@ -467,6 +467,11 @@ int upload_config(uint8_t *data, uint32_
