views:

578

answers:

5

Can anyone tell my why the code below would be missing or dropping data. I have been googling all night but can't find any pointers. the serial port is using the following settings; 38400,n,8,1 xon/xoff flow control. ReceivedBytesThreshold = 1.

isReceiving and stockReceived are both members of the form

       private void serialPort1_DataReceived(object sender, System.IO.Ports.SerialDataReceivedEventArgs e)
    {
        string dataReceived = serialPort1.ReadExisting();
        bytecount += dataReceived.Length;
        processSerialData(dataReceived);
    }

    private void processSerialData(string dataReceived)
    {
        if (isReceiving == false)
        {
            int stxpos = dataReceived.IndexOf('\x02');
            if (stxpos != -1)
            {
                dataReceived = dataReceived.Replace("\x02", "");
                labelcaption = "Receiving... ";
                this.Invoke(new EventHandler(SetLabel));
                isReceiving = true;
            }
        }

        int etxpos = dataReceived.IndexOf('\x03');
        if (etxpos != -1)
        {
            dataReceived = dataReceived.Replace("\x03", "");
            //tmpFile.Write(dataReceived);
            writeToFile(dataReceived);
            tmpFile.Close();
            isReceiving = false;
            stockReceived = true;
        }

        // Now we need to write the data to file
        if (isReceiving == true)
        {
            if ((bytecount / recordSize) % 100 == 0)
            {
                labelcaption = "Receiving... " + (bytecount / recordSize);
                this.Invoke(new EventHandler(SetLabel));
            }
            //tmpFile.Write(dataReceived);
            writeToFile(dataReceived);
        }
    }
+1  A: 

A couple potential problems (not saying they are the problem, but they raise red flags for me) are:

  • You're using a string and then looking for non-printable character data. This is a suspect processing idea IMO. You should be receiving bytes and then looking for byte values. Who know what the underlying encoding might do, but loss of some characters, especially if it's using ASCII encoding, wouldn't surprise me a bit.
  • This looks like a potential thread issue as well. When you get a DataRecieved event, you read the data and then go on and process the data in the context of teh receive handler. What happens if you get anotehr event while you're still processing the last? I'm betting the byteCount variable gets hosed. Personally I'd have a thread dedicated to receiving the data and anotehr for processing it. At the very least I'd add some sort of synchronization object in there.
ctacke
Thanks ChrisI did try the same withthe processing code in a lock block. but that didn't work. Should I be using something other than than lock(syncObject) in your opinion?I'm only looking for STX and ETX which should fall into the ASCII encoding shouldn't it? The lost data only occurs on largish files, the one I am testing on is about 484Kb and it is literally the odd few characters in the midle of the file. The code always seems to pick up the stx/etx ok.
Thanks Chris I did try the same withthe processing code in a lock block. but that didn't work. Should I be using something other than than lock(syncObject) in your opinion? I'm only looking for STX and ETX which should fall into the ASCII encoding shouldn't it? The lost data only occurs on largish files, the one I am testing on is about 484Kb and it is literally the odd few characters in the midle of the file. The code always seems to pick up the stx/etx ok
+1  A: 

You do not handle the case of recieving multiple STX's, or ETX's in one DataReceived packet.

jyoung
A: 

Thanks for the comments so far, I have modified the code as shown below but it is still losing data all help gratefully received if I had any hair left I would be ripping it out now!

        private void serialPort1_DataReceived(object sender, System.IO.Ports.SerialDataReceivedEventArgs e)
    {
        //manager.ReceivedBytesThreshold = 1024;
        lock (syncObject)
        {
            string dataReceived = String.Empty;
            dataReceived = manager.ReadExisting();
            processSerialData(dataReceived);
        }
    }

    private void processSerialData(string dataReceived)
    {
        if (isReceiving == false)
        {
            int stxpos = dataReceived.IndexOf('\x02');
            if (stxpos != -1)
            {
                dataReceived = dataReceived.Replace("\x02", "");
                labelcaption = "Receiving... ";
                this.Invoke(new EventHandler(SetLabel));
                isReceiving = true;
                stockReceived = false;
            }
        }

        // Now we need to write the data to file
        if (isReceiving == true)
        {
            int etxpos = dataReceived.IndexOf('\x03');
            if (etxpos != -1)
            {
                dataReceived = dataReceived.Replace("\x03", "");
                isReceiving = false;
            } 

            bytecount += dataReceived.Length;

            if ((bytecount / recordSize) % 100 == 0)
            {
                labelcaption = "Receiving... " + (bytecount / recordSize);
                this.Invoke(new EventHandler(SetLabel));
            }

            //tmpFile.Write(dataReceived);
            writeToFile(dataReceived);
            if (isReceiving == false)
            {
                tmpFile.Close();
                stockReceived = true;
            }
        }
    }

I hope it's not a problem posting the amended code again, my apologies if it is against site policies

A: 

Ok I am still getting issues with this, I rewrote the code to use threads, but I still get dropped characters, can anyone take a look at my hreading code below and tell me if there is something obvious I am doing wrong as threading is a new subject for me

        public frmSerialComms()
    {
        InitializeComponent();
        recLen = 94;
        sp.Handshake = Handshake.XOnXOff;
        sp.ReceivedBytesThreshold = 1;
        sp.ReadBufferSize = 8192;
        sp.DataReceived += new SerialDataReceivedEventHandler(sp_DataReceived);
        if (sp.IsOpen)
        {
            sp.Close();
        }
        sp.Open();
        readDataStop = false;
        writeDataStop = false;
        thdReadData = new Thread(this.readDataFromPort);
        thdReadData.IsBackground = true;
        thdWriteData = new Thread(this.writeDatatoFile);
        thdWriteData.IsBackground = true;
        areReading.Reset();
        areWriting.Reset();
        thdReadData.Start();
        thdWriteData.Start();

   }

    private void sp_DataReceived(object sender, System.IO.Ports.SerialDataReceivedEventArgs e)
    {
        areReading.Set();
        dataReady = true;
    }

    private void readDataFromPort()
    {
        do
        {
            areReading.WaitOne(500,false);
            if (dataReady)
            {
                if (sp.BytesToRead > 0)
                {
                    lock (lockObject)
                    {
                        receivedBuffer.Append(sp.ReadExisting());
                    }
                    areWriting.Set();
                }
                dataReady = false;
            }
        }
        while (!readDataStop);
    }

    private void writeDatatoFile()
    {
        using (StreamWriter sw = new StreamWriter(logfilename, false, Encoding.ASCII))
        {
            do
            {
                areWriting.WaitOne(500, false);
                if (receivedBuffer.Length > 0)
                {
                    lock (lockObject)
                    {
                        string writeBuffer = receivedBuffer.ToString();
                        receivedBuffer.Remove(0, receivedBuffer.Length);
                        writeBuffer = writeBuffer.Replace("\x02", "");

                        if (writeBuffer.ToString().IndexOf('\x03') != -1)
                        {
                            writeBuffer = writeBuffer.Replace("\x03", "");
                            readDataStop = true;
                            writeDataStop = true;
                            this.Invoke(new invokerDelegate(updateUI), new object[] { "Data file received" });
                        }
                        else
                        {
                            this.Invoke(new invokerDelegate(updateUI), new object[] { "Receiving... " + (dataBytes/recLen).ToString() });
                        }

                        sw.Write(writeBuffer);
                        dataBytes += writeBuffer.Length;
                    }
                }
            }
            while (!writeDataStop);
            sw.Close();
        }
    }
A: 

As serialPort1_DataReceived is a triggered event, it only ever occurs in the main thread. Therefore it cannot be entered by 2 threads simultaneously. To prove it, print out the thread ID of the executing thread (Thread.CurrentThread.ManagedThreadId).

You can stick to spooling the data in the serialPort1_DataReceived call. If you can guarantee a "down-time" in activity in the serial link, spool until you know when that will be then fire off your processing.

E.g. data will come in of up to X bytes/chars. Then another set of chars won't come in for 500ms. You then have the 500ms to process the received bytes.

SamCPP