snippetcsharpModerate

Read binary serial data and parse integers

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

readparseserialbinaryandintegersdata

Problem

I'm reading serial data in binary format from a stream, and I'm not happy with how I extract the data because it takes so much operations to extract the binary data from a simple line of binary data.

Each line starts with a character indicating the type of data, and afterwards follow a few 16 bit integers (big endian), followed by a checksum character and a newline.

Here's a sample of what line would be after reading:

line = "F{3x 16 bit int big endian}{checksum character}\n"

This is the simplified code in question:

byte[] b = new byte[2];
  System.Text.ASCIIEncoding enc = new System.Text.ASCIIEncoding();

  // Check that the port is open. If not skip and do nothing
    if (SerialPort.IsOpen)
    {
        while(SerialPort.BytesToRead > 50){

            // read line and turn string in byte array
            string line = SerialPort.ReadLine();
            byte[] encoded = enc.GetBytes(line);

            if (line[0]=='F'){

                // copy 16 bit int into byte-array
                Array.Copy(encoded, 1, b, 0, 2);
                // -> big endian
                Array.Reverse(b);

                // turn into integer
                short value1 = BitConverter.ToInt16(b, 0);

                Array.Copy(encoded, 3, b, 0, 2);
                Array.Reverse(b);
                short value1 = BitConverter.ToInt16(b, 0);

                Array.Copy(encoded, 5, b, 0, 2);
                Array.Reverse(b);
                short value3 = BitConverter.ToInt16(b, 0);
            }

        }
        this.bytesAvailable = SerialPort.BytesToRead;

Is there a smarter way to accomplish the result of my code above?

Solution

Data Acquisition

You asked:

Is there a smarter way to accomplish the result of my code above?

So, I have another suggestion, to work on top of the suggestions by Heslacher:

Eliminate the BitConverter work. This is easily done with boolean operators (most specifically the shift operators).

short value1 = (short)((short)encoded[1] << 8 | encoded[2]);
short value2 = (short)((short)encoded[3] << 8 | encoded[4]);
short value3 = (short)((short)encoded[5] << 8 | encoded[6]);

Why? It's fast as hell. And this tends to make it more clear what position each value represents. There's no need to place them backwards as you have to with the BitConverter. Looking at this code, it's extremely easy to determine that each pair consists of the high-byte, followed by the low-byte. With the BitConverter, the lowest byte must be followed by the subsequently higher bytes.

Can it really be faster, with all those casts, shifting ( 50){ line describing why we only want it if there are more than 50 bytes left to read. From your current code it's impossible to reason why.

Comments are very powerful; and though I am very liberal with my comments (placing one every 2-10 lines), you don't have to be. But they should be often enough that you don't have to examine a 200+ line file to get the relevant bits. Add minor information on where these values are used, what are they for, if you are using indices like this, then markup what each index represents. You don't have to go ape-s*** on them. Just describe what it is you are doing, and why.

Code Snippets

short value1 = (short)((short)encoded[1] << 8 | encoded[2]);
short value2 = (short)((short)encoded[3] << 8 | encoded[4]);
short value3 = (short)((short)encoded[5] << 8 | encoded[6]);

byte[] encoded = new byte[7];
encoded[0] = 0xF0;
encoded[1] = 0xF1;
encoded[2] = 0xF2;
encoded[3] = 0xF3;
encoded[4] = 0xF4;
encoded[5] = 0xF5;
encoded[6] = 0xF6;

Stopwatch sw1 = new Stopwatch();

short value1 = 0, value2 = 0, value3 = 0;

sw1.Start();
for (int i = 0; i < 100000000; i++)
{
    value1 = BitConverter.ToInt16(new byte[] { encoded[2], encoded[1] }, 0);
    value2 = BitConverter.ToInt16(new byte[] { encoded[4], encoded[3] }, 0);
    value3 = BitConverter.ToInt16(new byte[] { encoded[6], encoded[5] }, 0);
}
sw1.Stop();
Console.WriteLine(sw1.ElapsedTicks.ToString());
sw1.Reset();

Console.WriteLine(value1.ToString("X"));
Console.WriteLine(value2.ToString("X"));
Console.WriteLine(value3.ToString("X"));

sw1.Start();
for (int i = 0; i < 100000000; i++)
{
    value1 = (short)((short)encoded[1] << 8 | encoded[2]);
    value2 = (short)((short)encoded[3] << 8 | encoded[4]);
    value3 = (short)((short)encoded[5] << 8 | encoded[6]);
}
sw1.Stop();
Console.WriteLine(sw1.ElapsedTicks.ToString());
sw1.Reset();

Console.WriteLine(value1.ToString("X"));
Console.WriteLine(value2.ToString("X"));
Console.WriteLine(value3.ToString("X"));

12956403
F1F2
F3F4
F5F6
1505318
F1F2
F3F4
F5F6

short[] myValues = new short[3];

sw1.Start();
for (int i = 0; i < 100000000; i++)
{
    for (int t = 0; t < myValues.Length; t++)
        myValues[t] = (short)((short)encoded[1 + t * 2] << 8 | encoded[2 + t * 2]);
}
sw1.Stop();
Console.WriteLine(sw1.ElapsedTicks.ToString());
sw1.Reset();

for (int t = 0; t < myValues.Length; t++)
    Console.WriteLine(myValues[t].ToString("X"));

3759655
F1F2
F3F4
F5F6

Context

StackExchange Code Review Q#95983, answer score: 15

Revisions (0)

No revisions yet.