How to use SerialPort class more reliably

Question

I've been using the SerialPort class for a little while in an application that communicates with some external hardware that I've designed. During the debugging process of said hardware, I found a few things to be unreliable, and I recently stumbled across this which seems to be onto something.

I'm trying to implement this into my application, however I have two questions about receiving data...

So, in the article I linked, the author mentions that the DataReceived event is problematic, and shows a typical code sample of how it could be used...

port.DataReceived += port_DataReceived;

// (later, in DataReceived event)
try {
    byte [] buffer = new byte[port.BytesToRead];
    port.Read(buffer, 0, buffer.Length);
    raiseAppSerialDataEvent(buffer);
}
catch (IOException exc) {
    handleAppSerialError(exc);
}

And then what the author considers to be the correct method is shown...

byte[] buffer = new byte[blockLimit];
Action kickoffRead = null;

kickoffRead = delegate {
port.BaseStream.BeginRead(buffer, 0, buffer.Length, 
        delegate (IAsyncResult ar) {
            try {
                int actualLength = port.BaseStream.EndRead(ar);
                byte[] received = new byte[actualLength];
                Buffer.BlockCopy(buffer, 0, received, 0, actualLength);
                raiseAppSerialDataEvent(received);
            }
            catch (IOException exc) {
                handleAppSerialError(exc);
            }
            kickoffRead();
        }, null);
    };
    kickoffRead();

My questions revolve around the use of BaseStream.BeginRead; where abouts in my class that would be reading the data should this be put? My first thought was in the DataReceived event, as in the first example showing how NOT to use the SerialPort class the author mentions that the code is in the DataReceived event in the comment, but for the sample showing a better way the author makes no mention of where the code should be so I thought he was still referring to the DataReceived event, but then the author mentioned that the DataReceived event itself has problems, so...? Any guidance here would be great, and appologies if its obvious!

If I've not mentioned something that would benefit anyone trying to answer this then by all means let me know. Thanks in advance for any guidance and/ or feedback!

It is pretty common for a programmer to half-ass the code, yell "unreliable!", do it another way that makes it work and declare it "superior". Standard mistakes in DataReceived is to not pay attention to e.EventType, use BytesToRead even though you know there are bytes to read and to use Read() without using its return value. Not implementing an event handler for the ErrorReceived event is chronic, so you can't tell that data got corrupted. Ask only one question. — Hans Passant, Nov 16 '16 at 16:30
Are you suggesting the article doesn't hold any real merit or that I've 'half-assed' the code? If it's the latter then yes, I'm not referring to a finished project, but even Microsoft themselves mention on MSDN that the DataReceived event won't be raised for every byte received, and that's exactly what I'm experiencing. The article I linked states that the SerialPort class doesn't use the underlying Win32 serial port API properly, so I'm merely trying something different for an experiment. — howamith, Nov 16 '16 at 23:36
@HansPassant: You're right that ignoring errors would not be a good idea, but the `ErrorReceived` event is not the way to do it, because it arrives out-of-order with the data received successfully surrounding the erroneous condition. (I've read the source code, the IOPSP code indeed has insurmountable design flaws, this is not something based only on experimental data) Did you overlook that the Task returned by the `ReadAsync` I recommend does also report errors by completing in a faulted state? — Ben Voigt, Dec 02 '16 at 16:11
@HansPassant: Well he quoted the `BeginRead`/`EndRead` code, which also has error-checking.... again, present in the quoted code. — Ben Voigt, Dec 02 '16 at 16:24

score 4 · Accepted Answer · answered Dec 01 '16 at 23:41

4

It's a recursive function, so you call this code only ONCE after opening the port, and it will continue to repeat itself without blocking execution (or requiring a DataReceived event handler).

answered Dec 01 '16 at 23:41

Maestro

9,046
15
83
116

Thanks for your answer, I've done a little reading on recursive code and I think I'm with you. I'll do some tests on Monday and then I'll either mark this as the answer or comment with a question or two ;) Thanks again. – howamith Dec 02 '16 at 16:00
2

I don't know who downvoted this, but Muls is correct (+1). I should know, I'm the author of that blog post. – Ben Voigt Dec 02 '16 at 16:07
2

Also please note that it is not *true* recursion, the `BeginRead` block is called from the completion handler, not from itself, so the call stack won't grow indefinitely. – Ben Voigt Dec 02 '16 at 16:30
Thanks again Muis - exactly what I was looking for. And thank you for your input too Ben. – howamith Dec 05 '16 at 13:47
@BenVoigt I'm using your code snippet and i have one issue that i somehow cannot fix. It crashes when usb is unplugged no matter the try/catch that i use. It also doesn't happen in debug only when detached from VS. – Don P Apr 11 '20 at 02:32
2

@AysonBaxter: After a terminal exception (in the sense that the serial port is no longer usable at all, retrying the operation is hopeless) you should arrange for `kickoffRead()` to not get called any longer. This can be by keeping track of if the port is closed and checking this flag then returning from the callback without calling `kickoffRead()`... or simply when you close the port set the `kickoffRead` delegate to point to a no-op function like `kickoffRead = delegate { };` – Ben Voigt Apr 13 '20 at 05:02

How to use SerialPort class more reliably

1 Answers1

Linked