Re: [concordance-devel] Congruity Port to New API

2013-02-05 Thread Phil Dibowitz 
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> 24, (len & 0x00FF) >> 16,
+ (len & 0xFF00) >> 8, len & 0x00FF };

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 docsInsanity 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




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

Re: [concordance-devel] Congruity Port to New API

2013-02-05 Thread Phil Dibowitz 
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.
> 
> Here's the config dumping patch.

+   /*
+* Subtract both TCP (4) and UDP (2) headers from pkt[0] which is
+*  len - 1.  (len = pkt[0] + 1 - 6)
+*/
+   len = pkt[0] - 5;

That comment uses 'len' to mean two different things... which is SUPER
confusing. I had to read that several times to parse it correctly.

Also, you include the "size" header in your calculation of 'len', but I think
you mean "payload_size" which, if I understand correctly, would be pkt[0] - 6

-   memcpy(data, pkt + 1, len+3);
+   memcpy(data, pkt + 1, len+5); // include headers, minus the size

So wait - pkt[0] holds "length - 1", or length not including itself, or index
of the last byte, however you want to look at it.

So the size is is len+7 (6 bytes of header, plus the byte that holds the
size-1), right? so if the payload is say, 8, the tcp/udp headers are 6, the
first byte is "14" and the whole packet is actually 15 bytes.

So if you start at pkt+1, and you want the headers you want len - 1 + 6 bytes,
which is indeed len+5. It's confusing that this number changed by 2 when you
changed 'len' by 1, but I guess the old code was just broken.

That said, I THINK it's more obvious what's going on if the code goes like this:

  /*
   * pkt[0] describes the length of the _rest_ of the packet, not including
   * itself.
   * To get the payload size we subtract both the TCP (4) and UDP (2) headers:
   */
  payload_size = pkt[0] - 6;
  last_seq = pkt[2];
  last_ack = pkt[3];
  last_payload_bytes = payload_size + 3; // tcp payload size
  memcpy(data, pkt + 1, payload_size + 6); // plus headers
  return 0;

Or better yet, lets make two #defines:

  #define USBNET_TCP_HDR_SIZE 4
  #define USBNET_UCP_HDR_SIZE 2

And then the code becomes:

  /*
   * pkt[0] describes the length of the _rest_ of the packet, not including
   * itself.
   * To get the payload size we subtract both the TCP and UDP headers:
   */
  payload_size = pkt[0] - USBNET_TCP_HDR_SIZE - USBNET_UCP_HDR_SIZE;
  last_seq = pkt[2];
  last_ack = pkt[3];
  last_payload_bytes = payload_size + 3; // tcp payload size
  memcpy(data, pkt + 1,
 payload_size + USBNET_TCP_HDR_SIZE - USBNET_UCP_HDR_SIZE);
  return 0;


+// Searches for the "end of file" sequence in a set of two packets.  Returns
+// the number of bytes in the second packet up to and including the sequence.
+// Returns 0 if not found.  Parameters point to the data section of the 
packets.
+// Note for the first packet, only the last 3 bytes are provided.
+int FindEndSeq(uint8_t *pkt_1, uint8_t *pkt_2)
+{
+   uint8_t end_seq[4] = { 0x44, 0x4B, 0x44, 0x4B }; // end of file sequence
+   uint8_t tmp[57]; // 3 bytes from the 1st packet, 54 bytes from the 2nd
+   memcpy(&tmp, pkt_1, 3);
+   memcpy(&tmp[3], pkt_2, 54);
+   for (int i=0; i<54; i++) {
+   if (memcmp(&end_seq, &tmp[i], 4) == 0) {
+   return i + 1;
+   }
+   }
+   return 0;
+}
+

This ... is strange to me. For starters, you look at the FIRST 3 bytes of the
first packet, not the last 3 as stated in the comment. Second of all... Why do
you look at 3 bytes plus a full packet?

+int CRemoteZ_HID::ReadRegion(uint8_t region, uint32_t &rgn_len, uint8_t *rd,
+   lc_callback cb, void *cb_arg, uint32_t cb_stage)

You're still defining this as virtual:

+   virtual int ReadRegion(uint8_t region, uint32_t &len, uint8_t *rd,
+   lc_callback cb, void *cb_arg, uint32_t cb_stage);



+   if (!eof_found) {
+   eof_found = FindEndSeq(prev_pkt_tail, &rsp[5]);
+   if (eof_found) {
+   rlen = eof_found;
+   }
+   rgn_len += rlen;
+   memcpy(&prev_pkt_tail, &rsp[56], 3);
+
+   if (rd) {
+   memcpy(rd_ptr, &rsp[5], rlen);
+   rd_ptr += rlen;
+   }
+   }

Coming back to the FindEndSeq code from above... can you comment here whats'
going on? Or more importantly, why? :)


+   /* Return TCP state to initial conditions */
+   SYN_ACKED = false;

I know you said this isn't necessary to clear elsewhere, but while you're here
you want to clean it up in UpdateConfig?

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

"Be who you

Re: [concordance-devel] Concordance on the MAC

2013-02-05 Thread Phil Dibowitz 
On 02/03/2013 02:46 PM, Russell, J.J. wrote:
> Scott,
> 
> I will submit a feature request on Monday.  No, I had not planned on asking
> for the source code.  This is probably best left to the people who will be
> doing the work.

Ugh, it really bothers me when people do that with open source software.

Scott I was going to reach out, but I see you already did. Please keep me
informed, if you can't get source, I'll be happy to push harder.

-- 
Phil Dibowitz p...@ipom.com
Open Source software and tech docsInsanity 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




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

Re: [concordance-devel] Congruity Port to New API

2013-02-05 Thread Phil Dibowitz 
On 01/16/2013 08:35 PM, Scott Talbert wrote:
> On Sat, 12 Jan 2013, Scott Talbert wrote:
> 
>> Also, what do you think about moving the pre/post web function calls into
>> update_firmware/config()?
> 
> In case you were having trouble visualizing :), the attached is what I was
> thinking.  I think it makes the API just a little bit cleaner - all of the
> functionality related to updating the config or firmware can be done in a
> single call.

I don't see the benefit here. They're pretty discreet functions, and while
it's common to want to do them together, it's clearly not always the case.
Passing these configurations into an API on whether or not to call another API
seems more cumbersome than just letting the user call or not call that
additional API.

> I also rewrote some of the get_stages stuff to be a bit more dynamic, rather
> than have a bunch of arrays that are hard coded.

LOVE THIS.

-- 
Phil Dibowitz p...@ipom.com
Open Source software and tech docsInsanity 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




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

Re: [concordance-devel] Concordance on the MAC

2013-02-05 Thread andreas . schulz 
- Nachricht von s...@techie.net -
> Never got your previous message...

..just when I thought that I had mastered that jet-lag after two weeks..
OK, next try, now hopefully with the correct From: and To:
--- snip --

Hi there,
yes it's me, still occasionally browsing the mails from the mailing list.
As the one who first put Pronto Hex codes into concordance and congruity
I think it's about time for some clarification:

   The Pronto code input is available in congruity (implemented in the Python
code), which should also run on a MAC provided the corresponding Python
runtime and libconcord plus its Python bindings are available.

I also made a private version of concordance with Pronto code input back in
2008/9 with the Pronto stuff originally in a separate library (libir_remote,
somthing like that, have to look that up when I'm back home next week).

This library would eventually also include some more conversion stuff
between Pronto Hex, Logitech and different Standard codes (RC5, NEC, etc.).

I also posted a statically linked module to this mailing list for review
by Phil - somewhere around here:


Since Phil, IIRC, was busy with other stuff that time, and we somehow didn't
come to a conclusion how to do it right, my Pronto implementation slowly
died and never made it into the mainline of concordance.

Somewhere in 2010, HellG from the German and the official Logitech Harmony
forums seems to have compiled the latter version and distributed a Windows
build, initially also with sources (at least of concordance - libconcord
didn't require any change for this). I apparently never bothered (up to now)
to check whether he was actually using my code.

A discussion about it (in German, with HellG and myself participating) can
be found in the German Harmony forum:

which also includes a link to an archive with sources:
 (still working)
Eventually a stripped version (without any sources, just the Windows exe
and dll) was put by HellG into the Logitech forum and from there spread
over the world:

which is the unofficial, GPL-violating version we are now talking about.

Andreas



--
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