search

jcurl / RJCP.DLL.SerialPortStream

SerialPortStream is an independent implementation of System.IO.Ports.SerialPort and SerialStream for better reliability and maintainability. Default branch is 2.x and now has support for Mono with help of a C library.
Microsoft Public License
614 stars 196 forks source link

ReadAsync and WriteAsync implementation for .NET standard 1.5 #115

Closed meinsiedler closed 3 years ago

meinsiedler commented 3 years ago

Hi!

This PR resolves the issue described in #114. As discussed there, .NET standard doesn't support the Begin|EndRead and Begin|EndWrite methods and the current implementation of SerialPortStream didn't implement the ReadAsync and WriteAsync methods from the Stream base class.

With this PR, the two methods are now implemented. The two new methods simply delegate to the existing BeginRead and BeginWrite methods using the Task.FromAsync method. Since AsyncResult isn't available any more too, we can use a Task instead as described here (Task implements IAsyncResult).

With the approach of re-using the existing Begin|EndRead and Begin|EndWrite methods we can leverage the well tested and established code without introducing much more code to maintain.

Unfortunately, the Asynchronous Programming Model (APM) (using Begin|EndRead and Begin|EndWrite methods) didn't consider cancellation, so we cannot support the CancallationToken with the new ReadAsync and WriteAsync methods easily. The passsed cancellationToken will simply be ignored. We hope that's OK for now. Otherwise we would need to rewrite the entire read/write process using "real" asynchronicity and with "real" cancellation behavior. Since this would mean a lot more effort and more further code to maintain in future for little use, we think it's better to ignore the cancellation behavior in that case.

Regarding testing: We haven't added any more unit tests to your solution, since we saw that no .NET standard 1.5 tests were present. For testing our changes locally, we added my unit test from my test project directly, but didn't commit it to your repo. The tests now succeed with .NET Core 3.1 too.

We'd be very happy if our PR for fixing the issues with .NET Core lands in a new NuGet version of your project.

Best regards.

jcurl commented 3 years ago

I really appreciate your effort in making the changes. Had a quick look over it and only some minor details.

meinsiedler commented 3 years ago

I really appreciate your effort in making the changes. Had a quick look over it and only some minor details.

I also really appreciate your quick responses and your effort you make by providing useful information and your feedback. 😊

I'll update the PR with the requested changes maybe tomorrow or next week at latest.

meinsiedler commented 3 years ago

Hi,

I enabled ReadAsync and WriteAsync for .NET 4.5 too. Also, I added a unit test for .NET 4.5 only and fixed some code formatting issues (braces, #region indentation).

jcurl commented 3 years ago

Integrated with release v2.3.0 (bc94184904364411ef0f5406c918b4873e0b70a2) and published on NuGet.