Serial Port Best Practice Code Design Help

Archive
1

My application involves sending a command to a Cerberus over the serial port and the Cerberus responding back with the results. Pretty simple, really. I don’t expect the Cerberus to do anything that it isn’t asked to do. It only takes commands and responds to them.

I poured over every “serial” post in these forums and tried to avoid data overruns and other issues. These things popped up a lot. So I did a lot of experimentation. The real problem seems to be writing to the port to send the data. One forum post pointed out that the transmit buffer was only 256 bytes, so that would make sense.

So I wrote my code (based on RoSchmi’s great contributions - thanks!) and it’s shown below.

I understand that it would be a good idea to have the receiving function of the serial port running in a separate thread. That thread would raise events when it had received complete command strings. The main thread would respond to those events by doing the requested processing. So far so good.

Then the main thread would raise an event and a 2nd thread, a sending thread, would deliver the results to the serial port.

I tried delegates, events and every other trick I’d seen or heard of to get that to work and I never could. I don’t have the C# skills/experience.

So what resulted is the code below. I ran 500,000 iterations through it with no data loss and ended up getting about 153 responses per second at 38400. Even adding back in the processing of the requests and it’ll be more than fast enough.

But I worry about the architecture. Is this the right approach and/or is this code going to cause me trouble? Every bone in my body says I’m not doing it in a “best practices” way, but it works.

Thanks in advance, guys. I’ve only gotten this far because of the generosity of forum posters.

using System;
using System.Collections;
using System.Threading;
using Microsoft.SPOT;
using Microsoft.SPOT.Presentation;
using Microsoft.SPOT.Presentation.Controls;
using Microsoft.SPOT.Presentation.Media;
using Microsoft.SPOT.Presentation.Shapes;
using Microsoft.SPOT.Touch;
using Gadgeteer.Networking;
using GT = Gadgeteer;
using GTM = Gadgeteer.Modules;
using Gadgeteer.Modules.GHIElectronics;
using Serial = Gadgeteer.Interfaces.Serial;
using System.Text;

namespace CerberusThroughputTest2 {

    public partial class Program {
        Thread receivingThread;
        int counter = 0;
        int requestResponseCounter = 0;

        void ProgramStarted()
            {
            Debug.Print("Program Started");
            GT.Timer SendMessageTimer = new GT.Timer(1000, GT.Timer.BehaviorType.RunContinuously);
            SendMessageTimer.Tick += SendMessage_Tick;

            SetupSerialPort();
            SetupReceivingThread();
            SendMessageTimer.Start();
            }

        void SetupSerialPort()
            {
            try
                {
                this.rs232.Initialize(38400, 
                    Serial.SerialParity.None, 
                    Serial.SerialStopBits.One, 
                    8, 
                    Serial.HardwareFlowControl.NotRequired);
                }
            catch (Exception e)
                {
                Debug.Print(e.ToString());
                }
            if (!rs232.serialPort.IsOpen)
                {
                rs232.serialPort.Open();
                }
            }

        void SetupReceivingThread()
            {
            SerialReader serialReaderTarget = new SerialReader(rs232);
            serialReaderTarget.CommandReceived += CommandReceived;
            ThreadStart serialReaderThreadStart = new ThreadStart(serialReaderTarget.Start);
            receivingThread = new Thread(serialReaderThreadStart);
            receivingThread.Start();
            }

        void CommandReceived(string data)
            {
            requestResponseCounter++;
            ProcessingSingleCommand(data);
            }

        void SendMessage_Tick(GT.Timer timer)
            {
            counter++;
            char_Display.SetCursor(0, 0);
            char_Display.PrintString(counter.ToString());
            }

        void ProcessingSingleCommand(string command)
            {
            SendLine("OK12345678");
            }

        void SendLine(string response)
            {
            rs232.serialPort.WriteLine(response);
            while (rs232.serialPort.BytesToWrite > 0)
                {
                Thread.Sleep(5);
                }
            Debug.Print("Sent this line: " + response);
            }
        }
    }

And this is the serial class:

using System;
using Microsoft.SPOT;
using GTM = Gadgeteer.Modules;
using GT = Gadgeteer;

using Serial = Gadgeteer.Interfaces.Serial;

public delegate void SendLineHandler(string data);

namespace CerberusThroughputTest2 {
    public class SerialReader {
        static GTM.GHIElectronics.RS232 rs232;
        SerialBuffer mySerialBuffer;
        string dataLine = string.Empty;
        public event SendLineHandler CommandReceived;

        public SerialReader(GTM.GHIElectronics.RS232 rs232p)
            {
            rs232 = rs232p;
            mySerialBuffer = new SerialBuffer(256);
            }

        public void Start()
            {
            rs232.serialPort.DataReceived += serialPort_DataReceived;
            }

        public void Stop()
            {
            rs232.serialPort.DataReceived -= serialPort_DataReceived;
            }

        void serialPort_DataReceived(GT.Interfaces.Serial sender, System.IO.Ports.SerialData data)
            {
            var toRead = sender.BytesToRead;
            byte[] v = new byte[toRead];
            sender.Read(v, 0, toRead);
            mySerialBuffer.LoadSerial(v, 0, v.Length);
            while ((dataLine = mySerialBuffer.ReadLine()) != null)
                {
                CommandReceived(dataLine);
                }
            }
        }
    }
2

Hi willy,
you did not show the code of the class serialBuffer. I think you should add it to your post. Without this classs your code cannot be understood by others.

As you already mentioned, I think the problem with fast communication is mostly buffer overflow. Hardware flow control RTS/CTS is a measure that prevents the Sender from sending when the receiver buffer is full. Additionally the sender must have a means to know whether there is enough free space in the sender buffer to hold the next chunk of data which the program wants to write to it.
Another way to achieve lossless communications is principally that the receiver sends a receipt and the sender does not send new data until it got the receipt.
A third, but unreliable, way is to send slow and to hope that the transmission and the receiver works fast enough to manage the incoming data without loss.
Cheers
Roland

3

Ah!

I forgot to add your serial class! Here it is:

// Code of this Class is taken from:
// TinyCLR CodeShare, author: andrejk "SerialBuffer: simple, fast way to read serial data"
// Modifications by RoSchmi

using System;
using Microsoft.SPOT;

namespace CerberusThroughputTest2
{
    class SerialBuffer
    {
        private System.Text.Decoder decoder = System.Text.UTF8Encoding.UTF8.GetDecoder();
        private byte[] buffer;
        private int startIndex = 0;
        private int endIndex = 0;
        private char[] charBuffer;

        public SerialBuffer(int initialSize)
        {
            buffer = new byte[initialSize];
            charBuffer = new char[256];
        }

        public void LoadSerial(byte[] data, int startIndex, int length)
        {
            if (buffer.Length < endIndex + length) // do we have enough buffer to hold this read?
            {
                // if not, look and see if we have enough free space at the front
                if (buffer.Length - DataSize >= length)
                {
                    ShiftBuffer();
                }
                else
                {
                    // not enough room, we'll have to make a bigger buffer
                    ExpandBuffer(DataSize + length);
                }
            }
            //Debug.Print("serial buffer load " + bytesToRead + " bytes read, " + DataSize + " buffer bytes before read");
            //Array.Copy(data, 0, buffer, endIndex, length);
            Array.Copy(data, startIndex, buffer, endIndex, length);
            endIndex += length;
        }

        private void ShiftBuffer()
        {
            // move the data to the left, reclaiming space from the data already read out
            Array.Copy(buffer, startIndex, buffer, 0, DataSize);
            endIndex = DataSize;
            startIndex = 0;
        }

        private void ExpandBuffer(int newSize)
        {
            byte[] newBuffer = new byte[newSize];
            Array.Copy(buffer, startIndex, newBuffer, 0, DataSize);
            buffer = newBuffer;
            endIndex = DataSize;
            startIndex = 0;
        }

        public byte[] Buffer
        {
            get
            {
                return buffer;
            }
        }

        public int DataSize
        {
            get
            {
                return endIndex - startIndex;
            }
        }

        public string ReadLine()
        {
            lock (buffer)
            {
                int lineEndPos = Array.IndexOf(buffer, '\n', startIndex, DataSize);  // HACK: not looking for \r, just assuming that they'll come together        
                if (lineEndPos > 0)
                {
                    int lineLength = lineEndPos - startIndex;
                    if (charBuffer.Length < lineLength)  // do we have enough space in our char buffer?
                    {
                        charBuffer = new char[lineLength];
                    }
                    int bytesUsed, charsUsed;
                    bool completed;
                    decoder.Convert(buffer, startIndex, lineLength, charBuffer, 0, lineLength, true, out bytesUsed, out charsUsed, out completed);
                    string line = new string(charBuffer, 0, lineLength);
                    startIndex = lineEndPos + 1;
                    //Debug.Print("found string length " + lineLength + "; new buffer = " + startIndex + " to " + endIndex);
                    return line;
                }
                else
                {
                    return null;
                }
            }
        }

    }
}
4

RoSchmi,

Thanks for checking this out. I have basically restructured my protocol so that the host sends the command and waits for a response from the Cerberus. I deal with any timeout situation on the host end.

The benefit is that both the request from the host and the response from the Cerberus are pretty small and neither one does any work until the other says it’s OK. That pretty much assures that the sending buffer will be empty before it can overflow, and before the host can ask for anything more.

I worked with CTS/RTS in the late 70’s when monitoring CD was still really important and vendor approved serial cables were $200. :open_mouth: While that’s the best way, hardware flow control may not always be available to me.

My question was more about the way the threads work, with the receiver in it’s own thread and the sender running in the main thread, where other work will be done. Seems not very clean somehow.

I’m not 100% sure why even threading the receiving end is necessary now that I have defined the ack/nack protocol the way I have.

I would rather have the protocol be reliable rather than depend on slow speeds. :slight_smile:

Right now, it’s working fine, even at 115200 with no hardware flow control. I did run at test at 500,000 iterations at 38400 and it worked fine then, too.

I know there has to be some cooler way to do it, with delegates and events or something like that, something more elegant.

If no one has any suggestions, though, I guess I’ll leave it as is.

Thanks again for the work you did that I was able to include.

If you ever get to the Midwest, I’ll buy you beer.

BTW, your SerialBuffer class is unmodified.

5

Only to make it clear, the nice serialBuffer class is not from me:
https://www.ghielectronics.com/community/codeshare/entry/91

6

Whoops! Rookie mistake. I first found it in your Bluetooth contribution and missed that it was from andrejk.

So, thanks to andrejk and you. :slight_smile:

7

@ willy - as one newbie to another, I can appreciate your situation and I have the same question about what an effective architecture might be. However, I’m leaning toward staying away from extra threads, delegates or any other cool stuff that isn’t absolutely necessary. Oddly enough, I have a similar thread going over at https://www.ghielectronics.com/community/forum/topic?id=14920 . If you’re interested, maybe you could take a look at my last post with the code I’m using that seems to work and let me know what you think.

Cheers - Gene

8

Gene, thanks.

In a desktop environment, so much changes so quickly that uncoupling the parts of an app adds real value.

But here, everything seems to be so focused and dependent on the sensors or whatever that while loose coupling still has value, I guess I don’t need to be so obsessive about it.

Thanks for the link to the other thread.

In my past experience, not handling all communications on separate threads usually led to lost/corrupted data and headaches. This dates back to try to run 2400 baud over dial up with no error checking.

Pre internet. Pre error checking. Pre Zmodem. Pre dirt.

At one point, my company actually licensed the MNP protocol from Microcom so we could mimic in Turbo Pascal and Assembler what they were doing in their modems, which were the first error corrected modems in the market.