Fix UART overflow

Question

i am having a GSM module talking with my microcontroller throught Hardware UART. When I receive small data everything works. When I try to get a list of all the SMS messages stored in the SIM card I get UART buffer overflow error.

I have a dynamic string function that appends every received character in a buffer that uses malloc. This is how I receive. I commented out the line causing problems.

while( uart_available() > 0 )
    {
        /*
         *  There are characters in the buffer so
         *  keep receiving until buffer in empty.
         */
        unsigned int c = uart_getc();

        if ( c & UART_NO_DATA )
        {
            /*
             * No data available from UART. This is not an error.
             */
            error( AT, "No error but - No uart data" );// TODO: Delete it. It is not an error actually.
        }
        else if ( c & UART_FRAME_ERROR )
        {
            /* Framing Error detected, i.e no stop bit detected */
            errorOccured( ER_UART_FRAME_ERROR );
            error( AT, "Frame error" );
        }
        else if ( c & UART_OVERRUN_ERROR )
        {
            /* 
             * Overrun, a character already present in the UART UDR register was 
             * not read by the interrupt handler before the next character arrived,
             * one or more received characters have been dropped.
             */
            errorOccured( ER_UART_OVERRUN_ERROR );
            error( AT, "Overrun error" );
        }
        else if ( c & UART_BUFFER_OVERFLOW )
        {
            /* 
             * We are not reading the receive buffer fast enough,
             * one or more received characters have been dropped.
             */
            errorOccured( ER_UART_BUFFER_OVERFLOW );
            error( AT, "Buffer overflow error" );
        }
        else
        {
            /*
             * There are data available add them to cString.
             */
            appendChar( data, (const char)c );// This line is slow and causes UART overflow.
            if ( *data == 0 )
            {
                /*
                 * Report bad pointer.
                 */
                errorOccured( ER_SIM900_BAD_POINTER );
                error( AT, "Bad pointer" );
            }

            /*
             * We received something so change the flag
             * even if an error occurs in the next loop.
             */
            isReceivedData = true;
        }
    }

I want to ask you guys if you have a clue about malloc being slow or if you know UART being interrupted by UART receive interrupt is a problem. How to solve this without lowering UART bitrate without increasing clock speed?

Update:

After debugging I got sure that malloc is working(if malloc fails i get an assert) so my next guess is that appendChar is slow. Although only for reading SMS i will try what Mateo suggested using fixed char array one SMS at a time(160 characters max per SMS + sender info )

Update 2 - New code

I made it work but I did it by luck. There is a line in the code I can not understand why I need it. I have 8Mhz and 19200 UART bitrate. There is something wrong with timing I guess.

This is the line that if I remove the function does not work:

_delay_us(150); // TODO: WEIRD KNOWN BUG!!!

and here the whole function:

/**
 * @brief   Receives raw data from SIM900.
 *
 * @param   data    Get SIM900's raw data on exit.
 *
 * @return  True if at least one character received.
 */
bool SIM900_receive( cString* data )
{
    /*
     * Because we append chars we need to init it.
     */
    #define SIM900_MAX_RECEIVE_CHARACTERS 300
    char temp[SIM900_MAX_RECEIVE_CHARACTERS+1];
    strcpy_P( temp, PSTR("") );
    int32_t i = 0;

    /*
     * There are no received data so far.
     */
    bool isReceivedData = false;

    /*
     * Check UART buffer
     */
    while( uart_available() > 0 )
    {
        /*
         *  There are characters in the buffer so
         *  keep receiving until buffer in empty.
         */
        unsigned int c = uart_getc();
        _delay_us(150); // TODO: WEIRD KNOWN BUG!!!

        if ( c & UART_NO_DATA )
        {
            /*
             * No data available from UART. This is not an error.
             */
            error( AT, "No error but - No uart data" );// TODO: Delete it. It is not an error actually.
        }
        else if ( c & UART_FRAME_ERROR )
        {
            /* Framing Error detected, i.e no stop bit detected */
            errorOccured( ER_UART_FRAME_ERROR );
            error( AT, "Frame error" );
        }
        else if ( c & UART_OVERRUN_ERROR )
        {
            /* 
             * Overrun, a character already present in the UART UDR register was 
             * not read by the interrupt handler before the next character arrived,
             * one or more received characters have been dropped.
             */
            errorOccured( ER_UART_OVERRUN_ERROR );
            error( AT, "Overrun error" );
        }
        else if ( c & UART_BUFFER_OVERFLOW )
        {
            /* 
             * We are not reading the receive buffer fast enough,
             * one or more received characters have been dropped.
             */
            errorOccured( ER_UART_BUFFER_OVERFLOW );
            error( AT, "Buffer overflow error" );
        }
        else
        {
            /*
             * There are data available add them to cString.
             */
            if( i<SIM900_MAX_RECEIVE_CHARACTERS )
            {
                temp[i] = (char) c;
                temp[i+1] = '\0';
                i++;
            }

            /*
             * We received something so change the flag
             * even if an error occurs in the next loop.
             */
            isReceivedData = true;
        }
    }

    copyString( data, temp );

    /*
     * Exit.
     */
    return isReceivedData;
}

Answer 1

Read how heap-based dynamic memory allocation works; this is too broad to explain. Point it's runtime is non-deterministic and it will likely result in fragmentation of your heap, so you eventually will not get any blocks after some time.

Further you are printing something I presume. This also might be a problem. To sum it up: do not use functions with (potentially) longer run time than acceptable in a real-time scenario (the UART receiver imposes an upper limit of processing time.

Use a buffer long enough to get the max. size of a packet. You have to be prepared for this case anyway if you insist on using a linear processing sequence.

A better approach would be to use interrupts to recive data. This way, you can use two alternating buffers (each of max. size) and process one buffer in your normal code, while receiving the next via interrupt handler.

Note: Do not cast unless you really have to and have considered all implications. Very often a cast is a signal of bad interface design.