Recommended asynchronous usage pattern of SerialPort

[LeonidVasilyev]

The question is about common command scenario when you send a command to a device and want to check a response to see the result or simply to check that the command is acknowledged. What is the recommended way to organize that in a non blocking fashion in general and in an UI application?

Currently I see three options. First is SerialPort.BaseStream.*Async methods. SerialPort.BaseStream.ReadAsync() uses Stream.ReadAsync() implementation and ignores SerialPort.BaseStream.ReadTimeout and most of the time ignores CancellationToken instance. If you use Task.Delay() alongside Task.WhenAny() with SerialPort.BaseStream.ReadAsync() there is read task and byte buffer left to hang forever. So it doesn't seem like a good option.

Another options is to use SerialPort.DataRecieved event. But in this case you basically probaby need to organize state machine so the event handler behaves differently depending on which command is currently in process.

The third option is to use synchronous methods and offload the work to a Thread pool thread which seems the best of three:

static async Task OnClick(object sender, EventArgs args, CancellationToken cancellationToken)
{
    var jobTask = Task.Run(() => {
        // Organize critical sections around logical serial port operations somehow.
        lock (_syncRoot) 
        {
            return SomeCommand();
        }
    });
    if (jobTask != await Task.WhenAny(jobTask, Task.Delay(Timeout.Infinite, cancellationToken)))
    {
        // Timeout;
        return;
    }
    var response = await jobTask;
    // Process response.
}

static byte[] SomeCommand()
{
    // Assume serial port timeouts are set.
    _serialPort.Write(...);

    int offset = 0;
    int count = ...; // Expected response length.
    byte[] buffer = new byte[count];
    while (count > 0)
    {
        var readCount = _serialPort.Read(buffer, offset, count);       
        offset += readCount;
        count -= readCount;
    }
    return buffer;
}

[krwq]

For long running applications my recommendation is to use following pattern:

Stream s = sp.BaseStream;
// now for text based protocols use StreamWriter/StreamReader (pass: `leaveOpen: true`)
// use it the same way you would use stream asynchronously or synchronously whichever you prefer

DataReceived is the easiest option but I would personally not use it for anything running for several days - it is really easy to misuse. Currently it is not clearly defined when or if the event should occur:
ie.

• should it run for every byte received or some when at least some number of bytes received?
• should it wait until internal buffer is full if still receiving bytes?
• should it run while you are still running previous event? what happens if event is blocking forever? (i.e. if you put Console.ReadLine() which never gets triggered should it allow triggering new events which would potentially also block? or should it not trigger them? what happens when you press enter to finish the event but you received 3 bytes meantime? should it trigger events at the same time, wait until you're finished or perhaps generate 3 events the moment the last event finishes? (that would potentially block reading the data and causing internal queue to fill up and eventually causing you can't read any new data anymore and lose it permanently)).

for reading/writing I personally prefer running a background thread and use synchronous APIs (especially reading - writing should be fine and intuitive asynchronously as well) - it is much easier to understand what will happen in what order (Read/ReadAsync may read less bytes than requested so running asynchronously may cause to mix multiple reads at the same time and you will get random bytes from random threads - they will be processed in ordered they came in but since the are not guaranteed to fill the buffer it makes them not too reliable asynchronously)

please let me know if this answers your question

[LeonidVasilyev]

Thank you for the response.

for reading/writing I personally prefer running a background thread and use synchronous APIs (especially reading - writing should be fine and intuitive asynchronously as well)

Is it correct that if you want to set timeout and exclude possibility that write operation will hang forever (possibly because not-ready-to-communicate pin is set by the device?) holding byte arrays and tasks you still need to use synchronous API?

[krwq]

@LeonidVasilyev if you want to not hang then you'll need to pass CancellationToken which expires after desired time:

using (var cts = new CancellationTokenSource(1000))
{
   // pass cts.Token to async APIs
}

[LeonidVasilyev]

It seems like SerialStream ignores it as NetworkStream do. See Document that CancellationToken in Stream.ReadAsync() is advisory related issue. But the difference is that I am not aware of a way to cancel a SerialPort operation after timeout using some other SerialPort or SerialStream method.

[krwq]

@LeonidVasilyev could you share dotnet --info, csproj and sample code?

[IGx89]

We're banging our heads against this too, in our case with a SerialStream. We've tried applying https://devblogs.microsoft.com/pfxteam/how-do-i-cancel-non-cancelable-async-operations/ to it but that's deficient too.

[krwq]

@IGx89 which OS are you using? Could you share dotnet --info output on the platform you experience issues? How are you using the SerialStream?

[IGx89]

In our case it's both Windows 7 and Windows 10, running .NET Framework 4.7.2. But it looks like Microsoft hasn't changed the API/code/recommendations for .NET Core so the same deficiencies would be there too.

It just would be great to know that when we update our app to .NET Core 3.0 we had a much simpler approach to canceling a read. Or at the very least, new developers wouldn't have to spend hours reading StackOverflow and going through multiple QA cycles just to come up with the right code to handle all the different edge cases.

[krwq]

@IGx89 cancellation (cancellation token) with SerialStream should work correctly on Linux but not sure about Windows at the moment (I think it should be fixable if doesn't work) - if full framework doesn't work as expected at this point we will likely not fix it.

[IGx89]

Makes sense, thanks for looking into it!

[mikkleini]

Moving to Linux is the answer? This is not encouraging at all.
The async API is there and it's not functioning (neither timeout or cancellation) so you got to do something about it.

[krwq]

@mikkleini as far as I understand the timeouts should work (SerialPort.Read/WriteTimeout) - I'd expect at minimum scenarios covered by MSDN samples to work. The cancellation is something we'd like to improve but we currently have many issues in different areas and SerialPort is not high on our priority list at the moment. Linux implementation was written from scratch so we have added the support and accounted for it when adding support for timeouts.

[mikkleini]

Yes, SerialPort ReadTimeout works when used with blocking SerialPort.Read() function. But that and BaseStream.ReadTimeout do not have effect on BaseStream.ReadAsync(). I tried using CancellationTokenSource with CancelAfter and that didn't work either (on Windows).

Interesting is that I was successfully using blocking read with threads and signals and then I thought I'll modernize it with TPL and the first thing after refactoring I saw was stuck ReadAsync() function. It's very rare to see a .NET FW/Core issue so obviously it sticks out. Since I have the working old code, it's not that critical, but I would prefer to see that the issue is acknowledged and fix is planned properly. Right now from this "question" and other similar topic #24093 it feels like it's not treated as issue at all.

[krwq]

@mikkleini this is an issue but it's not treated with any priority because there are currently ways to achieve sort of cancellation (as you mentioned above - they are not perfect but possible). Note that the Windows version of this code has existed in full framework for a very long time and we never got many complains about it which puts it much lower than other issues in other areas we have. This is something I'd personally like to get fixed but cannot pick up at the moment. If you have some time to make the improvements we would be happy to take the PRs but note that we will still need to do bunch of testing on different pieces of hardware to ensure there is no regressions since those are not currently tested in the CI.

[mikkleini]

I took a look into Unix SerialStream and Windows SerialStream + Stream implementations to figure out the issue. I was surprised to find IOLoop and Thread.Sleep() in SerialStream.Unix and tasks in Windows C# streams implementation. I thought the asynchronous stuff goes deeper, maybe even to OS level, but unless I miss something important, under the hood the job is still done in classical waiting threads method. That means there's probably no significant performance increase in using asynchronous interface versus using own thread with blocking functions. So even this piece of code could provide the "async" interface but with working read timeout:

public static class SerialPortExtension
{
    public static Task<int> ReadAsync(this SerialPort port, byte[] buffer, int offset, int count)
    {
        return Task.Run(() =>
        {
            return port.Read(buffer, offset, count);
        });
    }
}

I don't have time to do deep performance tests now, but @krwq could you please comment - does it make sense?

[krwq]

@mikkleini it can go to OS level, it really depends on the API, on Unix serial ports could partially be done by OS but there would be not much benefit since we would still be required to poll and wait - there are some improvements we could do: i.e. one loop per all ports, waiting on multiple handles at the same time etc but for SerialPort specifically it was not worth much effort since usually there is no more than few ports per machine anyway (as opposed to sockets or files)

[carlossanlop]

It seems the questions have been responded. @mikkleini @LeonidVasilyev feel free to reopen if you have any additional questions.