Optimise serialport data recieved handling

Question

5.00/5 (1 vote)

See more:

Hi all,

I am receiving 5 data packets per second and loosing packets because handling the data received event and processing data takes too much time.

How can I improve my code?
This is the relevant code:

C#

private void portCs_Datarecieived(object sender, SerialDataReceivedEventArgs e)
      // Fixed frame 19 bytes      
      //<stx><dle><16 data bytes><etx>
      {
          if (!comportCcs.IsOpen) return;

          while (comportCcs.BytesToRead)
          {
              int indata = comportCcs.ReadByte();

              if (indata == (int)stx && readBuffer.Count == 0)//Check startbyte = stx
              {
                  readBuffer.Add((byte)indata);
                  indata = comportCcs.ReadByte();//Read next byte
              }
                  if (indata == (int)dle &&readBuffer.Count==1)//Check for
                                                              //Data delimiter dle
              {
                  readBuffer.Add((byte)indata);
                  indata = comportCcs.ReadByte();// Read next byte
              }
              else
              {
                  comportCcs.DiscardInBuffer();//Start over
                  readBuffer = new List<byte>();
              }
              while (readBuffer.Count < 18)//>???? for pasting

              {
                  readBuffer.Add((byte)indata);
                  indata = comportCcs.ReadByte();// Read next byte
              }
              if (readBuffer.Count == 18)indata == (int)etx)// Check byte[19] = etx
              {
                  readBuffer.Add((byte)indata);
                  processCsData();// This takes some time,
                                  //is already optimized with threading
                  readBuffer = new List<byte>();
              }
              else //Start over
              {

                  readBuffer = new List<byte>;();
              }
          }
      }

 private void processCsData()
        {
            CsreceivedBytes = new byte[19];
            CsreceivedBytes = readBuffer.ToArray();
            Ccs1 = bytesToInt(CsreceivedBytes, 2, 3);
            Ccs2 = bytesToInt(CsreceivedBytes, 4, 5);
            Ccs3 = bytesToInt(CsreceivedBytes, 6, 7);
            Ccs4 = bytesToInt(CsreceivedBytes, 8, 9);
            Ccs5 = bytesToInt(CsreceivedBytes, 10, 11);
            Ccs6 = bytesToInt(CsreceivedBytes, 12, 13);
            Ccs7 = bytesToInt(CsreceivedBytes, 14, 15);
            Ccs8 = bytesToInt(CsreceivedBytes, 16, 17);
            PaintCcs();
            paintBoxxes();
        }

 private static int bytesToInt(byte[] buffer, int index1, int index2)
        {
            int numLow = (int)(buffer[index2]);
            int numHigh = (int)(buffer[index1]);
            int value = 256 * numHigh + numLow;
            return value;
        }

<pre> private void PaintCcs()
        {            
            Color red = Color.Red;
            Color white = Color.White;             
            BzBitmapBuffer = new Bitmap(simuPnlBz.Width, simuPnlBz.Height);
            AzBitmapBuffer = new Bitmap(simuPnlAz.Width, simuPnlAz.Height);

            Task.Factory.StartNew(() =>
                {
                    using (Graphics bufferGrphBZ = Graphics.FromImage(BzBitmapBuffer),
                        bufferGrphAZ = Graphics.FromImage(AzBitmapBuffer))
                    {
                       
                        //Here calculations and
                        // Creating 2 bitmaps
                    }
                    simuPnlAz.Invoke(new Action(() =>
                    {
                        simuPnlAz.Invalidate();
                        simuPnlAz.Update();       
                    }));

                    simuPnlBz.Invoke(new Action(() =>
                    {
                        simuPnlBz.Invalidate();
                        simuPnlBz.Update();
                    }));
                });            
        }

        private void paintBoxxes()
        {
            PanelShiftBuffer = new Bitmap(panelShift.Width,panelShift.Height);
             Task.Factory.StartNew(() =>
            {
            using (Graphics bufferGrph = Graphics.FromImage(PanelShiftBuffer))
            {
                SolidBrush redBrush = new SolidBrush(Color.Red);
                SolidBrush greenBrush = new SolidBrush(Color.Green);
                SolidBrush myBrush = new SolidBrush(Color.Transparent);
                //int count = 0;
                //foreach (UInt16 element in ring)
                //{
                    count += 1;
                    if ( count % 2 == 0)myBrush = redBrush ;
                    else myBrush = greenBrush;
                    bufferGrph.FillRectangle(myBrush, new Rectangle(4 * 30, 4, 25, 20));
                //}
            }
            panelShift.Invoke(new EventHandler(delegate
            {
                panelShift.Invalidate();
                panelShift.Update();
            }));
            });

        }

I hope someone can improve my code for speed,

GrooverFromHolland;

Posted 24-Dec-13 2:08am

GrooverFromHolland

Updated 24-Dec-13 2:37am

Gitanjali Singh

v2

Add a Solution

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Sergey Alexandrovich Kryukov · Accepted Answer · 2013-12-24T05:22:00

Solution 1

Just some recommendations, but they might be a key solving your performance problem and the problem with loosing data.

Do all serial port communications in a separate thread and use only the synchronous read. Your thread is supposed to be blocked at the reading calls when the data sent is not yet arrived.

Instead of using ReadByte try to read by bigger blocks by using int System.IO.Ports.SerialPort.Read(byte[], int, int):
http://msdn.microsoft.com/en-us/library/ms143549%28v=vs.110%29.aspx[^].

The reading of a block will introduce a delicate problem related to blocking and not loosing some data not delivered from the other end of the cable at the moment of waking up your thread. In my recent answer I explained who to work with it in detail. Please see:
thread.suspend, thread.resume methods[^].

No, the question referenced above is not about thread suspend/resume, this is about the same serial port problem close to the one you have. :-)

—SA

Posted 24-Dec-13 5:22am

Sergey Alexandrovich Kryukov

Updated 24-Dec-13 5:24am

v2

Comments

[no name] 24-Dec-13 12:00pm

+5Is not also separation of reading input, processing input, presentation of processed data a question here?

Sergey Alexandrovich Kryukov 24-Dec-13 12:40pm

Thank you.
Is it a question? I cannot quite understand. This is not related to presentation, but of course the whole application cycle will affect the ability to process all the data in time to avoid loosing it.
—SA

[no name] 24-Dec-13 13:01pm

It's my english ;) Yes should be a question.

I mean reading a serial input without loosing packages is not a problem today.
Processing data maybe is a bottleneck.

So separating the two jobs should solve the problem. In the meaning, making a receiver which simply puts the packages into a queue which will be processed by another thread.

Then ok if processing raw data is really time consuming then maybe memory becomes a problem. But this I think anyway can only be solved by improving the algorithms or improving the hardware.

Sergey Alexandrovich Kryukov 24-Dec-13 13:43pm

I basically agree; serial port is slow enough. Nevertheless, OP ran into the insufficient throughput problem, which is always possible. My point is: the problem is not the fine optimization on low level but the general design of the code. Apparently, if something is slow enough, one can always devise such a perverted design that the software still won't cope with the job... :-)
—SA

GrooverFromHolland 24-Dec-13 12:20pm

Hi Sergey,

Thank You for answering,

I started with SerialPort.Read(byte[]and store the result in a buffer but than it is difficult to stay synchronized with the start and end byte and stay in sync, as the stream comes continuously to the port, even before my application has started.

Maybe I can start with reading byte by byte until a full and valid packet has arrived and then start reading 19 bytes at once and continue reading bytes[] until a packet is not valid or incomplete and start over again reading byte by byte until in sync again.
occasionally loosing a packet is not a problem.
what do You think of this approach?

Anny one can comment on this,

GrooverFromHolland

Sergey Alexandrovich Kryukov 24-Dec-13 12:41pm

Reading byte by byte slows generally slows down processing. Try to read by reasonably big blocks... What is the size of some logical chunk of data, typically? Try to receive the whole logical unit in one chunk if possible.
—SA

GrooverFromHolland 24-Dec-13 13:43pm

I have implemented my idea (reading byte by byte and when in sync read one packed of 19 bytes at once)in my project and tested it.
After opening serialport the first packet is incomplete the second is valid(reading byte by byte) and the rest reading full packet at once are all valid and processed in time.
running for 15 minutes at 6.3 packets of 19 bytes per second: no packets lost, all valid.

GrooverFromHolland

PS

I will submit my serial event code with comment after more testing at higher speed.

Sergey Alexandrovich Kryukov 24-Dec-13 13:49pm

No wonder, the serial port is slow enough. However, as you reported the problem, I sent you the advice which should help.
—SA

thatraja 24-Dec-13 21:10pm

5!

Sergey Alexandrovich Kryukov 24-Dec-13 21:27pm

Thank you, Raja.
—SA

GrooverFromHolland 25-Dec-13 1:27am

Hi all,

Found a serious bug in my code!
If a invalid packet is received the program hangs in the data received event.
I forgot to check invalid packet.
I will update my code,

GrooverFromHolland

GrooverFromHolland · Accepted Answer · 2013-12-24T08:59:00

Hi all,

Have been testing for 30 minutes at 50 packets per second (packet 19 bytes)and no data lost.
At higher speed I get an exception: System.InvalidOperationException occurred Message = Object is currently in use elsewhere. Source=System.Drawing

This is no problem for me as speed is 10 times higher than needed and has nothing to do with serial port.

Here is my modified serialport_Datareceived event:

C#

private bool sync = false;

       private void portCs_Datarecieived(object sender, SerialDataReceivedEventArgs e)

       // Fixed frame 19 bytes
       //<stx><dle><16><etx>

       {
           if (!comportCcs.IsOpen) return;
           if (sync) //one complete and valid packet was received
           {
               byte[] buffer = new byte[19];// packet size
               int bytes = comportCcs.BytesToRead;
               if (bytes > 18) //check if at least one packet has arrived
               {
                   comportCcs.Read(buffer, 0, 19);
                   if (buffer[0] == stx && buffer[1] == dle && buffer[18] == etx)
                   {
                       sync = true;//packet was valid
                       CsreceivedBytes = new byte[19];
                       CsreceivedBytes = buffer;
                       processCsData(CsreceivedBytes);
                   }
                   else// this I forgot
                   {
                       comportCcs.DiscardInBuffer();//Start over
                       sync = false;
                       if (readBuffer.Count > 0) readBuffer = new List<byte>();
                   }
               }
           }

           if (!sync)//read until one complete valid packet is received
           {
               while (comportCcs.BytesToRead > 0)
               {
                   int indata = comportCcs.ReadByte();

                   if (indata == (int)stx && readBuffer.Count == 0)//Check for startbyte stx
                   {
                       readBuffer.Add((byte)indata);
                       indata = comportCcs.ReadByte();//Read next byte
                   }
                   else
                   {
                       comportCcs.DiscardInBuffer();//Start over
                       sync = false;
                       if (readBuffer.Count > 0) readBuffer = new List<byte>();
                   }
                   if (indata == (int)dle && readBuffer.Count == 1) //Check for data delimiter dle
                   {
                       readBuffer.Add((byte)indata);
                       indata = comportCcs.ReadByte();// Read next byte
                   }
                   else
                   {
                       comportCcs.DiscardInBuffer();//Start over
                       sync = false;
                       if (readBuffer.Count > 0) readBuffer = new List<byte>();
                   }
                   while (readBuffer.Count > 1 && readBuffer.Count < 18)// Read data
                   {
                       readBuffer.Add((byte)indata);
                       indata = comportCcs.ReadByte();// Read next byte
                   }
                   if (readBuffer.Count == 18 && indata == (int)etx)// Check if last byte (19) = etx
                   {
                       sync = true;//valid and complete packet
                       readBuffer.Add((byte)indata);
                       CsreceivedBytes = new byte[19];
                       CsreceivedBytes = readBuffer.ToArray();
                       processCsData(CsreceivedBytes);// This takes some time, is already optimized with threading
                       readBuffer = new List<byte>();
                   }
                   else //Start over
                   {
                       sync = false;
                       readBuffer = new List<byte>();
                   }
               }
           }
       }

I hope this can help others and +5 for Sergey, leading me in the right direction.

Regards,

GrooverFromHolland