Serial Port Event Handler is called twice.

Question

Given:

    gSysSerialPort.DataReceived += new SerialDataReceivedEventHandler(CaptureData);
    //------------------------------
    private void CaptureData(object sender, SerialDataReceivedEventArgs e)
    {
      Thread.Sleep(200 * g.TIME_1_MSEC);
      byte[] abyBytes = new byte[gSysSerialPort.BytesToRead];
      gSysSerialPort.Read(abyBytes, 0, abyBytes.Length);

      this.BeginInvoke(new SetTextDelegate(ReceivedDelegate), new object[] { abyBytes });
    }
    //------------------------------

This works fine ... except that sometimes the event handler is called twice.  The data is all there the first time (9600 baud).

---

cte677

Answer 1

Make sure you do not add it multiple times. That's the most common cause.

---

http://blog.voidnish.com

Answer 2

Don't know what "add it multiple times" means.  The scope shows the data is sent only once. 

---

cte677

Answer 3

I meant to check that the code that adds the event handler is only called once.

---

http://blog.voidnish.com

Answer 4

I have no control of that ... it is handled by SerialPort.DataReceived ... which is defined when the dialog box is created.

Note that the program works most of the time ... but occasionally the handler is called twice for an inexplicable reason.

---

cte677

Answer 5

"The data is all there the first time (9600 baud)."

What's there the second time?

Answer 6

Zero bytes.

---

cte677

Answer 7

 Use this code to read your data:

    private void CaptureData(object sender, SerialDataReceivedEventArgs e)
    {
      byte[] abyBytes = new byte[BytesInMessage];
      int cnt = 0;
      while (cnt < BytesInMessage)
      {
        cnt += gSysSerialPort.Read(abyBytes, Cnt, BytesInMessage - cnt);
      }
      this.Invoke(new SetTextDelegate(ReceivedDelegate), new object[] { abyBytes });
    } 


BytesInMessage is the total number of bytes you expect to read.  Set ReadTimeOut and put a try...catch around the Read.

Answer 8

The number of bytes changes ... cannot be predicted. 

---

cte677

Answer 9

The number of bytes changes ... cannot be predicted. 

---

cte677

How do you know when you have a complete message?  Read until you get a complete message.

Answer 10

The messages are between 10 and 30 msec long.  The 200 msec delay gives time to receive all of the bytes before continuing.

---

cte677

Answer 11

It would be nice if the system detected a break in the bytes being sent ... a break being detected when there are no bytes after waiting for a length of two bytes.  This would simplify everything. 

---

cte677

Answer 12

It would be nice if the system detected a break in the bytes being sent ... a break being detected when there are no bytes after waiting for a length of two bytes.  This would simplify everything. 

---

cte677

That's fairly simple.  Set and catch the ReadTiveOut as I stated in my post with code.

Answer 13

I already have it set ... but it does not work.  When I remove the Thread.Sleep(), I get an event handler for each byte.  I had expected to get an event handler when there is a break in the bytes. 

---

cte677

Answer 14

I already have it set ... but it does not work.  When I remove the Thread.Sleep(), I get an event handler for each byte.  I had expected to get an event handler when there is a break in the bytes. 

---

cte677

You either haven't copied and pasted the code correctly, or your try...catch block is incorrect.  The DataReceived event can't be fired while code is executing within the block.  That is, the event can't be fired a second time until the Invoke is executed.  With a BytesInMessage value greater than can be recieved, the only way out of the while loop is through a timeout.  this occurs when there are zero bytes to be read for the timeout period.

Answer 15

Here is the code I wrote a few months ago:

gSysSerialPort.ReadTimeout = 2000;

//------------------------------
private void CaptureSerialPortDataReceived(object sender, SerialDataReceivedEventArgs e)
{
  byte[] abyBytes = new byte[100];
  int iCount = 0;
  int iOffset = 0;
  int iNumOBytes = gSysSerialPort.BytesToRead;
  while (iCount < 100)
  {
    try
    {
      iCount += gSysSerialPort.Read(abyBytes, iOffset, iNumOBytes);
      iOffset += iNumOBytes;
      iNumOBytes = gSysSerialPort.BytesToRead;
    }
    catch
    {
      this.BeginInvoke(new SetTextDelegate(ReceivedDelegate), new object[] { abyBytes });
    }
  }
  this.BeginInvoke(new SetTextDelegate(ReceivedDelegate), new object[] { abyBytes });
}
//------------------------------

Never could get it to work ... so I tried a different approach.

It would be nice if the event handler were not called until the timeout period. 

---

cte677

Answer 16

I posted code that does what you asked and should solve your problem with the handler being called twice.  What problems are you having with the code?

Answer 17

I tried your code ... after fixing the compiler errors, it did not work.  When the delegate was called, there was only one byte.

The problem with the code I last posted is the delegate was never called.  Looking at the SerialPort with a breakpoint in the handler, I saw ReadTimeout, but no variable to determine that the timeout occurred.

---

cte677

Answer 18

"I tried your code ... after fixing the compiler errors, it did not work.  When the delegate was called, there was only one byte."

Then your timeout is too short.  Post the code.

Answer 19

The timeout is 2 secs.  It would be good to verify code works before recommending it. 

---

cte677

Answer 20

The timeout is 2 secs.  It would be good to verify code works before recommending it. 

---

cte677

I can only post code that has been thoroughly tested by users of these forums for a period of 5 years.  I can't control how an individual modifies the code.  Obviously you have changed the code in some manner.  With a 2 second timeout and a large BytesInMessage value, there is no way the code can break out of the while loop with one byte unless only one byte was received in the timeout period.

Answer 21

Try the code you posted. 

---

cte677

Answer 22

Try the code you posted. 

---

cte677

There were 2 obious errors in the posted code.  The size of the byte[] was missing and the cnt position variable was missing from the read statement.  I did also explain the code and left the addition of the try...catch and timeout to you.

Answer 23

Other items: 1) iBytesInMessage is not specified, 2) The .Read is being asked to read too many bytes, 3) The handler is called over and over again ... never stops.  It does Invoke each time, but the handler is called again after the Invoke is done ... repetitively.

  int iBytesInMessage = 100;
  byte[] abyBytes = new byte[100];
  int cnt = 0;
  while (cnt < iBytesInMessage)
  {
    try
    {
      cnt += gSysSerialPort.Read(abyBytes, cnt, iBytesInMessage - cnt);
    }
    catch
    {
      this.BeginInvoke(new SetTextDelegate(ReceivedDelegate), new object[] { abyBytes });
    }
  }

---

cte677

Answer 24

Other items: 1) iBytesInMessage is not specified, 2) The .Read is being asked to read too many bytes, 3) The handler is called over and over again ... never stops.  It does Invoke each time, but the handler is called again after the Invoke is done ... repetitively.

  int iBytesInMessage = 100;
  byte[] abyBytes = new byte[100];
  int cnt = 0;
  while (cnt < iBytesInMessage)
  {
    try
    {
      cnt += gSysSerialPort.Read(abyBytes, cnt, iBytesInMessage - cnt);
    }
    catch
    {
      this.BeginInvoke(new SetTextDelegate(ReceivedDelegate), new object[] { abyBytes });
    }
  }

---

cte677

I corrected the original post. Here's the code with the try...catch. Please don't change Invoke to BeginInvoke. That's the source of most of your problems.

    private void CaptureSerialPortDataReceived(object sender, SerialDataReceivedEventArgs e)
    { 
      int iBytesInMessage = 999;
      byte[] abyBytes = new byte[iBytesInMessage];
      int cnt = 0;
      while (cnt < iBytesInMessage)
      {
        try
        {
          cnt += gSysSerialPort.Read(abyBytes, cnt, iBytesInMessage - cnt);
        }
        catch
        {
          break;
        }
      }
      this.Invoke(new SetTextDelegate(ReceivedDelegate), new object[] { abyBytes });
    }
You'll have to experiment with the timeout value as that's what determines when a complete message has been received as per your request.

Please don't change Invoke to BeginInvoke.  That's the cause of the event being triggered when you don't expect it.

Answer 25

I was unaware of Invoke vs. BeginInvoke ... which may the reason for the problem mentioned in the first post.

It appears the code in the first post consumes fewer resources ... no multiple reads, etc.  It is simpler ... and works just as well. 

Again, it would be nice if the system detected a break in the bytes being sent ... and ran the handler then.  This would simplify it even more, be faster, and eliminate any issues regarding the length of the bytes.  It could handle any length. 

---

cte677

Answer 26

"Again, it would be nice if the system detected a break in the bytes being sent ... and ran the handler then.  This would simplify it even more, be faster, and eliminate any issues regarding the length of the bytes.  It could handle any length."

The last code I poosted detects  a break  in the bytes being sent... and runs the handler then.

The handler fires when a byte is received.  It runs until a break in the bytes is detected.  It runs again when a byte is detected.

What problems are you having with the code?

Answer 27

Both versions of the code work fine.  However, they wait a specified amount of time.  They are not detecting a break (no bytes) after the group of bytes have been sent.

---

cte677

Answer 28

Both versions of the code work fine.  However, they wait a specified amount of time.  They are not detecting a break (no bytes) after the group of bytes have been sent.

---

cte677

For the last code, that means that you always receive 999 bytes.  Is that correct?

I define a breaK as no bytes being sent for a period of time.  What is your definition of a break?

How have you determined the correct ReadTimeout?  You should start with a timeout of 1 and increase it until you always receive 999 bytes.

Answer 29

Do not receive 999 bytes.  For your code, the code continues after reaching the ReadTimeout value.  For my code, the bytes are read after the Sleep.

I define a break similarly ... although I define a break as when there are no bytes for a period of time equal to two bytes ... at 9600 baud is around 2 msec of time ... at 19200 baud is around 1 msec of time.

There is no correct ReadTimeout ... other than it needs to be long enough.  The number of bytes received varies.  And the Baud rate can be changed by the user. 

---

cte677

Answer 30

"Do not receive 999 bytes.  For your code, the code continues after reaching the ReadTimeout value.  For my code, the bytes are read after the Sleep."

I don't understand.  The only way the last code I posted can continue after a ReadTimeout is for the DataReceived handler to be fired again, which should signify the start of new message. 

How long must the timerout be to receive the full 999 bytes?

Your device is unique in the transmission of a sequence of random bytes.  All other devices tranmit messages using a well defined protocol.  Defining the stop of a message as the loss of one byte will never work.  Serial transmission isn't that reliable. 

Answer 31

Yes, the handler is fired for every new message.  And the receive buffer is always empty when the handler is called ... before the newest bytes are retrieved.  That contents of that buffer are then passed on via the Invoke

For 9600 baud, 999 bytes times ~1 msec is around 1 sec.

Serial transmission must be reliable or it would not communicate. It needs a start bit and a stop bit.  The distance between rising and falling edges must be consistent.  Use a scope to see what is happening ... it is always consistent.

I think the minimum time between messages is two bytes.  Just one byte missing, I think, is defined as a framing error. 

---

cte677

Answer 32

"Just one byte missing, I think, is defined as a framing error."

A framing error is an error in the timing of the bit stream.  This is a low level hardware error.  I don't think any device would define a message based upon a framing error.  Framing errors fire the ErrorReceived event.

"I think the minimum time between messages is two bytes."

"The messages are between 10 and 30 msec long.  The 200 msec delay gives time to receive all of the bytes before continuing"

The hanler fires when the first byte is received.  If the maximum message length is 30 msec, the delay between message must be at least 200 -30 = 170 msec. 

Answer 33

The "10 and 30 msec long" is accurate for my current testing.  It will change significantly when more bytes of data are received ... and when the baud rate is changed. 

Again, it would be nice if the system detected a break in the bytes being sent ... and ran the handler after detecting the break.  It would handle the bytes regardless number of bytes and regardless of the baud rates ... and it would respond immediately (in msecs) instead of waiting for the time out. 

---

cte677

Answer 34

The code I've posted in this thread was taken from a GPS application.  GPS applications send their messages as a variable number of lines of characters every second.  The time between messages when nothing is being sent can be detected using the timeout feature of the SerialPort Read method.  If there are any bytes available, the Read method reads them and returns.  If no bytes are available, the Read method waits until a byte is available.  The wait can be broken by the ReadTimeOut property.  The grouping of the GPS messages together as they are sent each second simplifies processing. 

This is the complete code:

using System;
using System.IO.Ports;
using System.Text;
using System.ComponentModel;
using System.Windows.Forms;

namespace WindowsFormsApplication1
{
  public partial class Form1 : Form
  {
    public Form1()
    {
      InitializeComponent();
    }
    private SerialPort SP = new SerialPort("COM3", 4800);
    private MenuStrip MS = new MenuStrip();
    private TextBox TB = new TextBox();
    protected override void OnLoad(EventArgs e)
    {
      base.OnLoad(e);
      MS.Items.Add(new ToolStripButton("Open"));
      MS.ItemClicked += MS_Clicked;
      MS.Parent = this;
      TB.ScrollBars = ScrollBars.Both;
      TB.Multiline = true;
      TB.Parent = this;
      TB.BringToFront();
      TB.Dock = DockStyle.Fill;
      SP.DataReceived += SP_DataReceived;
      SP.ReadTimeout = 200;
    }
    protected override void OnClosing(CancelEventArgs e)
    {
      base.OnClosing(e);
      if (CloseForm)
      {
        return;
      }
      e.Cancel = true;
      CloseForm = true;
      SPClose = true;
    }
    public void MS_Clicked(object sender, ToolStripItemClickedEventArgs e)
    {
      if (e.ClickedItem.Text == "Open")
      {
        SPClose = false;
        SP.Open();
        e.ClickedItem.Text = "Close";
      }
      else
      {
        SPClose = true;
        e.ClickedItem.Text = "Open";
      }
    }

    public delegate void AppendText(string Text);
    public delegate void FormClose();
    private bool SPClose;
    private bool CloseForm;
    public void SP_DataReceived(object sender, SerialDataReceivedEventArgs e)
    {
      //There is at least 1 byte available to be read.
      char[] GPSChars = new char[1000]; //The byte[] is dimensioned larger than the largest message length.
      int Cnt = 0;  //Position in the array to write the bytes.
      while (Cnt < GPSChars.Length)
      {
        try
        {
          //Read as long as there are bytes available to be read.
          Cnt += SP.Read(GPSChars, Cnt, GPSChars.Length - Cnt);
        }
        catch (Exception)
        {
          //No bytes were available, so Read timed out.
          break;
        }
      }
      //Make a string of the bytes and process on UI thread.
      StringBuilder SB = new StringBuilder(Cnt);
      for (int I = 0; I < Cnt; I++)
      {
        SB.Append(GPSChars[I]);
      }
      this.Invoke(new AppendText(TB.AppendText), SB.ToString() + Environment.NewLine);
      //Close Port in reponse to user's request or when closing.
      if (SPClose)
      {
        SP.Close();
        if (CloseForm)
        {
          this.Invoke(new FormClose(this.Close));
        }
      }
      //Exit handler and wait for next message.
    }
  }
}

Answer 35

The GPS code also relies on a timeout ... but it doesn't matter since detecting when bytes stop is currently not an option. 

However, I discovered something new.  When I use Invoke, the system hangs when I try to change the baud params => hangs on gSysSerialPort.Close():

//------------------------------
private void SetSerialPortParams(string[] astBaudParams)
{
  string stComPort = astBaudParams[sps.FILE_COM_NUM];
  Int16 iBaudRate = Convert.ToInt16(astBaudParams[sps.FILE_COM_BAUD]);

  try
  {
    gSysSerialPort.Close();
    gSysSerialPort.PortName = stComPort;
    gSysSerialPort.Open();
    gSysSerialPort.BaudRate = iBaudRate;
    ...
  }
  catch
  {
    MessageBox.Show("Serial port connection problem.");
  }
}//SetSerialPortParams
//------------------------------

When I use BeginInvoke it works fine.

---

cte677

Answer 36

Hi,

It's been a long time but AFAIK for serial port and network programming the idea is rather to adapt to how the underlying layer exposes the data rather than to try to get a message using a single event (for example for a serial port how many events you'll get likely depends on the size of on the underlying software and perhaps even hardware buffers and IMO could vary on differents PCs).

So I would say rather that you have to get what is passed and this is up to you to determine when you received a full message.

If this is not solved yet :
- have you tried  to inspect SerialDataReceivedEventArgs.EventType. Also when something unclears happens my approach is to handle and inspect all I can (ErrorReceived, PinChanged and trace this)
- and/or to clarify the situation I would try the code shown at http://msdn.microsoft.com/en-us/library/system.io.ports.serialport.datareceived (not that it does not use exactly the same principle than yours).

For now my understanding is that data are correctly received but that you have an extra event that seems useless... (could be an EventType.Eof ?) but I'm not 100% sure if instead you are not seeing that data are currently not exactly what you expect.

---

Please always mark whatever response solved your issue so that the thread is properly marked as "Answered".

Answer 37

None of the responses resolved the issue.  None of the suggestions work better than my first post ... and they hang when changing the baud params.  The first post is simpler.

The improvement to the first post is to verify abyBytes contains bytes:

  //------------------------------
  gSysSerialPort.DataReceived += new SerialDataReceivedEventHandler(CaptureData);
  //------------------------------
  //------------------------------
  private void CaptureData(object sender, SerialDataReceivedEventArgs e)
  {
    Thread.Sleep(200 * g.TIME_1_MSEC);
    byte[] abyBytes = new byte[gSysSerialPort.BytesToRead];
    gSysSerialPort.Read(abyBytes, 0, abyBytes.Length);

    this.BeginInvoke(new SetTextDelegate(ReceivedDelegate), new object[] { abyBytes });
  }
  //------------------------------
  //------------------------------
  private void ReceivedDelegate(byte[] abyBytes)
  {
    if(abyBytes.Count() == 0)
      return;
    ...
  }
  //------------------------------

I know of no answer ... but a solution to handle the problem ... as I described.  I don't plan to vote for my posts.

It would be good if someone at Microsoft wrote code to make receiving serial port bytes easier and seamless.  The code would detect a break in the bytes being sent ... and run the handler after detecting the break.  The break (usually two bytes long) would be dependent on the baud rate set. 

This would make it easier for all code writers, regardless of the baud rate, number of bytes, etc. ... and faster.

---

cte677

Answer 38

"None of the suggestions work better than my first post ... and they hang when changing the baud params."

You must not have copied and pasted the code I posted correctly.  Post your complete code that hangs.

Carsten Kanstrup and Dick Grier, frequent contributors to these forums have published articles that might help you better understand the SerialPort.

Carsten has published a web based tutorial covering the SerialPort, events, delegates and threads in detail.  Dick has published a book that covers using the SerialPort with a wide variety of devices.

Answer 39

See the post dated Thursday, May 10, 2012 12:26 PM.

---

cte677

Answer 40

See the post dated Thursday, May 10, 2012 12:26 PM.

---

cte677

Why haven't you copied the code I posted?  The code you posted has no provision for closing the port.

Try Carsten's tutorial.  He covers communication between threads quite well.

Answer 41

I tried the code you posted.  See the post dated Thursday, May 10, 2012 12:26 PM. 

The BeginInvoke (or Invoke) function sorts the bytes.  If the bytes indicate the baud rate will change, appropriate action is taken.

The bottom line is that the code that uses Invoke instead of BeginInvoke does not allow baud params to be changed.

What does Carsten's tutorial say about the event handler being called twice? ... what does it say about changing serial port params via Invoke vs. BeginInvoke?

---

cte677

Answer 42

I had no problem changing the baud rate using the code I posted after adding this statement:

        if (SP.BaudRate == 4800) SP.BaudRate = 9600; else SP.BaudRate = 4800;

either within the DataReceived event or in a button event added to the MenuStrip.

After reading Carsten's tutorial, you should understand why your code results in the occasional random calling of the DataReceived event.

Answer 43

Serial port params ... not just baud rate ... see posts dated Friday, May 11, 2012 1:16 PM ... and Friday, May 11, 2012 2:47 PM.

---

cte677

Answer 44

Serial port params ... not just baud rate ... see posts dated Friday, May 11, 2012 1:16 PM ... and Friday, May 11, 2012 2:47 PM.

---

cte677

 The code I posted doesn't have problems changing any of  the Serial port parameters, particularly if you close the port before changing them.

Answer 45

Read the post dated Thursday, May 10, 2012 12:26 PM. 

---

cte677

Answer 46

Read the post dated Thursday, May 10, 2012 12:26 PM. 

---

cte677

Is there a reason?  I can only try to help you.  I can't force you to accept my assistance.

Answer 47

Read the post dated Friday, May 11, 2012 1:16 PM.

---

cte677

Answer 48

Read the post dated Friday, May 11, 2012 1:16 PM.

---

cte677

I've read all of your posts.  I've provided code that corrects problems with your code.  I've suggested documentation that you should read to acquire the knowledge to effectively use the SerialPort and understand the code I posted.  Your code has problems, the code I posted doesn't.  If you continue to use your code, you will continue to have problems.

Answer 49

Your code does not correct the problem.  Read the post dated Friday, May 11, 2012 1:16 PM.

---

cte677

Answer 50

Your code does not correct the problem.  Read the post dated Friday, May 11, 2012 1:16 PM.

---

cte677

What problem doesn't it correct?  It doesn't fire the DataReceived event twice and it doesn't hang.  I don't see a new problem in any of your posts.

Answer 51

From Thursday, May 10, 2012 12:26 PM:

When I use Invoke, the system hangs when I try to change the baud params => hangs on gSysSerialPort.Close():

//------------------------------
private void SetSerialPortParams(string[] astBaudParams)
{
  string stComPort = astBaudParams[sps.FILE_COM_NUM];
  Int16 iBaudRate = Convert.ToInt16(astBaudParams[sps.FILE_COM_BAUD]);

  try
  {
    gSysSerialPort.Close();
    gSysSerialPort.PortName = stComPort;
    gSysSerialPort.Open();
    gSysSerialPort.BaudRate = iBaudRate;
    ...
  }
  catch
  {
    MessageBox.Show("Serial port connection problem.");
  }
}//SetSerialPortParams
//------------------------------

When I use BeginInvoke it works fine.

---

cte677

Answer 52

From Thursday, May 10, 2012 12:26 PM:

When I use Invoke, the system hangs when I try to change the baud params => hangs on gSysSerialPort.Close():

//------------------------------
private void SetSerialPortParams(string[] astBaudParams)
{
  string stComPort = astBaudParams[sps.FILE_COM_NUM];
  Int16 iBaudRate = Convert.ToInt16(astBaudParams[sps.FILE_COM_BAUD]);

  try
  {
    gSysSerialPort.Close();
    gSysSerialPort.PortName = stComPort;
    gSysSerialPort.Open();
    gSysSerialPort.BaudRate = iBaudRate;
    ...
  }
  catch
  {
    MessageBox.Show("Serial port connection problem.");
  }
}//SetSerialPortParams
//------------------------------

When I use BeginInvoke it works fine.

---

cte677

We determined that your code performs in this manner many, many posts ago. What problems do you have incorporating the code I posted or code recommended by Carsten to eliminate this problem.

Answer 53

This problem occurs using your code.  Everything in your post works, except the Invoke -> which can include a call to a function that changes baud params.  Replace Invoke with BeginInvoke in your code, and your code works fine on my computer.

I strive to write code that is bullet proof ... that works on all computers.  The code you posted does not work on my computer, unless I replace Invoke with BeginInvoke.  The code I posted does work ... see Friday, May 11, 2012 1:16 PM for a summary.

---

cte677

Answer 54

This problem occurs using your code.  Everything in your post works, except the Invoke -> which can include a call to a function that changes baud params.  Replace Invoke with BeginInvoke in your code, and your code works fine on my computer.

I strive to write code that is bullet proof ... that works on all computers.  The code you posted does not work on my computer, unless I replace Invoke with BeginInvoke.  The code I posted does work ... see Friday, May 11, 2012 1:16 PM for a summary.

---

cte677

 "The code you posted does not work on my computer, unless I replace Invoke with BeginInvoke."

I simply don't believe you.  If you had read Carsten's tutorial you would understand how the code I posted synchronizes the threads to allow orderly closing of the port and the form.  Post your complete code that incorporates the code I use to synchronize the threads when Invoke is used.

You must have left something out when you copied my code and changed the names of the objects.  But you keep posting your code.  Your code simply doesn't provide for problems introduced by the DataReceived event running on its own thread.