On 01/16/2013 08:42 PM, Scott Talbert wrote:
> On Sat, 12 Jan 2013, Phil Dibowitz wrote:
> 
>>> Also, looking in my 'pending patches' directory, there are still a couple
>>> there that haven't been merged: the Harmony 300 patch (v4) and the patch
>>> to do config dumping for zwave (v6).  Although, these probably aren't
>>> super critical to get into a release, although the 300 support would be
>>> nice.
>>
>> Oh... I'll look for those, sorry.

The MH patch:

    if ((err = rmt->UpdateConfig(of->GetDataSize(), of->GetData(),
        cb, cb_arg, cb_stage)))
      return LC_ERROR_WRITE;
+ } else if (is_mh_remote()) {
+   if ((err = rmt->UpdateConfig(of->GetDataSize(), of->GetData(),
+       cb, cb_arg, cb_stage, of->GetXmlSize(),
+       of->GetXml())))
+     return LC_ERROR_WRITE;


You changed the virtual's signature, so can't this be combined into one
section of if?

+ const uint8_t msg_four[MH_MAX_PACKET_SIZE] =
+   { 0xFF, 0x01, 0x03, 0x02, 0x80, '/', 's', 'y', 's', '/',
+     's', 'y', 's', 'i', 'n', 'f', 'o', 0x00, 0x80, 'R', 0x00 };

LOL.

+ for (int i=0; i<5; i++) {
+   if ((err = HID_WriteReport(msg_one))) {
+     debug("Failed to write to remote");
+     return 1;
+   }
+ }

Why do we write msg 1 5 times?

+ while(!(err = HID_ReadReport(rsp))) {
+   // Ignore 1st two bits on 2nd type for length.
+   int len = rsp[1] & 0x3F;

You mean "1st two bits on 2nd byte" ?

+   // Skip 1st two bytes, read up to packet length.
+   for (int i=2; i<len+2; i++) {

Do you mean len-2?

+int CRemoteMH::SetTime(const TRemoteInfo &ri, const THarmonyTime &ht,
+ lc_callback cb, void *cb_arg, uint32_t cb_stage)
+{
+ return 0;
+}

Why isn't this LC_ERROR_UNSUPP?

+ const uint8_t msg_three[MH_MAX_PACKET_SIZE] =
+   { 0xFF, 0x01, 0x01, 0x03, 0x80, '/', 'c', 'f', 'g', '/',
+     'u', 's', 'e', 'r', 'c', 'f', 'g', 0x00, 0x80, 'W', 0x00,
+     0x04, (len & 0xFF000000) >> 24, (len & 0x00FF0000) >> 16,
+     (len & 0x0000FF00) >> 8, len & 0x000000FF };

These crack me up. :)

+   if (pkt_count == 50) {
+     if ((err = HID_ReadReport(rsp, MH_TIMEOUT))) {
+       debug("Failed to read from remote");
+       return 1;
+     }
+     print_packet(rsp);

print_packet calls debug, so it'll get compiled out, but it's not obvious to
the reader. Can we call it debug_print_packet or something?

+     /* 2nd parameter is the number of packets remaining,
+        plus one */
+     if (pkts_to_send < 50)
+       ack_rsp[7] = pkts_to_send + 1;

And if it's more than 50? You don't fill it in at all?

+ const uint8_t finish_msg[MH_MAX_PACKET_SIZE] = {

Jesus, these guys aren't playing around...

+ debug("msg_5");
+ print_packet(rsp);
+
+ cb(LC_CB_STAGE_FINALIZE_UPDATE, cb_count++, 3, 4,
+   LC_CB_COUNTER_TYPE_STEPS, arg);
+
+ /* write msg 7 */

Huh? what happened to 6?



-- 
Phil Dibowitz                             p...@ipom.com
Open Source software and tech docs        Insanity Palace of Metallica
http://www.phildev.net/                   http://www.ipom.com/

"Be who you are and say what you feel, because those who mind don't matter
 and those who matter don't mind."
 - Dr. Seuss


Attachment: signature.asc
Description: OpenPGP digital signature

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________
concordance-devel mailing list
concordance-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/concordance-devel

Reply via email to