'receiving char * from UART of AVR

I want to receive a string(pointer to characters) by UART using ATMEGA16. I burned this code on the kit then I used hyperterminal (realterm) and made a test to input a string ("on") and if it is received then portc (LEDS) will be set to 1 but it doesn't work ... anyone!? :D

Implementation of functions

#include <avr/io.h>

#define F_CPU 8000000UL
unsigned char *x;

#define USART_BAUDRATE 9600
#define BAUD_PRESCALE (((F_CPU / (USART_BAUDRATE * 16UL))) - 1)

void uartinit()
{
    UCSRB |= (1 << RXEN) | (1 << TXEN);   
                    // Turn on the transmission and reception circuitry
    UCSRC |= (1 << URSEL) | (1 << UCSZ0) | (1 << UCSZ1); 
                    // Use 8-bit character sizes

    UBRRL = BAUD_PRESCALE; // Load lower 8-bits of the baud rate value..
                           // into the low byte of the UBRR register
    UBRRH = (BAUD_PRESCALE >> 8); // Load upper 8-bits of the baud rate value..
                                  // into the high byte of the UBRR register
}

void uartsend( unsigned char *data )
{
    while(*data != '\0')
    {
        /* Wait for empty transmit buffer */
        while ( !( UCSRA & (1<<UDRE)) );
        /* Put data into buffer, sends the data */
        UDR = *data;
        data++;
    }


    while ( !( UCSRA & (1<<UDRE)) );

    UDR = *data;

}

unsigned char * uartrecieve()
{
    //unsigned char i=0;
    //    unsigned char * x;

    /* Wait for data to be received */

    //    char * ptr = &UDR;

    while ( !(UCSRA & (1<<RXC)) );

    while(UDR != '\0')
    {
        *x = UDR;
        x++;
        while ( !(UCSRA & (1<<RXC)) );
    }
    *x = UDR;

     return x;
} 

and this is main function

#include <avr/io.h>
#include "UARTInterface.h"


int main(void)
{
    DDRC=0xFF;
    uartinit();
    while(1)
    {
        unsigned char *y;
        y=uartrecieve();    
        if(strcmp(y,"on")==0)    
        {
            PORTC=0xff;

        }
        //uartsend(y);
        //TODO:: Please write your application code 
    }
} 


Solution 1:[1]

There are a few problems with your code:

1. You're not allocating any space for the received characters. You have a global unsigned char *x (which is not initialised) that you dereference and assign values to, then increment - this is just overwriting random positions in memory.

You should instead assign some space by creating an array from the calling function (main in this case) and passing a pointer to uartreceive along with the size of the buffer

unsigned char y[20]; // in main
unsigned char len;
len = uartreceive(y, 20);
...

Then (note this is untested)

unsigned char uartrecieve(unsigned char *x, unsigned char size)
{
    unsigned char i = 0;

    if (size == 0) return 0;            // return 0 if no space

    while (i < size - 1) {              // check space is available (including additional null char at end)
        unsigned char c;
        while ( !(UCSRA & (1<<RXC)) );  // wait for another char - WARNING this will wait forever if nothing is received
        c = UDR;
        if (c == '\0') break;           // break on NULL character
        x[i] = c;                       // write into the supplied buffer
        i++;
    }
    x[i] = 0;                           // ensure string is null terminated

    return i + 1;                       // return number of characters written
} 

Each time you call this function, it will overwrite the previous contents of rx_buffer so make sure you are finished using it first. Read up on arrays, pointers and strings if you're not certain what's happening here.

Better yet would be to pass in a pointer to uartreceive so that the calling function can supply the area of memory

2. It is unlikely that your serial terminal software will be sending NULL terminated strings by default (i.e. with '\0' at the end), normally it would send a new-line ('\n') character. I believe realterm can do this, but it's worth checking.

3. Reading from UDR will clear the RXC flag, allowing the AVR to write another character into UDR, so reading from UDR twice in a row is probably a bad idea

Solution 2:[2]

UBBRL must be written AFTER UBRRH to ensure atomic operation (ie : copy full 16 bits of ubrr AT THE SAME TIME on baud rate divisor. This is indicated on ATMega doc (for example ATMEGA16 doc http://ww1.microchip.com/downloads/en/devicedoc/doc2466.pdf page 168) :

Writing UBRRL will trigger an immediate update of the baud rate prescaler Atmel example are written this way

void USART_Init( unsigned int ubrr){
/* Set baud rate */ 
UBRRH = (unsigned char)(ubrr>>8);
**UBRRL = (unsigned char)ubrr;**
/* Enable receiver and transmitter */
UCSRB = (1<<RXEN)|(1<<TXEN);
/* Set frame format: 8data, 2stop bit */
UCSRC = (1<<URSEL)|(1<<USBS)|(3<<UCSZ0);
}*

When you write the wrong order (UBRRH last), UBRRH is not updated until the next UBRRL write. Doesn't mater most of the time as UBRRH = 0 the most of the time (until BAUD > 100bit/s)

Sources

This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.

Source: Stack Overflow

Solution Source
Solution 1 Peter Gibson
Solution 2 Stef