Serial-data read buffer handling

Question

This is a follow-up to yesterday's codereview-question about reading serial data and parsing it.

The code below is run into a seperate thread looped endlessly. Currently I track the position in my inputBufer manually and copy all bytes which haven't been processed into a new byte-array and assigning that to the old inputBuffer.

Can the read buffer handling be improved?

bytesRead = SerialPort.Read(inputBuffer, bufferPos, SerialPort.BytesToRead);
bufferPos += bytesRead;
bytesProcessed = 0;
// process data-packets
for (int i = 0; i < this.bufferPos; i++) {
 // Flex sensor data
 //
 if (this.bufferPos-i >= 14 && inputBuffer[i] == 'F'){
 i++;
 short[] sensorData = new short[6];
 sensorData[0] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // thumb
 sensorData[1] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // index finger (finger knuckle)
 sensorData[2] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // middle finger
 sensorData[3] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // ring finger
 sensorData[4] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // not used
 sensorData[5] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // index finger (hand knucke)
 // checksum
 byte checksum = inputBuffer[i++];
 ParseSerialData(PacketType.Flex, sensorData, checksum);
 bytesProcessed = i;
 //Debug.Log (sensorData[0] + " then " + bytesProcessed);
 }
 // Raw sensor data
 //
 if (this.bufferPos-i >= 20 && inputBuffer[i] == 'S'){
 i++;
 short[] sensorData = new short[9];
 sensorData[0] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // gyroscope X
 sensorData[1] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // gyroscope Y
 sensorData[2] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // gyroscope Z
 sensorData[3] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // accelerometer X
 sensorData[4] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // accelerometer Y
 sensorData[5] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // accelerometer Z
 sensorData[6] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // magnetometer X
 sensorData[7] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // magnetometer Y
 sensorData[8] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // magnetometer Z
 // checksum
 byte checksum = inputBuffer[i++];
 ParseSerialData(PacketType.Raw, sensorData, checksum);
 bytesProcessed = i;
 }
 // Quaternion of device's rotation
 //
 if (this.bufferPos-i >= 20 && inputBuffer[i] == 'Q'){
 i++;
 short[] sensorData = new short[4];
 sensorData[0] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // W
 sensorData[1] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // X
 sensorData[2] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // Y
 sensorData[3] = (short)((short)inputBuffer[i++] << 8 | inputBuffer[i++]); // Z
 // checksum
 byte checksum = inputBuffer[i++];
 ParseSerialData(PacketType.Quaternion, sensorData, checksum);
 bytesProcessed = i;
 }
}
// Copy unprocessed rest of stream
int rest = bufferPos - bytesProcessed;
 if (rest > 0){
 byte[] tempBuffer = new byte[4096];
 Array.Copy (inputBuffer, bytesProcessed, tempBuffer, 0, rest);
 inputBuffer = tempBuffer;
 bufferPos = rest;
}
else {
 inputBuffer = new byte[4096];
 bufferPos = 0;
}

Answer 1

Another suggestion, to go on top of Heslacher's:

Make the (short)((short)inputBuffer[... a method.

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static short BytesToShort(byte a, byte b)
{
 return (short)(a << 8 | b);
}

The MethodImplOptions.AggressiveInlining enumeration value tells the C# compiler to try to inline the method as much as possible. That is, if you come form a C/C++ environment, it is very similar to macro's.

The C++ macro:

#define LERP(a,b,c) (((b) - (a)) * (c) + (a))

In C# could be written as:

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static double Lerp(double a, double b, double c)
{
 return (b - a) * c + a;
}

And effectively be the same thing. The compiler would try to inline the method body instead of the method whenever possible.

This should help performance, while allowing you to move some of the more mundane code away from the main area you would be working.

Note: this does not guarantee that this is the choice the compiler would make, nor does it preclude you from doing other optimization strategies. However, for a method this small and simple, it would likely make that choice in the end.

https://msdn.microsoft.com/en-us/library/system.runtime.compilerservices.methodimploptions(v=vs.110).aspx

This means, that you could essentially do:

for (int i = 0; i < sensorData.Length; i++)
{
 sensorData[i] = BytesToShort(inputBuffer[startposition++], inputBuffer[startPosition++]);
}

With no ill-effects.

Answer 2

By extracting the retrieval of the different sensordata to separate methods this whole code block would be more readable and easier to maintain.

By using a for loop for reading the single values the whole reading of the sensor data can be done inside a single method.

By replacing the big if statements like so

int? numberOfSensorData;
int minimumBufferPosition = 0;
PacketType packetType;
switch (inputBuffer[i])
{
 case 'F':
 packetType = PacketType.Flex;
 numberOfSensorData = 6;
 minimumBufferPosition = 14;
 break;
 case 'S':
 packetType = PacketType.Raw;
 numberOfSensorData = 9;
 minimumBufferPosition = 20;
 break;
 case 'Q':
 packetType = PacketType.Quaternion;
 numberOfSensorData = 4;
 minimumBufferPosition = 20;
 break;
}
if(!numberOfBytesToProcess.HasValue || this.bufferPos - i < minimumBufferPosition) { continue; }
bytesProcessed = i = ProcessSensorData(inputBuffer, i++, numberOfSensorData, packetType); 

and the extracted methods like so

private int ProcessSensorData(byte[] inputBuffer, int startPosition, int numberOfSensorData, PacketType packetType)
{
 byte checksum;
 short[] sensorData = new short[numberOfSensorData];
 startPosition = ReadSensorData(inputBuffer, sensorData, startPosition, out checkSum);
 ParseSerialData(packetType, sensorData, checksum);
 return startPosition;
}
private int ReadSensorData(byte[] inputBuffer, short[] sensorData, int startPosition, out short checksum)
{
 for(int i = 0; i < sensorData.Length; i++)
 {
 sensorData[i] = (short)((short)inputBuffer[startPosition++] << 8 | inputBuffer[startPosition++]);
 }
 checksum = inputBuffer[startPosition++];
 return startPosition;
}