Problems with receiving strings via UART0 on Arduino UNO

Question

I'm trying to write my own library for sending messages from my Arduino UNO via UART0 to my computer.

The library works successfully except the part where I want to receive strings. The code of the library is:

#define F_CPU 16000000

#define EVEN_P 0
#define ODD_P 1

#define BAUD_RATE 57600

#include <avr/io.h>
#include <math.h>
#include <avr/io.h>
#include <string.h>
#include <util/delay.h>

// Initialize UART0 communication
void UART0_Init_Custom(unsigned long BaudRate, char AsyncDoubleSpeed, char DataSizeInBits, char ParityEVENorODD, char StopBits)
{
    uint16_t UBBR_Value = lrint (F_CPU / 16 / BaudRate - 1 ); // maybe 16L??

    // Setting the U2X bit to 1 for double speed asynchronous (default = 0, normal speed)
    if (AsyncDoubleSpeed == 1) UCSR0A = (1 << U2X0);

    // Upper part of the baud number (bits 8 to 11)
    UBRR0H = (unsigned char)(UBBR_Value >> 8);
    // Rest of the baud number
    UBRR0L = (unsigned char)(UBBR_Value);

    // Enable the receiver and transmitter
    UCSR0B = (1 << RXEN0) | (1 << TXEN0);

    // Set 2 stop bits (default = 1)
    if (StopBits == 2) UCSR0C = (1 << USBS0);

    // Set parity
    if (ParityEVENorODD == EVEN_P) UCSR0C |= (1 << UPM01);
    if (ParityEVENorODD == ODD_P) UCSR0C |= (3 << UPM00);

    // Set data length (default = 5 bits)
    if (DataSizeInBits == 6) UCSR0C |= (1 << UCSZ00); // 6-bit
    if (DataSizeInBits == 7) UCSR0C |= (2 << UCSZ00); // 7-bit
    if (DataSizeInBits == 8) UCSR0C |= (3 << UCSZ00); // 8-bit
    if (DataSizeInBits == 9) UCSR0C |= (7 << UCSZ00); // 9-bit
}

void UART0_Init(unsigned long BaudRate)
{
    if (BaudRate == 115200)
    {
        UART0_Init_Custom((BaudRate/2),1,8,0,2);
    }
    else
    {
        UART0_Init_Custom(BaudRate,0,8,0,2);
    }
}

// Receive Data UART0
char UART0_GET(void)
{
    while (!(UCSR0A & (1 << RXC0)));
    return UDR0;
}

// Transmit Data UART0
void UART0_PUT(char data)
{
    while (!(UCSR0A & (1 << UDRE0)));
    UDR0 = data;
}

// Transmit Data-String UART0
void UART0_PRINT(char* String)
{
    while(*String)
    {
        UART0_PUT(*String++);
    }   
}

// Receive Data-String UART0
char* UART0_READ(void)
{
    char* ReceivedString;
    char ReceivedBit;
    int StringBit = 0;
    memset(&ReceivedString,0,sizeof(ReceivedString));
    while ((ReceivedBit=UART0_GET())!=13)
    {
        UART0_PUT(ReceivedBit);
        ReceivedString[StringBit] = ReceivedBit;
        StringBit++;
    }
    ReceivedString[StringBit] = 13;
    ReceivedString[StringBit++] = 10;
    UART0_PUT(10);
    strncpy(ReceivedString, ReceivedString, StringBit+1);
    return(ReceivedString);
}

char* Input;
int main(void)
{
    UART0_Init(BAUD_RATE);
    UART0_PRINT((char*)"Give a message and I will return it\r\n");
    while(1)
    {   
        Input = UART0_READ();
        UART0_PRINT((char*)"The message was:");
        UART0_PRINT(Input);
    }
}

When running this code, PuTTY shows some random tokens and stops. I cannot insert anything anymore.

Answer 1

I think the main issue you have is that you have not allocated any buffer in which to store the received data (as pointed out in the comments by @WeatherVane).

When writing a function in C that is going to return a string, you have two choices: either the caller provides a buffer and passes a pointer to it into the function, or the function allocates the buffer and returns a pointer to it.

If the function is going to allocate it, then it will have to allocate it on the heap, using the malloc function. You could not use a local variable inside the function, because if you did then as soon as you exited the function, the variable is out of scope. In theory you could use a static variable as a buffer, but if you did that you could only read one string at a time.

If the caller is going to provide a buffer then it could use a local variable, a static variable, or allocate from the heap.

When the buffer is allocated on the heap, no matter who allocates it, the caller must free it when it is finished with it, using the free function.

Embedded systems generally minimize the use of heap allocated variables - see this question for some info about the reasons for that - so you might be better to allow the caller to allocate the buffer, like this:

void UART0_READ(char* buffer, int buflen)
{
//  ... here goes code to read into buffer
}

#define BUFSIZE 100

int main(void)
{
    char input[BUFSIZE];
    UART0_READ(input, BUFSIZE);
    ...
}

Note that I defined the UART0_READ function to take two arguments: the address of the buffer, and it's length. Another thing your program is missing is any guard against the buffer overflowing.

Inside your input routine, use the buflen argument in eg. the call to memset, and inside the loop that reads characters to exit early if too many characters are read.

The buffer is defined as a local character array in the calling function.

Some other notes:

• Don't forget to add a zero, one past the last character added to the array. This also means that you must exit the loop after reading buflen-1 characters, so you have enough space for the 0.

• I'm not sure why you are calling strncpy ? You seem to be copying the string onto itself.

• Running a program with stray pointers in a full size OS will often result in a segfault. Embedded environments often lack the sophistication to detect incorrect memory accesses so instead you might get less predictable behaivour (such as random garbage coming out the serial port).