Need HELP!! Serial communication has a very "Strange Problem"

Question

I am working on building a serial command receiver. The idea is came from the boot loader. However, I got a very strange problem that I have no idea what could cause it.

As the first screenshot show, the print isn't finished because "===a=" is stuck, like this.

In the code, it should print out and the others.

char* receiveCmd(Stream &_serial, char &cmd_ptr, int &l_ptr) {
  Stream* serial = &_serial;
if (serial->available() && (uint8_t)serialRead(_serial) == CMD_START) {
    int buf_count = 0; //start at data
    cmd_ptr = serialRead(_serial); //cmd_byte
char low_byte = serialRead(_serial);
char high_byte = serialRead(_serial);

uint16_t length = low_byte; //data_length
length = length | (high_byte &lt;&lt; 8);


Serial.println(&quot;\n\n===a===&quot;);
Serial.println(length);
Serial.println(&quot;===b===&quot;);

l_ptr = (int)length;

char* _buf = (char*)malloc(length * sizeof(char));

while (buf_count != length) {
  if (serial-&gt;available()) {
    _buf[buf_count] = serial-&gt;read();
    buf_count++;
  }
}

char checksum = serialRead(_serial); //checksum, don't bother it yett
char stop_byte = serialRead(_serial);

if (stop_byte != CMD_EOF) {
  free(_buf);
  return NULL; //error
}

return _buf;

}
return NULL;
}

Anyway, the problem is fixed when I print out other variables first, then the length var..

char* receiveCmd(Stream &_serial, char &cmd_ptr, int &l_ptr) {
  Stream* serial = &_serial;
if (serial->available() && (uint8_t)serialRead(_serial) == CMD_START) {
    int buf_count = 0; //start at data
    cmd_ptr = serialRead(_serial); //cmd_byte
char low_byte = serialRead(_serial);
char high_byte = serialRead(_serial);

uint16_t length = low_byte; //data_length
length = length | (high_byte &lt;&lt; 8);


Serial.println(&quot;\n\n===a===&quot;);
Serial.println(low_byte, HEX);
Serial.println(high_byte, HEX);
Serial.println(length);
Serial.println(&quot;===b===&quot;);

l_ptr = (int)length;

char* _buf = (char*)malloc(length * sizeof(char));

while (buf_count != length) {
  if (serial-&gt;available()) {
    _buf[buf_count] = serial-&gt;read();
    buf_count++;
  }
}

char checksum = serialRead(_serial); //checksum, don't bother it yett
char stop_byte = serialRead(_serial);

if (stop_byte != CMD_EOF) {
  free(_buf);
  return NULL; //error
}

return _buf;

}
return NULL;
}

Other code is exactly the same. Does anyone know what may cause it? I can't print out those thing during production because it is only for debugging. If I remove those print, the code is also stuck(because of the length variable again).

For additional information, the serial signal is generated from other Arduino and AltSerial Library. The structure is like this.

void sendCmd(Stream &_serial, char cmd, char* payload, int length) {
  Stream* serial = &_serial;
  char buf[6 + length]; //start, data_length, data, checksum, stop
buf[0] = CMD_START; //start_byte
  buf[1] = cmd; //cmd_byte
  buf[2] = length & 0xff; //data_length - low byte
  buf[3] = (length >> 8) & 0xff; //data_length - high byte
  buf[4 + length] = (char)calcCRC(payload); //checksum
  buf[5 + length] = CMD_EOF; //stop_byte
for (int i = 0; i < length; i++) //load buf
    buf[4 + i] = payload[i];
for (int i = 0; i < sizeof(buf); i++)
    serial->print(buf[i]);
}

I have used Serial.print(altSerial.read(), HEX) to check whether the byte is received correctly. They are all correct. That's why it is confusing me.QAQ

===== UPDATED INFORMATION =====

I have narrowed the problem. It seems like the below code is the main cause regard the issue. But no solution for this still.

uint16_t length = low_byte; //data_length
length = length | (high_byte << 8);

Thank you.

score 2 · Accepted Answer · answered Jun 05 '21 at 09:40

Why it does not work? You already got the answer in comments:

You are reading more bytes from the serial stream than what you know are available. Then, your length variable can contain garbage, which makes malloc() fail even if you have plenty of available memory.
You write into the memory buffer even if the allocation failed, which makes your program corrupt its memory. From this point, anything can happen.

You may notice that the behavior of your program depends on the timing. Adding more instructions, or delays, may make it work. This is simply because you give your peer more time to send its data packet. Of course, relying on the timings is not a good approach: you instead have to make sure you never read() more than what is actually available().

Here is an (untested) attempt to do this correctly and in a non-blocking fashion. It is based on a finite state machine. What is original about this state machine is that it is implemented as a switch/case where every single case falls through:

/*
 * Packet format:
 *   CMD_START cmd_byte length_L length_H [payload] checksum CMD_EOF
 */
char *receiveCmd(Stream &serial, char &cmd_ref, int &l_ref) {
// The current state is defined by what parts of the packet
// we have already received.
static enum {
    GOT_NONE, GOT_START, GOT_HEADER, GOT_PAYLOAD
} state = GOT_NONE;

// Variables only valid in the states GOT_HEADER and GOT_PAYLOAD.
static char cmd_byte;
static uint16_t length;
static char *buffer;
static size_t buffer_pos;

switch (state) {
    case GOT_NONE:
        if (serial.available() &lt; 1 || serial.read() != CMD_START)
            break;
        state = GOT_START;
        // fallthrough
    case GOT_START:
        if (serial.available() &lt; 3)
            break;
        cmd_byte = serial.read();
        length = serial.read();
        length |= (uint16_t) serial.read() &lt;&lt; 8;
        buffer = (char *) malloc(length);
        if (!buffer) {
            Serial.print(F(&quot;ERROR: Failed to allocate &quot;));
            Serial.print(length);
            Serial.println(F(&quot; bytes.&quot;));
            state = GOT_NONE;
            break;
        }
        buffer_pos = 0;
        state = GOT_HEADER;
        // fallthrough
    case GOT_HEADER:
        while (buffer_pos &lt; length &amp;&amp; serial.available())
            buffer[buffer_pos++] = serial.read();
        if (buffer_pos &lt; length)
            break;
        state = GOT_PAYLOAD;
        // fallthrough
    case GOT_PAYLOAD:
        if (serial.available() &lt; 2)
            break;
        serial.read();  // consume checksum
        if (serial.read() != CMD_EOF) {
            Serial.println(F(&quot;ERROR: Expected CMD_EOF.&quot;));
            free(buffer);
            state = GOT_NONE;
            break;
        }
        cmd_ref = cmd_byte;
        l_ref = length;
        state = GOT_NONE;
        return buffer;
}
return NULL;

}

You can think of this switch/case as a set of actions to perform sequentially in order to process the whole packet. However, if the function gets stuck for lack of incoming data, it just bails out. On the next call, the switch puts it back where it left.

Need HELP!! Serial communication has a very "Strange Problem"

1 Answers1