Program Crashing on Serial Communication for Industrial Controller - Help!

Hi,
I have created an industrial controller that communicates with a PC to turn on and off large loads and read inputs/ analog to digital/ set pwm, etc. The controllers are crashing - just freezing up, and I am having a hard time catching the issue, because it might take hours of use before it happens.

The controller is based on the Panda II.

The program’s main loop just flashes the onboard LED to show that it is alive and working. It checks an ‘isData’ flag to see if serial data is present:

            while (true)
            {
                // Check to see if there is data available, then process
                if (isData)
                {
                    ProcessData();
                }

                // If 500 ms have passed, blink LED
                if (i > 50)
                {
                    LED.Write(!LED.Read());
                    i = 0;
                }

                Thread.Sleep(10);

                i++;
            }

The serial data interrupt:

        /// <summary>
        /// Handles Data Received by COM1
        /// </summary>
        /// <param name="sender"></param>
        /// <param name="e"></param>
        static void COM1_DataReceived(object sender, SerialDataReceivedEventArgs e)
        {
            try
            {
                // Set data flag
                isData = true;
            }
            catch (Exception)
            {
                return;
            }
        }

If there is data, it is processed as a string to determine what action the PC wants:

        /// <summary>
        /// Processes incomming serial data
        /// </summary>
        static void ProcessData()
        {
            try
            {
                // Wait for full string
                Thread.Sleep(50);

                // set write buffer
                bufferString = String.Empty;
                byte[] bufferWrite = null;

                // Set read buffer
                byte[] buffer = new byte[COM1.BytesToRead];

                COM1.Read(buffer, 0, COM1.BytesToRead);

                char[] read = System.Text.Encoding.UTF8.GetChars(buffer);

                string readString = new string(read);

                string valueString = new string(new char[3] { read[3], read[4], read[5] });
                // determine command value
                int value = Convert.ToInt16(valueString);

                // determine what action to take...
                // Check for proper start character
                if (read[0] == '$')
                {
                    // Check for analog
                    if (read[1] == 'A' || read[1] == 'a')
                    {
                        // Check for input
                        if (read[2] == 'I' || read[2] == 'i')
                        {
                            // Read and print input
                            bufferString = ReadADC(Convert.ToInt16(read[3].ToString()), Convert.ToInt16(read[5].ToString()));
                        }

                    }

                    // Check for digital
                    else if (read[1] == 'D' || read[1] == 'd')
                    {
                        // Check for input
                        if (read[2] == 'I' || read[2] == 'i')
                        {
                            // Read inputs
                            bufferString = ReadDigitalInputs();
                        }
                        // Check for output
                        else if (read[2] == 'O' || read[2] == 'o')
                        {
                            // Write outputs
                            bufferString = WriteDigitalOutputs(value);
                        }
                        // Check for read outputs
                        else if (read[2] == 'R' || read[2] == 'r')
                        {
                            // Read Outputs
                            bufferString = ReadDigitalOutputs();
                        }
                    }

                    // Check for I/O expansion
                    else if (read[1] == 'X' || read[1] == 'x')
                    {
                        // Check for output
                        if (read[2] == 'O' || read[2] == 'o')
                        {
                            ExpansionSetOutput(read[4], read[5], read[6]);
                        }

                            // Check for set
                        if (read[2] == 'S' || read[2] =='s')
                        {
                            string outputValue = new string(new char[5] { read[4], read[5], read[6], read[7], read[8] });
                            ExpansionSetOutputs(read[3], Convert.ToInt16(outputValue));
                        }

                        // Check for input
                        else if (read[2] == 'I' || read[2] == 'i')
                        {
                            ExpansionReadInputs(read[3]);
                        }

                        // Check for read outputs
                        else if (read[2] == 'R' || read[2] == 'r')
                        {
                            ExpansionReadOutputs(read[3]);
                        }
                    }

                    // Check for PWM control
                    else if (read[1] == 'P' || read[1] == 'p')
                    {
                        // Check for PWM Set
                        if (read[2] == 'S' || read[2] == 's')
                        {
                            // Determine frequency
                            string dutyString = new string(new char[2] { read[4], read[5] });
                            // determine command value
                            int duty = Convert.ToInt16(dutyString);

                            // Set PWM
                            if (read[3] == '1')
                            {
                                if (duty == 0)
                                {
                                    PWM1.Set(false);
                                }
                                else
                                {
                                    PWM1.Set(1000, (byte)duty);
                                }
                            }
                            if (read[3] == '2')
                            {
                                if (duty == 0)
                                {
                                    PWM2.Set(false);
                                }
                                else
                                {
                                    PWM2.Set(1000, (byte)duty);
                                }
                            }

                            bufferString = "PWM Set";
                        }

                    }

                    // Create buffer string
                    bufferWrite = System.Text.Encoding.UTF8.GetBytes(bufferString + "\n");
                    // Return values
                    COM1.Write(bufferWrite, 0, bufferWrite.Length);
                }

            }
            catch (Exception)
            {
                return;
            }
            finally
            {
                // Reset Data Flag
                isData = false;

                // Reset buffer string
                bufferString = String.Empty;
            }
        }

I have a number of machines on my production floor with this problem, and it is getting to be a major issue. Any help that can be offered is very much appreciated.
Thank you,
Tom

You are assuming that the problem is in the code you posted. Why?

You have not given any details on data arrival rate.

You are including a sleep in your serial read code to allow a full record to be read. This is a bad practice.

In the code below, there is a potential race condition

             Thread.Sleep(50);
 
                // set write buffer
                bufferString = String.Empty;
                byte[] bufferWrite = null;
 
                // Set read buffer
                byte[] buffer = new byte[COM1.BytesToRead];
 
                COM1.Read(buffer, 0, COM1.BytesToRead);

You size the buffer based upon COM1.BytesToRead. You then read using COM1.BytesToRead as the read length. What would happen if another byte arrived in the serial buffer between the allocation of the buffer and read?

You set a flag in the data received event handler, and then call a routine from the main thread. Why bother with the event handler? Do your data reading in a separate thread.

You do not valid the size of the message you are receiving. If the message is short, you will get an indexing exception.

I think you have to be a bit more diligent in handling exceptions. How attaching a laptop to one of the floor machines, run under the debugger, and see if an unhandled exception is occurring.

Wow. Thanks for the input. I am not a programmer by training, as you can well see. Do you have or know of better examples of handling serial data? Maybe someone has produced a class that is more comprehensive?

I am assuming this is the issue because it always freezes when trying to communicate with the unit.

The data rate is 115200

Why is it bad practice to include the sleep? What if I know that given the data rate it will never take longer than that?

I am not sure what the race condition means, or how to correct it…

The message size is set by the PC that is sending it. I know the size of all of the messages, they are the same.

I have the debugger running right now… More on that if I get some input from it.

Thank you very much for your input,
Tom

Yes, you read correctly. I am used to working with the .NET version that has some very useful methods such as ReadTo, ReadLine, and ReadExisting.
Tom

Unfortunately I am using a Panda II. I will have to come up with something else.
Thanks

@ tommyvdp - I think I might have found your problem.

When a data received event occurs, you set a flag. This then causes the processing routing to be called. You then process the message, and in the finally section you set the data flag off.

But, what would happen if while you were processing a message another message arrives? The data received event would fire, and the data flag would be set. When you finish processing, you would reset the data flag.

There is now some data in the serial queue which would not be read until more characters arrive. Depending upon timing, you could read two messages at once or a message and a half. Since you do not do anything to assure message alignment, you could get funny situations. This is one of the reasons that timing is not a good way of assuring atomic messages.

As far as race condition. Instead of using the characters ready count for the serial port read, use the size of the buffer just allocated. This will insure there is not a buffer overflow. For further info on races Race condition - Wikipedia.

If you are using the data received event, I recommend you do the read in the event handler, and place the data into a queue. Then, you have a thread that reads the queue and processes the data.

I would second Mike’s last comment. Have the data received event put all the data in a queue and then process this queue from your main thread. This way, all the serial handler does is read and store the bytes.

Thanks for the information. @ Mike - you suggest reading the queue and processing data from a different thread, while @ Dave McLaughlin suggests processing the queue from the main thread. I guess I will try both…

I am going to have to look for some similar code snippets for some help, but thank you both. If you feel so inclined, a simple code example would be much appreciated.
Thanks,
Tom

I suggested a separate thread because you were blinking the LED in the main thread. But, it makes no difference where you read and do the processing.

As far as code samples, have you tried to enter the word “queue” into the search box on the Codeshare page?