1

I am trying to use the SerialPort class in .net.

I've opted to keep my service async, so I am using the async-methods on SerialPort.BaseStream.

In my async method, I write a byte[] to the serial port, then start reading until I haven't received any more data in n milliseconds, and return that result.

The problem is, however, that I seem to miss the first byte in all replies other than the very first reply after opening the serial port.

If I close the port after every response (Read), and open it again before doing a new request (Write), the first byte is not missing. This, however, often results in a "Access to the port 'COM4' is denied." exception, if I try to open the port too soon after closing. It also seems very unnecessary to open/close for every write/read.

This is basically what my method looks like:

private async Task<byte[]> SendRequestAsync(byte[] request)
{    
    // Write the request
    await _serialPort.BaseStream.WriteAsync(request, 0, request.Length);

    var buffer = new byte[BUFFER_SIZE];
    bool receiveComplete = false;
    var bytesRead = 0;

    // Read from the serial port
    do
    {
        var responseTask = _serialPort.BaseStream.ReadAsync(buffer, bytesRead, BUFFER_SIZE - bytesRead);
        if (await Task.WhenAny(responseTask, Task.Delay(300)) == responseTask)
        {
            bytesRead += responseTask.Result;
        }
        else
            receiveComplete = true;


    } while (!receiveComplete);

    var response = new byte[bytesRead];
    Array.Copy(buffer, 0, response, 0, bytesRead);

    return response;
}

Is there anything obviously wrong in the way I am doing this? Is there a smarter way to achieve the same asynchronously?

Walkingsteak
  • 329
  • 4
  • 16
  • 1
    doing a `.Result` after a `WhenAny` or `WhenAll` always makes me cringe, I know it should never have a problem, but I still don't like it. I always like to rather do `bytesRead += await responseTask`, there is no overhead to the `await` becaues the task is complete however you get the advantage of a unwrapped exception thrown instead of getting a `AggregateException` from `.Result` in the event you do get a error. – Scott Chamberlain Mar 08 '16 at 16:23
  • Are you sure that there are no bytes on the stream after your 300 ms timeout? How do you know you're missing the first byte? Is that first byte special in some way? The reason I ask, is because maybe there were bytes left from the previous response, and so every read after the first read actually does contain the first byte, but after the leftover bytes from the previous request. You might need to flush the stream before each request. – Ayo I Mar 09 '16 at 17:45
  • I know the first byte is missing, since every response should start with the ASCII character '$' (NMEA messages). I have now switched to using synchronous API, but running it in a background task, and it is working. Though I don't like this solution .. – Walkingsteak Mar 11 '16 at 09:03

1 Answers1

3

Just because you're not observing the last ReadAsync() doesn't mean it gets canceled, it's still running, which apparently manifests by it reading the first byte of the following message.

What you should do is to cancel the last ReadAsync() by using a CancellationToken. Note that there is a possible race between the timeout and the read, but I'm assuming that if the timeout elapsed, it's not possible for the read to complete without another write.

The code would look like this:

var cts = new CancellationTokenSource();

do
{
    var responseTask = _serialPort.BaseStream.ReadAsync(
        buffer, bytesRead, BUFFER_SIZE - bytesRead, cts.Token);

    if (await Task.WhenAny(responseTask, Task.Delay(300)) == responseTask)
    {
        bytesRead += responseTask.Result;
    }
    else
    {
        cts.Cancel();
        receiveComplete = true;
    }
} while (!receiveComplete);

Note that both the cause and the solution are my guesses, it's certainly possible that I'm wrong about one or both of them.

svick
  • 236,525
  • 50
  • 385
  • 514