On Tue, 2 Apr 2013, Phil Dibowitz wrote:

> I considered asking of we should replace _update_configuration_mh since it's
> identical to _update_configuration_zwave()... but since they're just a single
> function call away, I think the abstraction is cleaner.

Yeah, I thought about that too.

> Only two minor comments before I merge:
>
> +void debug_print_packet(uint8_t* p)
> +{
> + debug("%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x " \
> +   "%02x %02x %02x %02x",
> +   p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7], p[8], p[9],
> +   p[10], p[11], p[12], p[13], p[14], p[15]);
> +}
>
> Do we perhaps want to make this a #define? I realize that debug() is a #define
> that gets compiled out in normal compile and so this becomes an empty
> function, which I really hope GCC compiles out, but it may be more clear
> what's going on. I don't feel that strongly.

Eh, it probably gets compiled out, but even if not, it's probably not a 
major performance impact.

> + /* reset the sequence number to 0 */
> + if ((err = HID_WriteReport(msg_six))) {
> +   debug("Failed to write to remote");
> +   return LC_ERROR_WRITE;
> + }
>
> Is that really what msg_six does, or is that an errant comment?

That's what it does, or at least what I think it does.  :)

------------------------------------------------------------------------------
Minimize network downtime and maximize team effectiveness.
Reduce network management and security costs.Learn how to hire 
the most talented Cisco Certified professionals. Visit the 
Employer Resources Portal
http://www.cisco.com/web/learning/employer_resources/index.html
_______________________________________________
concordance-devel mailing list
concordance-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/concordance-devel

Reply via email to