Arjen de Korte
2008-Nov-06 21:21 UTC
[Nut-upsdev] [nut-commits] svn commit r1549 - in trunk: . drivers
Citeren Arjen de Korte <adkorte-guest at alioth.debian.org>:> Author: adkorte-guest > Date: Wed Nov 5 07:56:47 2008 > New Revision: 1549 > > Log: > Fix problem with reading replies on slow systems. > > The ser_get_line() function is unreliable when reading multi-line > replies from the UPS. See > docs/new-drivers.txt for an explanation why. Basically this will > only work if the driver is able > to keep up with the flow of data from the UPS and the select_read() > function in ser_get_line() > reads one character at a time. In that case, switching to > ser_get_char() is more reliable, > since that is not prone to losing data after reading ENDCHAR.What was I thinking? I completely missed the fact that the UPScode II driver sets the communication parameters to ICANON (Canonical Mode Input Processing), so the above is not an issue at all. There is a (slightly) better way to do this however, by using select_read() instead and let the input processing ignore the <CR> character. For any length of input, this guarantees that there is exactly one select() and one read() operation, which helps interpretation of 'strace' output.> Another problem is that the driver seems to expect that on > errors/timeout, the length of the > returned string is 0 (strlen(buf) == 0). This is not guaranteed to > be the case (ser_get_char() > will return truncated replies on timeout for instance).Not at all, this buffer will be empty on timeouts or errors if you use ser_get_line(), provided the ENDCHAR is <NL> and the above communication parameters are used.> Last but not least if the length of the returned string is 0, this > doesn't mean that there is no > more data left in the input buffer. So flushing the input buffers > was not guaranteed to work.This still holds true, a zero length reply doesn't mean there is nothing more in the input buffer (this might be an empty line with only a <NL> in it), so we'd better use ser_flush_in() or ser_flush_io() instead. Best regards, Arjen -- Please keep list traffic on the list