7 replies to this topic

[Omar (OZ)]

I am trying to read data from the uart port... and I am using the .net's serial port (drag and drop from windows form toolbox) here is my code:

 public partial class Main : Form
    {
        byte[] buff = new byte[0];

        public Main()
        {
            InitializeComponent();
        }

        private void Main_Load(object sender, EventArgs e)
        {
            com.Open();
            updater.Start();
        }

        private void com_DataReceived(object sender, System.IO.Ports.SerialDataReceivedEventArgs e)
        {
            buff = new byte[buff.Length + com.BytesToRead];
            com.Read(buff, 0, com.BytesToRead);
        }
        private void updater_Tick(object sender, EventArgs e)
        {
            if (buff != null)
            {
                dataReceived.Text += Encoding.UTF8.GetString(buff);
                dataReceived.ScrollToCaret();
                buff = new byte[0];
            }
        }

    }

I am trying to get the bytes received to be written on the rich text box called 'dataReceived'. The thing is I can't write things in the text box from the com_DataReceived event, because it runs on a separate thread. what am I supposed to do now? the timer seems to cause issues, and it skips bytes. In that code you can see my attempts to get around disappearing bytes.

here is what I am getting looks like:
30,8NP0003
G0,M,G,,EG06,00$T,0,3
070,EP,,1C0,,,,P0,,,
G0,,,
1,,M0V,0,B,00K3
.,,,G,,,G0
09,,0*T0.,3
G0,,M
A,,EC30,64G0002
05,,9P,,EP0V00,
0T,0
$A3,,$A,,$C0,01,P0,,NP060,PA,,
0.,,1
G0002
P46,,P,,,P0
M0,,1,V000N
030,$A,,
C000*P,,,*
G.,,MP,,1G0,00140T,0
P0,,MP,,,$C3,00N
0.,*
G03,,G,,,G03,,1VT00N
P9,,MP,,,G07R.,0,,$0M0K
G0,,MPA,,
C3,06,VTM.2
G1,,,P,,*$13.0,4,M00
0.,,$A,,
02,,,N
0.,K
P0,MG,,*
0V

it should look something like this:
$GPGSA,A,1,,,,,,,,,,,,,,,*1E
$GPGSV,1,1,00*79
$GPRMC,000129.036,V,,,,,0.00,0.00,060180,,,N*4D
$GPVTG,0.00,T,,M,0.00,N,0.00,K,N*32
$GPGGA,000130.036,,,,,0,0,,,M,,M,,*4F
$GPGSA,A,1,,,,,,,,,,,,,,,*1E
$GPRMC,000130.036,V,,,,,0.00,0.00,060180,,,N*45
$GPVTG,0.00,T,,M,0.00,N,0.00,K,N*32
$GPGGA,000131.036,,,,,0,0,,,M,,M,,*4E
$GPGSA,A,1,,,,,,,,,,,,,,,*1E
$GPRMC,000131.036,V,,,,,0.00,0.00,060180,,,N*44
$GPVTG,0.00,T,,M,0.00,N,0.00,K,N*32
$GPGGA,000132.036,,,,,0,0,,,M,,M,,*4D
$GPGSA,A,1,,,,,,,,,,,,,,,*1E
$GPRMC,000132.036,V,,,,,0.00,0.00,060180,,,N*47

A Console app works great..

class Program
    {
        static SerialPort com;
        static byte[] buff;

        static void Main(string[] args)
        {
            com = new SerialPort("COM10", 4800, Parity.None, 8, StopBits.One);
            com.DataReceived += new SerialDataReceivedEventHandler(com_DataReceived);
            com.Open();
            Thread.Sleep(-1);
        }

        static void com_DataReceived(object sender, SerialDataReceivedEventArgs e)
        {
            char character = (char)com.ReadByte();

            if (character == '$')
                Console.Write("----------------------------------------------------------\n");

            Console.Write(character); 
        }
    }

[Chris Walker]

Hi Omar,

Your timer is running at a regular interval, and it's quite likely that your buffer is getting overwritten by new data before you display it. In fact, I'm pretty sure that's what is going on.

The good news is that you can create an "anonymous method" on the fly and call it from within your handler. The anonymous method will be invoked on your Windows forms thread.

Try this (from memory...not tested):

this.Invoke( delegate {
    dataReceived.Text += Encoding.UTF8.GetString(buff); 
    dataReceived.ScrollToCaret(); 
});

Chris

[Omar (OZ)]

Hi Omar,

Your timer is running at a regular interval, and it's quite likely that your buffer is getting overwritten by new data before you display it. In fact, I'm pretty sure that's what is going on.

The good news is that you can create an "anonymous method" on the fly and call it from within your handler. The anonymous method will be invoked on your Windows forms thread.

Try this (from memory...not tested):

this.Invoke( delegate {
    dataReceived.Text += Encoding.UTF8.GetString(buff); 
    dataReceived.ScrollToCaret(); 
});

Chris

Thanks Chris.

Here is it if anyone needs it:

public partial class main : Form
    {
        char c;
        string line = "";
        public delegate void delegate1();
        delegate1 updater;

        public main()
        {
            InitializeComponent();
        }

        private void main_Load(object sender, EventArgs e)
        {
            updater = new delegate1(updateData);
            com.Open();
        }

        private void com_DataReceived(object sender, System.IO.Ports.SerialDataReceivedEventArgs e)
        {
            c = (char)com.ReadByte();
            this.Invoke(updater);
        }

        private void updateData()
        {
            if (c != '\n')
                line += c;
            else
            {
                datalines.Items.Add(line);
                line = "";
            }
        }

        private void stop_Click(object sender, EventArgs e)
        {
            com.Close();
        }
    }

[Corey Kosak]

Omar, going back to your original code, the confused output you are seeing was most likely caused by the corrupt logic in this routine:

private void com_DataReceived(object sender, System.IO.Ports.SerialDataReceivedEventArgs e)
{
  buff = new byte[buff.Length + com.BytesToRead];
  com.Read(buff, 0, com.BytesToRead);
}

It looks like your intention was that if multiple calls to com_DataReceived were to happen in a row, without an intervening call to updater_Tick, that you would keep appending incoming data to buff (hence the calculation buff.Length+com.BytesToRead). However, that's not what happens here. You do correctly calculate the array length, but you have two other problems: (a) in line 1, you allocate an entirely new buffer, and (b ) in line 2, you copy your new incoming data to the beginning of the buffer, rather than at the proper offset. The major effect of this would be to lose data (in addition, you'll see buffers with extra 0 bytes at the end, but that probably wasn't noticeable).

One way to fix this is to allocate a new buffer of the proper size, and then copy the old data to the beginning of the new buffer and then read in the new data from the serial port starting at the proper offset rather than 0. Even if you did so, you'd still have to do something about the race condition Chris referred to. (By the way, there is a second, more theoretical problem, in that all this array copying you are doing would be quadratic in complexity. The really nice solution would have you double the allocated capacity every time you ran out of space).

I don't much care for the solution involving Invoke(), because it will block the calling thread until the Forms thread is done with the call. I guess it will work though. My preference would be to (a) not worry about appending to buff, and then (b ) to remove it from your member variables and therefore stop sharing it altogether:

private void com_DataReceived(object sender, System.IO.Ports.SerialDataReceivedEventArgs e)
{
  var buff = new byte[com.BytesToRead]; //note: buff is private
  com.Read(buff, 0, buff.Length);
  this.BeginInvoke( delegate {
    dataReceived.Text += Encoding.UTF8.GetString(buff); 
    dataReceived.ScrollToCaret(); 
  });
}

Now, your char-at-a-time approach may be correct, but is pretty inefficient, as it requires a thread context switch for every single received character. (Additionally, "line+=c" is also pretty inefficient, because it takes time that is quadratic in the number of characters appended)

While I'm at it, I should also take this moment to point out that this kind of usage of Encoding.UTF8.GetString() will also break if anyone ever actually tries to send Unicode characters over a serial port, rather than 7-bit ASCII, because this code doesn't properly handle the case where a multibyte Unicode character is split across multiple calls to com.Read() (i.e. split over multiple buffers)

[Omar (OZ)]

I don't much care for the solution involving Invoke(), because it will block the calling thread until the Forms thread is done with the call. I guess it will work though. My preference would be to (a) not worry about appending to buff, and then (b ) to remove it from your member variables and therefore stop sharing it altogether:

You're completely right, I can't close my form! What do you suggest I do?

[Corey Kosak]

You're completely right, I can't close my form! What do you suggest I do?

Well, that's probably some other problem :-)

Is your solution isolatable enough that you can post the whole thing here in such a way that I or others can run it? That would help a lot

[Omar (OZ)]

Well, that's probably some other problem :-)

Is your solution isolatable enough that you can post the whole thing here in such a way that I or others can run it? That would help a lot

Nah, I don't want to waste your time. I'll figure something out, its a fun problem anyways. Thanks for the info though!

[Omar (OZ)]

Fixed! I can stop it now.
(easy fix)

public partial class main : Form
    {
        char c;
        string line = "";
        public delegate void delegate1();
        delegate1 updater;
        bool stopCom = false;
        public main()
        {
            InitializeComponent();
        }

        private void main_Load(object sender, EventArgs e)
        {
            updater = new delegate1(updateData);
            com.Open();
        }

        private void com_DataReceived(object sender, System.IO.Ports.SerialDataReceivedEventArgs e)
        {
            if (!stopCom)
            {
                c = (char)com.ReadByte();
                this.Invoke(updater);
            }
            else
                com.Close();
        }

        private void updateData()
        {
            if (c != '\n')
                line += c;
            else
            {
                datalines.Items.Add(line);
                line = "";
            }
        }

        private void stop_Click(object sender, EventArgs e)
        {
            stopCom = true;
        }
    }