Indexer created to look at array doesn't see changes made to array

Question

I am very new to c#, and I'm creating a serial port class for a board I have designed. In which this class contains methods to open/close a serial port connected to the board. It should also read messages from the board and write messages from the UI to the board (I am using a forms application to input and display values).

I read the internal input buffer and place the bytes into my own software buffer, when a message is complete, this will prompt the form to analyse the message...

For this I have created an indexer to point to the array (from the form) and take the bytes that it desires.

    uint[] serialPortReceiveBuffer = new uint[3];
    public delegate void Del();
    Del promptFormAction = Form1.MsgReceived;

    public void serialPort1_DataReceived(object sender, SerialDataReceivedEventArgs e)
    {   
        for (int i = 0; i <= 2; i++) 
        {
            serialPortReceiveBuffer[i] = (uint)serialPort1.ReadByte();
        }

        promptFormAction();  
    }

    public uint this[uint i]
    {
        get { return serialPortReceiveBuffer[i]; }
    }

this is the code within my pcbSerialPort class, and the code related to it in the Form1 class is as follows:

    public static void MsgReceived()
    {
        Form1 _frm = new Form1();
        _frm.analyzeIncomingMessage();
    }

    public void analyzeIncomingMessage()
    {
        if (PCB[0] == 63)
        {
            setBoardDesignator(PCB[1], PCB[2]);
        }
    }

My problem is that the when I use the indexer to access the serialPortReceiveBuffer, it doesn't see the changes that I made to it when placing received bytes into the same array. For example, when I receive a string of my own protocol --> "?10" the buffer is filled with [63][49][48]

Though when I try to access this buffer using the indexer I get [0][0][0]

Please can anyone help? Also, I'm aware there is probably a few other things I could have done better so if you have any general tips that would be great. Also in a language I may understand. I am just getting my head around many of the c# aspects, I have been doing embedded software for the past year but I wouldn't consider my self to be a competent programmer.

Thank you

Answer 1

From your code I'm not quite sure that the PCB object you're working with in your form is actually the one that receives the data. Might well be that you're working with two different instances, especially as you're creating a new instance of Form1 whenever data comes in!

(EDIT: From your comment to the question it is clear that this is exactly the problem. Follow these instructions to get closed to what you want).

I suggest that you redesign your code to pass the received message as an event to the existing form instance instead of how you do it now. Another problem you might run into will be that data you think you get will be overridden by the next message coming in, das the DataReceived event is asynchronous.

I'd declare an event that the form instance can subscribe to, passing the data to be analyzed into the event:

 public class MessageReceivedEventArgs: EventArgs
 {
     public MessageReceivedEventArgs(byte[] data) : base()
     {
         Data = data;
     }

     public byte[] Data
     {
         get;
         private set;
     }
}

public event EventHandler<MessageReceivedEventArgs> MessageReceived;

Then, I'd change your DataReceivedevent as follows:

public void serialPort1_DataReceived(object sender, SerialDataReceivedEventArgs e)
{   
    for (int i = 0; i <= 2; i++) 
    {
        serialPortReceiveBuffer[i] = (uint)serialPort1.ReadByte();
    }

    byte[] dataCopy = new byte[serialPortReceiveBuffer.Length];
    Array.Copy(serialPortReceiveBuffer, dataCopy, dataCopy.Length);

    promptFormAction(dataCopy);  
}

private void promptForAction(byte[] data)
{
    if (MessageReceived != null)
        MessageReceived(this, new MessageReceivedEventArgs(data));
}

Also I'd keep the serialPortReceiveBuffer totally private to that class, as I said, you may run into synchronization issues if you don't. That'y why I copy the array before passing it to the event.

This change allows any subscriber to register for notifications whenever you realize that new data came in.

To use this, Form1 should look like this (roughly);

public class Form1
{
    pcbSerialPort PCB; // The name of that class I don't know from your code

    public Form1()
    {
        PCB = new pcbSerialPort();
        PCB.MessageReceived += MessageReceived;
    } 

    private void MessageReceived(object sender, pcbSerialPort.MessageReceivedEventArgs e)
    {
        analyzeIncomingMessage(e.Data);
    }

    private void analyzeIncomingMessage(byte[] data)
    {
        if (data[0] == 63)
        {
            setBoardDesignator(data[1], data[2]);
        }
    }
}

---

Another piece of advice on how you handle serial data: You need to decide whether you read from a serial port in a loop or whether you rely on the DataReceived event. Putting a loop into the event is not a good idea, as the event may be called upon arriving data again while you're waiting.

What you need to do is create a buffer that takes all the information from the serial port that's available. If you don't have enough data, don't wait for it. Instead add to the buffer whenever DataReceived is called and handle a message when enough data is present.

Answer 2

I think Thorsten's answer is good and it would make a lot of sense to redesign it along those lines, but as an absolute minimum, if you want to have it such that you create a new Form1 instance for every received message, then you will need to pass the instance of pcbSerialPort to MessageReceived and then to the constructor of your Form1 class. Something like:

Action<pcbSerialPort> promptFormAction = Form1.MsgReceived;

public void serialPort1_DataReceived(object sender, SerialDataReceivedEventArgs e)
{   
    // as Thorsten noted, you need to rethink this loop anyway
    // what if there aren't at least three bytes to read?
    for (int i = 0; i <= 2; i++) 
    {
        serialPortReceiveBuffer[i] = (uint)serialPort1.ReadByte();
    }

    promptFormAction(this);  
}

And your static method:

public static void MsgReceived(pcbSerialPort pcb)
{
    Form1 _frm = new Form1(pcb);
    _frm.analyzeIncomingMessage();
}

And you constructor for Form1:

public Form1(pcbSerialPort pcb)
{
    PCB = pcb;
}