Code review from v5 first, then replies below that. +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. + /* 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. - 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. + 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. > 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. > I already added a comment very similar to this in the destructor. Do you > think we need more than that? Ah, that's fine. > 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. Testing: 885 (hid): works (minor delay on reboot, but that's OK) 895 (hybrid): works 1100 (usbnet): getinfo, gettime, settime all work[1] [1] well getinfo worked, then I unplugged it, and thought I should try get/set time, plugged it in and the usb subsystem crashed and I had to powercycle. After reboot, I plugged it in, and it still thought it had it's IP and wouldn't DHCP. I unplugged it, messed around with the menus, re-plugged it in and everything worked as expected. I have a feeling that config update will work with little or no tweaking - I've emailed Ernst, the awesome guy who donated his old 1100 and asked for a config update for his current one that I can use against this one. -- 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
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ 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