Weird issue with Serial.readBytes()

So I have a program that is supposed to be connected to my PC and allow controlling a couple of Adafruit_NeoPixel strips.
I am using Serial for communication between the Arduino Mega 2560 and my PC.

There is a problem with one of the Serial.readBytes() where only one of the commands wont work at all.

PS. I am using 500'000 baudrate. But I tested with 115'200 baudrate aswell as 9'600. With the same results so there isn't packet loss.

Arduino Command Recieve Code:

else if (opCode == (unsigned char)ControlCodes::CREATE_LED_AREA)	// Does not work
{
	unsigned char* buffer = (unsigned char*)malloc(5);		// Create a 5 byte size buffer
	unsigned int bytesRead = GetData(buffer, 5);			// Get the data from the Serial port
	if (bytesRead == 5)						// Check if bytes read is equal to the buffer size
	{
		unsigned short startLed = buffer[0] << 8 | buffer[1];	// Construct a short (2 byte) value from the first 2 bytes recieved
		unsigned short endLed = buffer[2] << 8 | buffer[3];	// Construct a short (2 byte) value from the next 2 bytes recieved
		unsigned char mode = buffer[4];
		CreateLEDArea(startLed, endLed, mode);			// Create LED Area using the newly constructed values
		WriteData((unsigned char)ControlResults::OK);		// Send back a OK result
	}
	else
	{
		WriteData((unsigned char)ControlResults::NOT_OK);	// Send back a NOT_OK result
	}
}
else if (opCode == (unsigned char)ControlCodes::DELETE_LED_AREA)	// Works
{
	unsigned char* buffer = (unsigned char*)malloc(2);		// Create a 2 byte size buffer
	unsigned int bytesRead = GetData(buffer, 2);			// Get the data from the Serial port
	if (bytesRead == 2)						// Check if bytes read is equal to the buffer size
	{
		unsigned short index = buffer[0] << 8 | buffer[1];	// Construct a short (2 byte) value from the bytes recieved
		RemoveLEDArea(index);					// Remove LED Area using the newly constructed short
		WriteData((unsigned char)ControlResults::OK);		// Send back a OK result
	}
	else
	{
		WriteData((unsigned char)ControlResults::NOT_OK);	// Send back a NOT_OK result
	}
}

Arduino GetData function:

unsigned int ArduinoLEDController::GetData(unsigned char* buffer, unsigned int length)
{
	return (unsigned int)Serial.readBytes(buffer, length);		// Read %length% bytes into the buffer and return the number of bytes actually read.
}

C++17 with Common Language Runtime enabled in Visual Studio 2017.
Code for ControlCodes::CREATE_LED_AREA command:

unsigned char CreateLEDArea(SerialPort^ port, unsigned short startLed, unsigned short endLed, unsigned char mode)
{
	port->DiscardInBuffer();						// Clear read buffer so we only read for this command
	array<unsigned char>^ data = gcnew array<unsigned char>(6);		// Create a new 'managed' unsigned char array with 6 elements
	data[0] = (unsigned char)RGBAll::ControlCodes::CREATE_LED_AREA;		// Set '0' to the command CREATE_LED_AREA
	data[1] = (unsigned char)(startLed >> 8 & 0xFF);			// Set '1' to the upper 8 bits of startLed
	data[2] = (unsigned char)(startLed & 0xFF);				// Set '2' to the lower 8 bits of startLed
	data[3] = (unsigned char)(endLed >> 8 & 0xFF);				// Set '3' to the upper 8 bits of endLed
	data[4] = (unsigned char)(endLed & 0xFF);				// Set '4' to the lower 8 bits of endLed
	data[5] = mode;								// Set '5' to mode
	port->Write(data, 0, 6);						// Send the array to the Arduino
	unsigned char result = (unsigned char)RGBAll::ControlResults::NOT_OK;	// Initialize a result as NOT_OK
	try
	{
		result = port->ReadByte();					// Try to read the result
	}
	catch (System::TimeoutException^)
	{
		std::wcout << "ReadByte() timed out" << std::endl;		// Read byte timed out
	}
	return result;								// Return result
}

Code for ControlCodes::DELETE_LED_AREA command:

unsigned char DeleteLEDArea(SerialPort^ port, unsigned short index)
{
	port->DiscardInBuffer();						// Clear read buffer so we only read ffor this command
	array<unsigned char>^ data = gcnew array<unsigned char>(3);		// Create a new 'managed' unsigned char array with 3 elements
	data[0] = (unsigned char)RGBAll::ControlCodes::DELETE_LED_AREA;		// Set '0' to the command DELETE_LED_AREA
	data[1] = (unsigned char)(index >> 8 & 0xFF);				// Set '1' to the upper 8 bits of index
	data[2] = (unsigned char)(index & 0xFF);				// Set '2' to the lower 8 bits of index
	port->Write(data, 0, 3);						// Send the array to the Arduino
	unsigned char result = (unsigned char)RGBAll::ControlResults::NOT_OK;	// Initialize a result as NOT_OK
	try
	{
		result = (unsigned char)port->ReadByte();			// Try to read the result
	}
	catch (System::TimeoutException^)
	{
		std::wcout << "ReadByte() timed out" << std::endl;		// Read byte timed out
	}
	return result;															// Return result
}

When i run the code and try to use the function like this "CreateLEDArea(port, 0, 50, 0)"
it returns 'NOT_OK'.
When i run the other function like this "DeleteLEDArea(port, 0)"
it returns 'OK'.

So there seems to be a weird thing about that one case.

Why are you sending 6 bytes of data but only read 5?

Oh, the first byte is the command code which I am reading before the if statements

Here is some partial code from the start of the Serial handling:
It gets the command code with "GetOpCode();" which is basically just "Serial.read();"
Then it checks if that command code is the same as some predefined codes.

if (HasIncomingTraffic())
{
 unsigned char opCode = GetOpCode();
 if (opCode == (unsigned char)ControlCodes::IS_CONTROLLABLE)

Do you have a problem on the PC side or the arduino side?

I actually can't figure out if the SerialPort from .Net actually sends the data or if the Arduino just won't read it

Then, take a step back, send and receive 1 byte at a time. Repeat until you find the problem..

Ok I tested sending one extra byte each time, and as soon as I send three bytes it fails.
Though I noticed that it was slowed down, so the Arduino tried to read the data but couldn't read all bytes before it timed out.

Which side failed?

It was on the Arduino's side, probably a memory collide, though that shouldn't have been the issue.

Post your complete arduino program.
Also, you malloc-ed memory but you never free them.

Also, you malloc-ed memory but you never free them.

If I am not totally off with C++11 the memory will get freed as soon as the scope is closed.
But I am not a C++ master.

Just tested something,
so earlier I had the serial handling function called from within an
Update function

void LEDController::Update(float currentTime)
{
 HandleIncomingTraffic();     <----
 for (unsigned int areaIndex = 0; areaIndex < numLEDAreas; areaIndex++)
 {
 ledAreas[areaIndex]->Update(currentTime);
 }
 UpdateNative();
}

And I just moved it out into the loop function

void loop()
{
 controller->HandleIncomingTraffic(); <----
}

and it has no data loss which seems to have been the issue...

Only for automatic variables. The pointers go out of scope, but the memory you allocated is still there. Any time you have a malloc you must have a free to go with it.

Oh. Yeah I am not a C++ wizard.

Also I changed the buffer to a pre-allocated buffer that is 128 bytes large, should be max size I will ever use. (Already requiring 128 bytes for a command TEST_CONNECTION which tests the connection to the Arduino, basically by checking how many of the received bytes are a predefined value and then returning how good the connection is between 0 and 255 (8 bits))

Found out what was causing the issues, it is from the Adafruit_NeoPixel show function. It makes the Arduino turn off interupts while it is running making the Arduino get data loss, so I will make a command that runs the show function only when it is given.

Found out what was causing the issues, it is from the Adafruit_NeoPixel show function. It makes the Arduino turn off interupts while it is running making the Arduino get data loss

It's a known 'problem' with Neopixels; the timing is quite critical.

The solution is to send whatever you want to send one byte at a time from the PC and let the Arduino echo it back. Only when the PC receives the echo, it will send the next byte.