Read and write data to/from Serial Port in C#

Question

I am building a console application which will be scheduled in Task Scheduler of Windows to run my code every day at a set hour. Summing up, this application will read and write through serial port. When I send something to the Arduino, I need to receive something from it to finish up what I've sent and execute the command.

In other words, I will send something so see if the door is opened, if it is the application will run a code to close it. If the door is already closed I will send a bunch of characters to be displayed into the Arduino Led Display.

So I've developed a code but I am not sure if it's totally correct, if possible help me improve it. There is any changes I could make?

static void Main(string[] args)
{
    SerialPort comport = new SerialPort("COM1", 9600, Parity.None, 8, StopBits.One);
    comport.Open();

    string start = "?";
    string carriageReturn = "\r";
    string text = string.Empty;

    string mensage = "@" + "r" + "\r";
    string mensage2 = "@" + "{" + texto + "\r";

        try
            {
                while (true)
                {
                    //Send to the Arduino
                    comport.Write(start + "*" + carriageReturn);

                    //If the serial port have bytes to read
                    if (comport.BytesToRead > 0)
                    { 
                        //Buffer with data
                        byte[] data = HexStringToByteArray(mensage);

                        //Handle data
                        comport.Read(data, 0, data.Length);

                        //Send again to execute the the command
                        comport.Write(start + "*" + carriageReturn);
                    }

                    if (comport.BytesToRead > 0)
                    { 
                        comport.Write(start + "*" + carriageReturn);

                        byte[] data2 = HexStringToByteArray(mensage2);

                        comport.Read(data2, 0, data2.Length);

                        comport.Write(text);
                    }
                    comport.Close();
                }
            }

        catch(Exception ex)
        {
        }
}

private static byte[] HexStringToByteArray(string s)
{
    s = s.Replace(" ", "");
    byte[] buffer = new byte[s.Length / 2];

    for (int i = 0; i < s.Length; i += 2)
        buffer[i / 2] = (byte)Convert.ToByte(s.Substring(i, 2), 16);
        return buffer;
}

Answer 1

There are a number of strange things going on here...

First, you have this:

//Buffer with data
byte[] data = HexStringToByteArray(mensage);

//Handle data
comport.Read(data, 0, data.Length);

The first line takes the mensage (message?) and turns it into a byte array, but then you immediately overwrite the data by reading into the same buffer. In essence, you are using HexStringToByteArray simply to create a new array of some length (3 bytes?).

Based on how you are using HexStringToByteArray, it certainly does not take a hex string and turn it into a byte array. mensage and mensage2 contain things other than valid hex characters. Maybe something more descriptive like StringToByteArray or StringToHexByteArray would be better. It does look like you are trying to convert a "hex" string input to byte data, but given the inputs I can't see how you can run this and not get an exception. The other problem with this method is that it assumes you have an input string length divisible by 2, and you are not checking that (nor are your values passed in adhering to that).

Since you are not doing anything with the data and data2 arrays, you are basically using them to flush some data out of the read buffer. I would have to assume that the code you posted is not complete, otherwise you could do something like SerialPort.Flush() instead.

Now, lets look at your variables:

string start = "?";
string carriageReturn = "\r";
string text = string.Empty;

string mensage = "@" + "r" + "\r";
string mensage2 = "@" + "{" + texto + "\r";

I would advise against using the carriageReturn and either call it something like messageDelimiter or just don't use it. It would be akin to defining something like:

int zero = 0;

Zero will always equal zero, so you shouldn't have variables for them. The name text doesn't really help here, you should give it a descriptive name like emptyMessage since it doesn't look like it ever changes. mensage and mensage2 are also not descriptive (or spelled correctly), maybe something like doorOpenQuery and doorCloseCommand. (And in addition, since texto isn't defined, I don't see this even compiling...)

The last problem I see is that you have an un-throttled while(true) loop in your program with no exit conditions other than something throwing an exception. How many times do you want to run this loop? You shouldn't have an unbounded while, you are going to have a not-responding program if you let it run for about 10 seconds.

From what I see I don't believe that this is functioning code. Have you run this through the debugger? If you have functionality problems this probably isn't the StackExchange site for that.