[Nut-upsdev] [PATCH] tripplite driver updates

[Arjen de Korte]

> The tripplite driver was developed on a machine with a reliable serial
> connection, and inherited the assumption that the serial line connection
> would not drop, reorder, or fail character read and writes.  This patch
> adds significantly improved failure mode handling and also does basic
> checks of data validity.

The above seems to be a good idea, however I have some comments on the
changes you made:

1) Flushing buffers *after* a serial command, is (usually) pointless. If
you flush buffers (which is a good idea), you should do it right *before*
sending a command. Otherwise, junk characters after flushing the buffer
will still garble up the communication.

2) Get rid of the 'goto' statements. See also the 'docs/developers.txt'
document about the use of goto.

3) Never, *ever* use a while loop in a driver that is not guaranteed to
exit at some point. The driver will now retry W/L/V/X forever if the
commands fail. It may be a bit harsh to give up on the first failure (like
it did before), but the number of retries must be limited.

4) The new code in hex2d() seems to be over complicated. You're right
about your comments on the use of strncpy(), so we prefer to use
snprintf() now in most cases. Something like

        char buf[32];
        snprintf(buf, sizeof(buf), "%.*s", len, start);
        return strtol(buf, NULL, 16);

will do the job just fine and is much easier to follow.

5) Don't call dstate_datastale() on the first failure, unless you already
know retrying is not going to fix the problem. Otherwise, it is generally
a good idea to mask out the first two or three failures before
complaining. When in doubt, don't call neither dstate_dataok() nor
dstate_datastale() so that the server will report the last state the
driver was in.

Best regards, Arjen
-- 
Eindhoven - The Netherlands
Key fingerprint - 66 4E 03 2C 9D B5 CB 9B  7A FE 7E C1 EE 88 BC 57