HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Async SerialPort Wrapper

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
asyncwrapperserialport

Problem

I've been working with the SerialPort class for a while, trying to figure out the best way to work with it, and especially adding support for Async-Await in C#. I also started before 4.5, so I didn't have access to the BaseStream member (so no BaseStream.ReadAsync()). I wanted to get some opinions on it before I started relying on it too heavily. Performance considerations and stability are high priority, but also figuring out if I'm reinventing the wheel.

The code is designed to wrap around a SerialPort to provide Async-Await operations. It does this by intercepting calls to Write and the SerialPort.DataRecieved events and relaying the data to a pair of Reactive Subjects that client code can subscribe to. Note this code requires the nuget package Rx-Main.

```
using System.IO.Ports;
using System.Reactive;
using System.Reactive.Linq;
using System.Reactive.Subjects;
namespace Core.IO {
public sealed class ComPort : IDisposable, INotifyPropertyChanged{
private Subject _txDataSubject;
private Subject _rxDataSubject;
private SerialPort _port;

public String PortName {
get {
return _port.PortName;
}
set {
_port.PortName = value;
RaisePropertyChanged("PortName");
RaisePropertyChanged("IsOpen");
}
}
public Boolean IsOpen {
get {
return _port.IsOpen;
}
}
public IObservable DataTx {
get {
return _txDataSubject.AsObservable();
}
}
public IObservable DataRx {
get {
return _rxDataSubject.AsObservable();
}
}
public ComPort() {
_port = new SerialPort();
_txDataSubject = new Subject();
_rxDataSubject = new Subject();
_port.DataReceived+=(sender,args)=>{
#if DEBUG
Console.WriteLin

Solution

_port.Encoding.GetBytes(_port.ReadExisting())


If you want to work with byte[], then your data should never appear as string, even temporarily, because some byte sequences are likely going to be invalid in your encoding.

I believe you should be able to use the byte[] overload of Read() for this. (Though switching to byte[] and async methods of the BaseStream is probably a better choice in the long run.)

Your class is schizophrenic about whether it wants to use string or byte[] (the observables use byte[], Write() uses string). You should figure out which choice makes more sense and stick to it.

What is the purpose of DataTx? I think that being able to read what you have written is not commonly needed.

I realize that Tx and Rx are abbreviations used in networking, but I find them confusing. Are you sure users of this library will immediately understand them? If not, you should rename the properties.

Boolean
String


It's common to write those types in their keyword form: bool, string.

~ComPort() {
    Dispose(false);
}


You don't have any unmanaged resources, so Dispose(false) basically doesn't do anything. But this means there is no reason for this class to have a finalizer.

The Dispose Pattern makes sense when you're writing a class that is going to be inherited. But since your class is sealed (nice, most people don't bother with that), I think there is also no reason to have the Dispose(bool) overload, just a single Dispose() method will be enough.

Code Snippets

_port.Encoding.GetBytes(_port.ReadExisting())
Boolean
String
~ComPort() {
    Dispose(false);
}

Context

StackExchange Code Review Q#84319, answer score: 3

Revisions (0)

No revisions yet.