[Rack] Building some new electronics to open the gate?

[Michael Shields]

On Sat, Nov 12, 2011 at 9:37 PM, Jonathan Lassoff <jof at thejof.com> wrote:
>
> This is my hack job: https://gist.github.com/1361655
> I'm still a little new to writing C from scratch (I'm usually modifying,
> porting, or fixing stuff), so I'd appreciate any feedback on this. It
> compiles and works on my machine, but that's the extent of my testing -- I
> still haven't tested it out at 2169.

This code looks pretty good overall.

It has a bug: if the packet received is larger than 255 bytes, the
buffer won't be NUL-terminated, so strncmp will read past the end of
it.  You can either check for this case, explicitly set the last byte
of command_buffer to NUL, or use memcmp.

is_buzzer_ringing() can return -1, but its only caller doesn't check for this.

You'll want to run it through gcc -Wall, which has a few suggestions:

    parport_daemon.c: In function ‘is_buzzer_ringing’:
    parport_daemon.c:76:3: warning: suggest parentheses around
comparison in operand of ‘&’ [-Wparentheses]

This is a style issue but I agree.  Using & instead of && and =
instead of == in a condition is such a common bug that there needs to
be some way to indicate that you know what you're doing.

    parport_daemon.c: In function ‘main’:
    parport_daemon.c:222:7: warning: implicit declaration of function
‘strncmp’ [-Wimplicit-function-declaration]

Fix this with #include <string.h>.

    parport_daemon.c:226:66: warning: passing argument 4 of
‘send_response’ from incompatible pointer type [enabled by default]
    parport_daemon.c:58:5: note: expected ‘char *’ but argument is of
type ‘char (*)[31]’
    parport_daemon.c:231:66: warning: passing argument 4 of
‘send_response’ from incompatible pointer type [enabled by default]
    parport_daemon.c:58:5: note: expected ‘char *’ but argument is of
type ‘char (*)[25]’
    parport_daemon.c:239:66: warning: passing argument 4 of
‘send_response’ from incompatible pointer type [enabled by default]
    parport_daemon.c:58:5: note: expected ‘char *’ but argument is of
type ‘char (*)[6]’
    parport_daemon.c:244:66: warning: passing argument 4 of
‘send_response’ from incompatible pointer type [enabled by default]
    parport_daemon.c:58:5: note: expected ‘char *’ but argument is of
type ‘char (*)[9]’

You mean to use r_acknowleged instead of &r_acknowledged, etc.  Or
&r_acknowleged[0] if you prefer.

By the way, you might well make these const.

    parport_daemon.c:168:9: warning: unused variable ‘childpid’
[-Wunused-variable]

Remove.

    parport_daemon.c: In function ‘send_response’:
    parport_daemon.c:65:1: warning: control reaches end of non-void
function [-Wreturn-type]

Return 0 at the end.

The macros on lines 37-42 aren't used and can be removed.

ringer_state and buzzer_state can be bools.  You might also want to
give them names like recently_buzzed.  If they could have more than
two states, an enum would be a good choice.