On 03/10/2012 07:53 PM, Scott Talbert wrote:
> Since I exposed the micro version that the 900 appears to have, I went 
> ahead and exposed that through concordance.  It isn't 100% necessary, 
> though.  I think this was my only change to concordance.  If you'd prefer, 
> I could leave this out and my changes would be isolated to libconcord.

lgtm

> This only gets called when a non-usblan remote cannot be found, so it 
> won't come into play normally.  It will get called (probably) when a 
> non-usblan remote is rebooting (e.g., after a config update) but I think 
> it only hangs for 5 seconds until the TCP connect attempt times out.

I can test this, I seem to remember weird stuff happening if you didn't have a
driver attached, errors or hangs or something. It was one of those "If users
run into this it'd be bad" kinda things.

> For figuring out how to make the final contact to the website, for 
> example, in an update config.  Here's an example:
> 1. Write new config to remote.
> 2. Reset remote (calls deinit_concord()).
> 3. Contact website to tell it that everything's good.  Fails because the 
> operation file has been deleted.

Ah. Just nuke those lines then, no need to comment them out. However, we
should probably delete this... somewhere. Just to be clean.

> OperationFile was causing a glibc memory free error in its destructor 
> because, in some cases, the "data" pointer is being set to point to 
> something inside the xml structure.  Basically, it ended up trying to free 
> memory that it didn't allocate.  I put this in to make sure it only tried 
> to free memory it allocated.

Ohh!! Ha, I remember running into that and thinking I needed to track that
down. Good call... but add a comment, please.

> I'm not really entirely sure.  But the Harmony Software sends them to the 
> website, so I figured we should probably add them.  I don't think they are 
> technically required, though.  The connectivity test and update 
> configuration function seemed to work fine before I added them.

lgtm, lets keep 'em. We may need them for other functions, and it's good to
have them.

>> I'm starting to feel like the CRemoteZ_TCP class should be renamed CRemot=
>> eZ_USB.
> 
> Heh, probably.  :)

Wanna do that? Having 3 different definitions of TCP makes me cry.

> The usblan remotes don't seem to be packet-size limited like the regular 
> zwave remotes are.  1033 seems to be the largest packet that we currently 
> need to send (the COMMAND_WRITE_UPDATE_DATA can send up to a 1024 byte 
> data packet, plus the various header bytes, equals 1033).

Wanna turn that into a DEFINE with a sane name? And - I know it's not code you
were touching, but bonus points if you do it for the HID size too :)

> Well, there isn't really an ACK for us to check since for usblan remotes, 
> there isn't really an ACK.  Unlike the regular zwave remotes that seem to 
> have the two types of messages "UDP" and "TCP", the usblan remotes seems 
> to have just one type, I guess because you already have a real TCP 
> connection with the remote, there was no need to try and fake it out.  The 
> main problem with combining regular zwave stuff with usblan stuff is that 
> the response byte ordering differs, so we would have to do different 
> checks for each.  It could be done, but may not be worth the effort?

Hmm. Definitely not worth it at this point, given that... but it bugs me we
have so many functions that are _almost_ the same. I may come back to this at
some point.

>> This all still assumes we only need to write to one region, if I read it
>> correctly (which is probably true for the 900, but I'm fairly certain not=
>> true
>> for most zwave/usb remotes).
> 
> Well, the existing regular zwave code only writes to region 4 "USER 
> CONFIG" as well.  There isn't any code for updating firmware or anything 
> like that.

Yeah. I think with later remotes even doing a config update will have to write
to more than one region...

>>>     //ri.hw_ver_major =3D hw >> 4;
>>>     //ri.hw_ver_minor =3D hw & 0x0F;
>>
>> I wonder what's up with this.
> 
> Dunno, I left that as I found it.  :)

LOL, yeah, I was just commenting outloud. We'll leave that until we have more
zwave-usb remotes fleshed out.


>>> +int GetXMLUserRFSetting(char **data)
>>
>> Can you explain how this stuff goes together? Either in the docs/ directo=
>> ry or
>> at as part of the patch description or in comments, or some combination t=
>> hereof?
> 
> Yes.  This is another area where the Harmony software was grabbing some 
> data and I figured we should probably grab that data, too, so we can send 
> it to the website.  I don't know if it is actually required, though.

So we just make an HTTP request to the remote which gets us ... some data
which we send to the site? Oh - this is probably how you program/set/do stuff
with the RF extender thing...

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

------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing 
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
_______________________________________________
concordance-devel mailing list
concordance-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/concordance-devel

Reply via email to