CODEWITHSUNDEEP
c#.netserial-port
Kiran
Kiran

Reputation: 21

.NET SerialPort DataReceived event thread interference with main thread

I am writing a serial communication program using the SerialPort class in C# to interact with a strip machine connected via a RS232 cable. When I send the command to the machine it responds with some bytes depending on the command. Like when I send a "\D" command, I am expecting to download the machine program data of 180 bytes as a continuous string. As per the machine's manual, it suggests as a best practice to send an unrecognized characters like comma (,) character to make sure the machine is initialized before sending the first command in the cycle. My serial communication code is as follows:

public class SerialHelper
{
    SerialPort commPort = null;
    string currentReceived = string.Empty;
    string receivedStr = string.Empty;
    private bool CommInitialized()
    {
        try
        {
            commPort = new SerialPort();
            commPort.PortName = "COM1";
            if (!commPort.IsOpen)
                commPort.Open();
            commPort.BaudRate = 9600;
            commPort.Parity = System.IO.Ports.Parity.None;
            commPort.StopBits = StopBits.One;
            commPort.DataBits = 8;
            commPort.RtsEnable = true;
            commPort.DtrEnable = true;

            commPort.DataReceived += new SerialDataReceivedEventHandler(commPort_DataReceived);
            return true;
        }
        catch (Exception ex)
        {
            return false;
        }
    }

    void commPort_DataReceived(object sender, SerialDataReceivedEventArgs e)
    {
        SerialPort currentPort = (SerialPort)sender;
        currentReceived = currentPort.ReadExisting();
        receivedStr += currentReceived;
    }

    internal int CommIO(string outString, int outLen, ref string inBuffer, int inLen)
    {
        receivedStr = string.Empty;
        inBuffer = string.Empty;
        if (CommInitialized())
        {
            commPort.Write(outString);
        }

        System.Threading.Thread.Sleep(1500);

        int i = 0;
        while ((receivedStr.Length < inLen) && i < 10)
        {
            System.Threading.Thread.Sleep(500);
            i += 1;
        }

        if (!string.IsNullOrEmpty(receivedStr))
        {
            inBuffer = receivedStr;
        }
        commPort.Close();

        return inBuffer.Length;

    }
}

I am calling this code from a windows form as follows:

len = SerialHelperObj.CommIO(",",1,ref inBuffer, 4)
len = SerialHelperObj.CommIO(",",1,ref inBuffer, 4)
If(inBuffer == "!?*O")
{
   len = SerialHelperObj.CommIO("\D",2,ref inBuffer, 180)
}

A valid return value from the serial port looks like this: \D00000010000000000010 550 3250 0000256000 and so on ...

I am getting something like this: \D00000010D,, 000 550 D,, and so on...

I feel that my comm calls are getting interfered with the one when I send commands. But I am trying to make sure the result of the comma command then initiating the actual command. But the received thread is inserting the bytes from the previous communication cycle.

Can anyone please shed some light into this...? I lost quite some hair just trying to get this work. I am not sure where I am doing wrong

Upvotes: 2

Views: 9997

Answers (3)

KeithS
KeithS

Reputation: 71565

Serial ports are not thread-safe resources, and neither are the wrappers that work with them, or the string you're using to hold the results. What's happening is that the DataReceived event handler is firing multiple times, and you're ending up with multiple handlers executing at the same time. Try adding a lock block to your event handler:

...

Object lockingObj = new Object();

...

void commPort_DataReceived(object sender, SerialDataReceivedEventArgs e)
{
    lock(lockingObj)
    {
        SerialPort currentPort = (SerialPort)sender;
        currentReceived = currentPort.ReadExisting();
        receivedStr += currentReceived;
    }
}

The lock statement prevents more than one thread from executing code in the block at a time, so receivedStr will get the full contents of one ReadExisting before another is written to it.

Be aware that this DOES NOT guarantee an order of execution; first-in wins. So, let's say Thread 1 gets here first, then Thread 2 arrives 250ms later, and Thread 3 250ms after that. Thread 1 is likely going to enter first, perform the Read which will take a little while, then exit. In that time, Threads 2 and 3 enter the function and are blocked on the lock statement, waiting for Thread 1 to release its lock. Once that happens, it's down to which thread gets scheduled first by the kernel, and that could be Thread 3 depending on a number of OS and hardware factors.

Upvotes: 2

John Saunders
John Saunders

Reputation: 161773

I see a number of problems with your code.

  1. Never ignore exceptions. Returning false is ignoring an exception. You have no idea which exception happened when you do that.
  2. In particular, your commPort object may never have been instantiated. It could be in any state at all, but you're ignoring exceptions and proceeding with accessing the potentially-invalid object.
  3. That DataReceived event can happen at any time while the port is open. It may or may not be raised on the same thread. Your code might be closing that port out from under the event handler, which would throw an exception that your code is not catching.
  4. It probably doesn't matter here, but did you know that string concatenation is a relatively expensive operation in .NET? Strings are immutable. receivedStr += currentReceived doesn't append to receivedStr - it creates a new string object to hold both pieces.

Upvotes: 0

zsalzbank
zsalzbank

Reputation: 9857

Why are you re-initializing your port every time? I would think one is enough.

Upvotes: 0

Related Questions