On Sat, 17 Mar 2012, Phil Dibowitz wrote:

> +void delete_file()
> +{
> + if (of)
> +   delete of;
> +}
>
> Lets call that something like delete_opfile_obj or delete_operations_file -
> the other one (delete_blob) is generic because you pass in various types of
> blobs you may get back from the API.

Done, v6.

> + /* In certain places, the data pointer is used to point to an area
> +    inside of the xml memory.  In those cases, we don't want to delete
> +    it.  Use the data_alloc variable to keep track of when we've actually
> +    allocated unique memory to it, and in those cases, delete it.
> + */
> + if (data && data_alloc)
>
> Please use the same comment style used everywhere else.

Done.

> -   hid.mfg = "Logitech";
> -   hid.prod = "Harmony 1000";
> -   ri.flash_mfg = 0; // todo
> -   ri.flash_id = 0;  // todo
> +   ri.flash_mfg = 0x01; // Seems to be the same as regular zwave
> +   ri.flash_id = 0x49;  // Seems to be the same as regular zwave
>  } else {        // Not 1000
>    ri.flash_mfg = 0x01;// todo
>    ri.flash_id = 0x49; // todo
>
> If they're the same, pull them out of the if-else, please.

Done.

> + if (IsUSBNet()) {
> +   hid.mfg = ri.model->mfg;
> +   hid.prod = ri.model->model;
> + }
> +
>
> Is this the same as the if hid.pid == 0 check above? Why not just do that
> where it was being done before, in that if? I'm not opposed to changing that
> conditional to IsUSBNet() though.

Well, the problem is that the above needs to occur after the 
setup_ri_pointers() call.  I went ahead and moved the if hid.pid == 0 
stuff into the lower IsUSBNet() block, though, so now the earlier if/else 
is now gone.

>> Odd, so do HID ZWAVE remotes automatically reset after you upload a
>> config?  Or how do you get them to take the new config otherwise?
>
> Neither - nothing appears to happen, but you unplug the USB and it's just
> using the new config. I don't know what's really happening under the hood.

Very weird.

>> BTW, I know you wanted to move to that other bug report, but I seem to be
>> unable to attach files to it.  In the interim, I've posted v5 of the patch
>> here:
>> http://www.techie.net/~talbert/harmony_900_support_v5.patch
>
> Stupid SF tracker. I re-opened that bug as a code-review bug.

Yeah, I am thoroughly unimpressed with the SF tracker.

v6 posted in the tracker.

Scott

------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here 
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
concordance-devel mailing list
concordance-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/concordance-devel

Reply via email to