On 02/09/2013 10:34 AM, Scott Talbert wrote: > On Tue, 5 Feb 2013, Phil Dibowitz 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. > > OK, I guess I'll back off on that piece. I left the set_time() move in > because I think that should at least be consistent between update_config() and > update_firmware(), unless you have a specific reason for that. > (update_firmware currently does a set_time whereas update_config does not. In > the update_config case, concordance was calling set_time directly.) > >>> 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. > > Updated in attached v3. Note that the MH and this patch are touching the same > area of code, so depending on which one gets merged first, the other will need > minor tweaking.
+ if (!noreset && !(is_z_remote() && !is_usbnet_remote()))
Ouch. I don't what's more clear that or this:
if (!noreset && (!is_z_remote || is_usbnet_remote)) {
No, yours more accurately reads to what we are saying. Hmm. That comment just
needs to be more clear.
// If reset is enabled (!noreset), we do reset, except that
// zwave-hid (is_z_remote() && !is_usbnet_remote()) doesn't need it.
// thus...
if (!noreset && !(is_z_remote() && !is_usbnet_remote())) {
I'll just update that for you when I commit. This looks good.
--
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
------------------------------------------------------------------------------ Own the Future-Intel(R) Level Up Game Demo Contest 2013 Rise to greatness in Intel's independent game demo contest. Compete for recognition, cash, and the chance to get your game on Steam. $5K grand prize plus 10 genre and skill prizes. Submit your demo by 6/6/13. http://altfarm.mediaplex.com/ad/ck/12124-176961-30367-2
_______________________________________________ concordance-devel mailing list concordance-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/concordance-devel
