Nut upsdev - Sep 2011 - [windows port] blazer_ser issues, select_read() from serial semantics

Hi,

I tried using 2.6.1 version for windows with a Megatec-compatible UPS
and was surprised to find out it doesn't work at all.

The reason for that is blazer_ser using ser_get_buf specifying a buffer
large enough to accept any reasonably possible response, setting
buflen accordingly. That calls select_read() and it's supposed to wait
until something is available and then to read up all the data from
port, returning the number of bytes read. But the windows version
apparently differs, the way it's currently implemented it either reads
up to buflen bytes or returns an error indicating a timeout.

I can't say for sure how i'd imagine the correct solution as it seems
to me that even *nix version is currently implemented in a fragile
way, making hidden assumptions that might broke under e.g. a heavy
system load.

TIA

Something like this might be appropriate for the windows version (tested
here with our UPSes):

diff --git a/common/wincompat.c b/common/wincompat.c
index 86c786f..f271a3a 100644
--- a/common/wincompat.c
+++ b/common/wincompat.c
@@ -411,12 +411,15 @@ int w32_serial_read (serial_handler_t * sh, void *ptr,
size_t ulen, DWORD tim
                                               
upsdebugx(4,"w32_serial_read : characters are avail
                                                break;
                                        case WAIT_TIMEOUT:
-                                               CancelIo(sh->handle);
-                                               sh->overlapped_armed = 0;
-                                               ResetEvent
(sh->io_status.hEvent);
-                                              
upsdebugx(4,"w32_serial_read : timeout %d ms ellaps
-                                               errno = EIO;
-                                               return 0;
+                                               if (!tot) {
+                                                       CancelIo(sh->handle);
+                                                       sh->overlapped_armed
= 0;
+                                                       ResetEvent
(sh->io_status.hEvent);
+                                                      
upsdebugx(4,"w32_serial_read : timeout %d m
+                                                       errno = EIO;
+                                                       return 0;
+                                               }
+                                               break;
                                        default:
                                                goto err;
                                }

-- 
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercerpav at gmail.com

Hello,

(adding mailing list back to Cc as i consider this to be of general
interest, hope you do not mind)

On Wed, Sep 28, 2011 at 09:18:38AM +0200, Fr?d?ric Boh?
wrote:
> I would like to integrate your patch in the next release of Windows
> binary. But I want to check if it does not introduce regression on
> serial devices I have.
I hope i made it clear enough that it has different (though more
correct imho) semantics from the current *nix version and that this
issue should probably be discussed in a more detailed manner.

Here follows my humble opinion with regard to that:

It should be put straight: current idea of asking for "some" data and
being happy with whatever first "chunk" received feels like a misuse
of the POSIX serial handling semantics. Given that we do not know in
advance how many bytes we want to receive and also provided you do not
want to introduce protocol-specific "end-of-data/line" handling, a
sane solution seems to be reading as much data as available until we
stop receiving (as there's nothing left) and hit timeout. That's what
i (hopefully) implemented in this patch. This can't work properly with
any device that constantly sends something (as e.g. GPS) and this
introduces an additional latency (waiting for a timeout after the last
byte received) unless the supplied buffer was exactly the response
size.

So it'd would be nice to hear the opinion of the *nix serial hackers
here: are you sure the current approach is correct? Do you think it
should be changed in similar way to my windows patch? Or do you think
a protocol-specific handling for end-of-data condition should be
introduced? Depending on this decision, windows code should be
correspondingly altered.
> All this to say to you that I do not forget your patch but I will treat
> it in several weeks.
Thanks for letting me know, i was about to think my mail slipped
through the cracks ;)
> Thanks for your interest in this project.
Thanks for your work on this rather cool software :)

-- 
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercerpav at gmail.com