Class for Serail Port Comms

KeithF

Hi Guys, Just a quick question I have c# code working for communicating with a Serial device using the SerialPort Class (see code below):

private void SendEnq(bool blnIncStep)
{
if (blnIncStep)
{
iStep++;
}

        byte\[\] c = new byte\[\] 
        {
            0x05,
        };

        spBPDione.Write(c, 0, c.Length);
        Console.WriteLine("OUT:  " + ByteToHex(c));
        
    }

    private void DataRecvEvent()
    {
        int bytes = spBPDione.BytesToRead;

        byte\[\] comBuffer = new byte\[bytes\];

        spBPDione.Read(comBuffer, 0, bytes);

        Console.WriteLine("IN:  " + ByteToHex(comBuffer));
        Console.WriteLine("STEP:" + iStep);

        if (ByteToHex(comBuffer).Trim() == "06")
        {
            if (iStep >= 2)
            {
                //All good to here
            }
            else
            {
                byte\[\] c = new byte\[\] {
                   0x02, 0x31, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x31, 0x30, 0x31, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x31,
                   0x30, 0x30, 0x30, 0x30, 0x31, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x31, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 
                   0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x31, 0x20, 0x20, 0x20, 
                   0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x03,
                   0x02 /\*LRC\*/
                };

                spBPDione.Write(c, 0, c.Length);
                Console.WriteLine("OUT:  " + ByteToHex(c));
                iStep++;
            }
        }
        else if (ByteToHex(comBuffer).Trim() == "07")
        {
            byte\[\] c = new byte\[\] {
               0x04,
            };

            spBPDione.Write(c, 0, c.Length);
            Console.WriteLine("OUT:  " + ByteToHex(c));
            iStep++;
        }
        else if (ByteToHex(comBuffer).Trim() == "05")
        {
            if (iStep >= 2)
            {
                byte\[\] c = new byte\[\] {
                    0x06,
                };

                //spBPDione.ReceivedBytesThreshold = 29;

                spBPDione.Write(c, 0, c.Length);
                Console.W

Luc Pattyn

There are many ways to implement a comm protocol, the best one very much depends on the requirements on reliability, robustness and performance. Here are two extremes: 1. A very easy approach would do everything in a synchronous way, which means NOT using events (e.g. DataReceived); e.g. have a separate thread that basically does what your protocol diagram shows, including some explicit delays:

Write ENQ
Wait 100
Read 1 and check for ACK0
Write STX msg ETX LRC
Wait 100
Read 1 and check for ACK1
...

The waits are necessary if you want a good chance of getting the next input all in one read(buf); if you read too early, you might get a partial message. Warning: You have to decide what should happen when something unexpected is read! 2. The general approach would be completely asynchronous, and it would use a state machine, so the DataReceived handler would consist of a big switch on the state variable, and a number of cases each handling one input line of your protocol diagram. Here too you must make sure your diagram is complete, i.e. in each state what should happen if arbitrary stuff is received? And then you should also take care of time-outs: what when no data at all, or an insufficient amount of data, is received? Your app probably doesn't want to wait forever... :)

Luc Pattyn [My Articles] Nil Volentibus Arduum

KeithF

Thanks Luc. Is the If / Else block in my code not already doing what you talk about with the State Machine pattern?

Luc Pattyn

Not really, you are switching/if-ing on the incoming data; a real state machine would switch on its own state (i.e. on what it expects), not on what it gets. What it gets would make it change states. If you are fortunate enough to have a situation where one party is basically asking questions (e.g. all starting with a different byte value) which the other party is to answer in one message, then that isn't really a protocol, and you obviously can switch on the question, as there then is no state whatsoever. :)

Luc Pattyn [My Articles] Nil Volentibus Arduum

KeithF

Hi Luc, Quick question, I have created a class for the message protocol using States I think, i'm just wondering if this is the correct approach:

using System;
using System.Collections.Generic;
using System.Text;
using System.Windows.Forms;
using System.IO.Ports;

namespace TEST
{
public class clsTEST
{
public enum CurrentCommsState
{
Idle,
SentENQ,
RecvdENQ,
SentACK0,
RecvdACK0,
SentACK1,
RecvdACK1,
SentMessage,
RecvdMessage,
SentEOT,
RecvdEOT
}
private enum MessageDirection
{
In,
Out
}

    public CurrentCommsState TheState = new CurrentCommsState();
    private MessageDirection TheDirection = new MessageDirection();
    private SerialPort spTest= null;
    private byte\[\] btRecvdMsg = new byte\[200\];

    public clsOpenShift(SerialPort spTheSP)
    {
        spTEST= spTheSP;
        TheState = CurrentCommsState.Idle;
        TheDirection = MessageDirection.Out;
    }

    public void SendAndRecv(out byte\[\] TheBufferRecvd)
    {
        TheBufferRecvd = new Byte\[200\];

        if (SendMsg())
        {
            if (RecvMsg(ref btRecvdMsg, 1))
            {
                Array.Copy(btRecvdMsg, TheBufferRecvd, 200);
            }
        }
        else if (RecvMsg(ref btRecvdMsg, 1))
        {
            if (SendMsg())
            {

            }
        }
    }

    private bool RecvMsg(ref byte\[\] btOutRecvd, int iBytesToRecv)
    {
        bool blnRetVal = false;

        try
        {
            while (spOpenShiftSP.BytesToRead == 0)
            {
                System.Threading.Thread.Sleep(100);
            }

            byte\[\] theBuffer = new byte\[iBytesToRecv\];
            spTEST.Read(theBuffer, 0, iBytesToRecv);
            ChangeState(theBuffer);
            Array.Copy(theBuffer, btOutRecvd, theBuffer.Length);

            blnRetVal = true;
        }
        catch (Exception e)
        {
            blnRetVal = false;
        }

        return blnRetVal;
    }

    private bool SendMsg()
    {
        bool blnRetVal = false;
        
        try
        {

Luc Pattyn

Hi Keith,

KeithF wrote:

Am I on the right tracks here?

yes, well, maybe. I did not study your code in much detail, I do see a state-machine-like approach, which could be fine; however I have issues in several aspects: 1. I'm not sure I know enough about your "protocol" to judge the approach. As I said, what you need as an implementation heavily depends on your protocol. e.g. what should happen when an unexpected response is received (say you send an ENQ and then receive an ETX where it "should" have been an ACK) 2. I didn't see your actual transmit and receive operations; is your receiver operating asynchronously (i.e. using a completion event)? or blocking (i.e. just read-and-wait-till-done)? 3. You seem to somewhat merge, then split, inbound and outbound messages; not sure why. 4. Whatever the goals and needs, I'm pretty sure my code would look quite differently, but that is a matter of style. In particular I'm not fond of methods that need data, don't get it as a parameter, and then go fetch it somehow (as your SendMsg-MessageToSend pair is doing). Here is what I might consider doing if your protocol is somewhat close to what I think you intend: - I would opt for (pseudo)synchronous transmit and asynchronous receive; - so I would transmit using SP.Write() and act as if the data is gone instantly (if necessary just include a sleep period that corresponds to the normal transmission time, probably not necessary at all); - and I would not read except inside the DataReceived handler. - that would halve the number of states I need, as each state typically would be an "awaiting" state, say "waiting for ACK0"; DataReceived should check the next inbound message; if it is an ACK0, send a new message (synchronously!), adapt the state (say "waiting for "ACK1"), and return; if it isn't, send a new message ("aborting"), set state="IDLE" (=waiting for anything), and return; - so that boils down to a big switch inside DataReceived, switching to the current state (what am I expecting?), then looking at what actually was received, react (probably by sending something) and adapt the state, so the next DataReceived switch will go to another case. That would work for simple cases; it would not cope well with slow transmission conditions: if you want overlaps (comparable to overlapped file I/O in Windows) or if your state actions are too large