Reputation: 13
This is my window and serial port declaration:
public SerialPort sp = new SerialPort();
public MainWindow()
{
InitializeComponent();
ConnectToCOM();
}
private void ConnectToCOM()
{
if (sp.IsOpen)
{
//do some stuff like show notification bar
}
try
{
sp = new SerialPort(SerialPortsList.SelectedItem as string, 9600, Parity.None, 8, StopBits.One);
sp.DataReceived += new SerialDataReceivedEventHandler(DataReceivedHandler);
sp.Open();
}
catch (Exception ex) { }
}
and this is handler for data received:
private void DataReceivedHandler(object sender, SerialDataReceivedEventArgs e)
{
var input = sp.ReadExisting();
}
Program works great with Windows 10 or 8.1!
The problems starts when I try to run an application on Windows 7 or XP.
Problem 1: Some of incoming data differ from that what I am sending
For example:
I send: 24FGTG32
I received: FGTG32
Problem 2: Second and most serious problem is:
I am trying to send data back when I receive something from SerialPort.
For example:
COM sends: data1 -> COM received: nothing
COM sends: data2 -> COM received: data1
COM sends: data3 -> COM received: data2
private void DataReceivedHandler(object sender, SerialDataReceivedEventArgs e)
{
var input = sp.ReadExisting();
if (/*something*/)
{
sp.Write($"{worker.WorkerName} {result.EventType}\r\n");
}
return;
}
Project target framework: .NET Framework 4
Upvotes: 1
Views: 499
Reputation: 942099
What you describe is entirely normal and you make all the traditional mistakes that programmers make when they write SerialPort code. Dealing with the fundamentally asynchronous nature of serial port I/O is just tricky, not otherwise any different from the way a TCP socket behaves.
Most important detail to keep in mind here is that you have two threads running in your program. First one is your WPF main thread, it is probably the one that makes the first Write() call. The other one is hard to see, it is the one that makes the DataReceived event handler run. A threadpool thread. They run completely out of sync with each other. Exactly when the event handler run is highly unpredictable. It just takes a while for the device to receive the command and return a response. If you have no code that synchronizes the two then it is guaranteed to go wrong sooner or later, you'll have a threading race bug.
Using ReadExisting() makes it a lot worse, you get back whatever happens to be received so far. On a real serial port then it is typically only one or two bytes, could be "24" and "F" and "GTG" and "32" for example. In your case it is probably a USB emulator which by nature tends to group more bytes together. So "24" and "FGTG32". We can't tell what you do with received string, but a classic bug would be to store it in a string that you read in your WPF thread. That is race bug waiting to happen. If the WPF thread is a bit slow, old machine for example, then it can easily miss "24" completely and only see "FGTG32".
You'll have another race bug when the DataReceived event handler is slow to get running. Your WPF thread might now not wait long enough to see the response and conclude there is no response. That's quite fatal, you're now permanently out of sync with the device. It is important to realize that this can always happen, it doesn't depend on the OS or how fast the WPF code runs. Even the fastest Win10 machine is going to be slow occasionally when a lot of programs are running, demanding attention from the processor. It just happens less often, makes it harder to diagnose the bug. And makes you think it is an OS problem.
Nasty too is that it seems to work just fine when you debug the DataReceived event handler. That slows down your program enough so the device driver has time to receive more bytes. And ReadExisting() returns the full response, you'll never observe the failure mode.
So first thing to do is fix the DataReceived handler. It must ensure that it received the full response from the device before doing anything with the data. Serial devices always use a protocol to make it easy for the receiver to determine this. A very simple one is using a unique byte to mark the end of the response. High odds that this device works that way since you Write() strings with \n at the end. Which gives good odds that ReadLine() works, at best you need to change the NewLine property.
Then you'll have to fix the threading race bugs. One very simple way is to not use DataReceived at all, but let the WPF thread call ReadLine(). Making it synchronous with only one thread. It is risky however, pretty important to use the ReadTimeout property so a device failure cannot hang your user interface. And it won't work very well when the device is slow to return data, that makes your UI unresponsive because it is hanging too much on the ReadLine() call. And you run the risk of losing data if your UI thread goes off in the woods, querying a dbase for example. Albeit that it is not a risk when the device only ever sends something when you write a command, then the receive buffer takes up the slack.
If synchronous code causes a problem, like it often does, then you do have to make it async. Leverage DataReceived by having it BeginInvoke() code that runs on the UI thread, that code needs to sequence the communications correctly. Or call BeginRead() on the SerialPort.BaseStream for example. Or leverage the async/await keywords by using ReadAsync(). If that causes data loss because of a pokey UI thread problem then you'll have to dedicate a thread to talking to the device.
Upvotes: 2