View topic - Basic UART puts() function won't work how I expect.

Search

Basic UART puts() function won't work how I expect.

AVR Freaks Forum Index » AVR (8-bit) Technical Forums » AVR forum

Author

Message

benjwy

Posted: Jun 07, 2012 - 12:15 PM

Joined: Jun 07, 2012
Posts: 13

This question is an academic curiosity now.

I had written some very basic UART IO functions, and was trying to string together a basic puts() function myself (I am aware there are standard IO facilities available, but I was mucking around and when I found this doesn't work I had to know why).

I hope the code is small enough to dissect, I've made the "puts()" function so simple that I can't see why it does not work!

When I run this attached code, all I see is

Code:

H.H.H.H.H.H.H.H.H.H.H.H.H.H.H.

repeating forever on my terminal. I have tested the uart_putchar() function; it works.

Why on earth won't this do what I expect (print "hi!")?

I am compiling and loading with:

Code:

avr-gcc -mmcu=atmega324a -Os main.c -o main.elf

avr-objcopy -O ihex main.elf main.hex

avrdude -c usbasp -p m324a -U flash:w:main.hex

Here is the code to save downloading (it's small)

Code:

#include <avr/io.h>

int uart_init(void)

{

    unsigned int baud = 12; /* 4800 baud @ 1 MHz */

    /* Set baud rate. */

    UBRR0H = (unsigned char)(baud >> 8);

    UBRR0L = (unsigned char)(baud >> 0);

    /* Enable receiver and transmitter. */

    UCSR0B |= (1 << RXEN0) | (1 << TXEN0);

}

void uart_putchar(char c)

{

    while (!(UCSR0A & (1 << UDRE0) ) ) {

        continue;

    }

    UDR0 = c;

}

void my_puts(char *message)

{

    int i;

    for (i = 0; i < 2; i++) {

        uart_putchar(message[i]);

    }

}

int main(void)

{    

    uart_init();

    char *message = "hi!";

    my_puts(message);

    while (1) {

    }

}

david.prentice

Posted: Jun 07, 2012 - 12:31 PM

Joined: Feb 12, 2005
Posts: 16547
Location: Wormshill, England

I suspect that your AVR is not well calibrated for 1MHz. It is unlikely to see 'H' instead of 'h' unless you have a very bad clock.

Your string function should be:

Code:

void my_puts(char *message)

{

    while (*message) {

        uart_putchar(*message++);

    }

}

David.

benjwy

Posted: Jun 07, 2012 - 12:38 PM

Joined: Jun 07, 2012
Posts: 13

Hi David,

Hmmm if I set up a UART RX interrupt which does

Code:

uart_putchar(uart_getchar())

my text is echoed back fine so I think timing is OK.

Thanks for the function, I had tried to "unravel" the function I posted here as much as possible to show my problem.

I'm more worried about the fact that so much is spewed out to the console so quickly, and over and over again. It is like it is being reset constantly!

For the sake of demonstration I have inserted your function (amending the first '+' to a '*', a typo I assume) and I have the same problem.

Cheers.

david.prentice

Posted: Jun 07, 2012 - 12:58 PM

Joined: Feb 12, 2005
Posts: 16547
Location: Wormshill, England

I would use = rather than |=

Code:

UCSR0B = (1 << RXEN0) | (1 << TXEN0);

Are you sure that your terminal is set up correctly?
Have you READ your fuse settings?
Just add -v to your avrdude cmd.
Are you sure that you are running the code as posted?

The multiple print suggests watchdog reset or duff interrupt.
The 'H' suggests incorrect baud rate. e.g. bad clock.

David.

bobgardner

Posted: Jun 08, 2012 - 05:15 PM

Joined: Sep 04, 2002
Posts: 21396
Location: Orlando Florida

Doesnt message need to be declared somewhere as an array of char?

_________________
Imagecraft compiler user

clawson

Posted: Jun 08, 2012 - 05:30 PM

Joined: Jul 18, 2005
Posts: 62944
Location: (using avr-gcc in) Finchingfield, Essex, England

Quote:

Doesnt message need to be declared somewhere as an array of char?

Um why? There's an anonymous array created when he does:

Code:

char *message = "hi!";

Now if he had made the classic mistake of:

Code:

char *message; 

strcpy(message, "hi!");

then I agree that would be a problem. But when you provide a string initialiser for a char pointer the storage is allocated anyway. In fact a common debug routine I use is something like:

Code:

typedef enum {

  IDLE,

  RUNNING,

  WAITING

} states_t;

void use_state(states_t foo) {

#ifdef DEBUG

  char * statetxt[] = { "Idle", "Running", "Waiting" };

  printf("use_state called with %s\n", statetxt[foo]);

#endif

 ...

}

Creating an array of char pointers to fixed initialisation text is about the quickest way to do something like this.

_________________

Koshchi

Posted: Jun 08, 2012 - 06:02 PM

Joined: Nov 17, 2004
Posts: 13961
Location: Vancouver, BC

Quote:

Hmmm if I set up a UART RX interrupt which does

Code:

uart_putchar(uart_getchar())

my text is echoed back fine so I think timing is OK.

If the timing is slightly off, echoing a single character may not show a problem. When sending a string of characters, any error will be cumulative and become evident.

_________________
Regards,
Steve A.

The Board helps those that help themselves.

benjwy

Posted: Jun 09, 2012 - 02:58 AM

Joined: Jun 07, 2012
Posts: 13

Hi all, thank you for your replies.

I am now convinced that this is a memory access problem, because I see the same reset happening as when I randomly write to memory I don't own. I think this may be important:

This (inside main) works:

Code:

    char *message = "Hello!";

    uart_putchar(message[0]);

    uart_putchar(message[1]);

    uart_putchar(message[2]);

    uart_putchar(message[3]);

    uart_putchar(message[4]);

    uart_putchar(message[5]);

but the following causes constant resets:

Code:

    char *message = "Hello!";

    int i;

    for (i = 0; i < 6; i++) {

        uart_putchar(message[i]);

    }

I am at a total to explain this. I'm going to look at the asm output. Can anyone see what is going on?

david.prentice

Posted: Jun 09, 2012 - 09:12 AM

Joined: Feb 12, 2005
Posts: 16547
Location: Wormshill, England

I simply don't believe you.

I compiled your original code from the first post.

I compiled for ATmega324A with your commands, then ran it on a ATmega324PA chip @ 1MHz. It worked fine.

So are you sure that you have used the right C and HEX file? We have all scratched our heads when we are testing the 'wrong' file.
Hint. "main.c" and "main.hex" are not very intuitive.

Please quote the answer to:

Code:

avr-gcc -v

David.

engineer.pk

Posted: Jun 09, 2012 - 10:56 AM

Joined: Aug 12, 2011
Posts: 44
Location: Pakistan

//use this before main
void uart_init(void);
void uart_sendch(unsigned char);
void uart_sendstr(char*); //pass array by pointer

//use this after main
void uart_init()
{
UBRRL=12;
UCSRB=(1<<TXEN)|(1<<RXEN);
UCSRC=(1<<UCSZ1)|(1<<UCSZ0|(1<<URSEL);

}
void uart_sendch(unsigned char mych)
{
while(!(UCSRA & (1<<UDRE)));
UDR=mych;
}
void uart_sendstr(char *myarray)
{
unsigned char index_ch;
index_ch=0;

//starting from index 0 to n-1
while(*(myarray+index_ch) != '\0')
{
uart_sendch(*(myarray+index_ch));
index_ch++;
}
}

benjwy

Posted: Jun 09, 2012 - 11:18 AM

Joined: Jun 07, 2012
Posts: 13

Hi David,

Yes I understand this seems unbelievable, I've compiled the same code myself on other targets and have not had a problem; this is why I'm trying to gather hints as to what might be the problem in this case.

Here's the information about avr-gcc version:

Code:

> avr-gcc -v

Using built-in specs.

COLLECT_GCC=avr-gcc

COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/avr/4.5.1/lto-wrapper

Target: avr

Configured with: ./../gcc-4.5.1/configure --target=avr --disable-libssp --with-gmp=/usr/local --prefix=/usr/local --mandir=/usr/local/man --infodir=/usr/local/info/ --build=i386-portbld-freebsd8.2

Thread model: single

gcc version 4.5.1 (GCC)

Quote:

So are you sure that you have used the right C and HEX file?

I always use a

Code:

make clean && make program

to program the micro, and the clean target blasts all *.hex files, so I am sure I am writing the correct programs to flash.

Cheers.

benjwy

Posted: Jun 09, 2012 - 11:29 AM

Joined: Jun 07, 2012
Posts: 13

This is getting beyond ridiculous.

When message is set thus it works:

Code:

    char message[7];

    message[0] = 'H';

    message[1] = 'e';

    message[2] = 'l';

    message[3] = 'l';

    message[4] = 'o';

    message[5] = '!';

    message[6] = 0;

but not when set like this:

Code:

char message[] = "Hello!";

snigelen

Posted: Jun 09, 2012 - 11:33 AM

Joined: Jan 08, 2009
Posts: 1199
Location: Lund, Sweden

Is there any difference if you declare message outside main?

Code:

char *message = "Hello!";

int main(void)

{

    int i;

    for (i = 0; i < 6; i++) {

        uart_putchar(message[i]);

    }

}

benjwy

Posted: Jun 09, 2012 - 11:39 AM

Joined: Jun 07, 2012
Posts: 13

Hi snigelen,

Same issue if it's declared outside main Sad

I'm beginning to think something very strange is going on here...

sternst

Posted: Jun 09, 2012 - 12:20 PM

Joined: Jul 23, 2001
Posts: 2472
Location: Osnabrueck, Germany

That all sounds like a missing .data section in the HEX file. Therefore I am with David, I don't believe you, especially related to this:

benjwy wrote:

I am compiling and loading with:

Code:

avr-gcc -mmcu=atmega324a -Os main.c -o main.elf

avr-objcopy -O ihex main.elf main.hex

avrdude -c usbasp -p m324a -U flash:w:main.hex

benjwy wrote:

I always use a

Code:

make clean && make program

Then post your Makefile.

_________________
Stefan Ernst

benjwy

Posted: Jun 09, 2012 - 12:44 PM

Joined: Jun 07, 2012
Posts: 13

No worries, if you can see the problem I'll be excited Smile

Code:

CC = avr-gcc

CFLAGS = -mmcu=atmega324a -Os

OBJCOPY = avr-objcopy

LDFLAGS = 

all: main.hex

main.hex: main.elf

   $(OBJCOPY) -O ihex $(.ALLSRC) $(.TARGET)

main.elf: main.c

   $(CC) $(CFLAGS) $(LDFLAGS) $(.ALLSRC) -o $(.TARGET)

main.s: main.c

   $(CC) -S $(CFLAGS) $(LDFLAGS) $(.ALLSRC) -o $(.TARGET)

program: main.hex

   avrdude -c usbasp -p m324a -U flash:w:$(.ALLSRC)

clean:

   rm -rf *.o *.hex *.elf *.s

Here's the output of make -n program for clarity:

Code:

> make -n program

avr-gcc -mmcu=atmega324a -Os  main.c -o main.elf

avr-objcopy -O ihex main.elf main.hex

avrdude -c usbasp -p m324a -U flash:w:main.hex

sternst

Posted: Jun 09, 2012 - 01:02 PM

Joined: Jul 23, 2001
Posts: 2472
Location: Osnabrueck, Germany

Strange.
Please post the hex file, to check whether the .data section or the do-copy-data loop is missing.

(IMO it could only be a buggy toolchain. Where does it come from?)

_________________
Stefan Ernst

benjwy

Posted: Jun 09, 2012 - 01:14 PM

Joined: Jun 07, 2012
Posts: 13

Thanks for replies. Here is the hex file.

Comparing it to the object dump, I can see both the do_copy_data section and the string itself.

It's just avr-gcc installed from FreeBSD ports. Could be quite old, but I've used it for AVR projects in the past.

Cheers.

bobgardner

Posted: Jun 09, 2012 - 01:21 PM

Joined: Sep 04, 2002
Posts: 21396
Location: Orlando Florida

Since we are 'firing for effect', can you try to init the string in ram manually?

Code:

char message[10];

.

.

.

strcpy(message,"hi");

puts(message);

just for fun? If this works, then all the hidden declare the string in rom and copy it to ram on startup stuff has a problem.

_________________
Imagecraft compiler user

benjwy

Posted: Jun 09, 2012 - 01:37 PM

Joined: Jun 07, 2012
Posts: 13

Hi Bob,

That doesn't work either. The strcpy itself makes things crash.

Just to be clear: the "error" I see is the device constantly resetting.

I say resetting because the LED I enable at the start of the code turns on and off very quickly.

I'll just give a quick update of the "problem". I've stripped it down to the simplist listing that causes the problem (I've verified by compiling and loading)

Code:

#include <avr/io.h>

// #include <string.h>

// #include <stdio.h>

#define F_CPU 4000000L

#include <util/delay.h>

void uart_init(void)

{

    unsigned int baud = 25; /* 9600 baud @ 4 MHz */

    /* Set baud rate. */

    UBRR0H = (unsigned char)(baud >> 8);

    UBRR0L = (unsigned char)(baud >> 0);

    /* Enable receiver and transmitter. */

    UCSR0B = (1 << RXEN0) | (1 << TXEN0);

}

void uart_putchar(char c)

{

    while (!(UCSR0A & (1 << UDRE0) ) ) {

        continue;

    }

    UDR0 = c;

}

void my_puts(char *str)

{

    while (*str) {

        uart_putchar(*str++);

    }

}

int main(void)

{    

    /* Set clock to 4 MHz */

    CLKPR = (1 << CLKPCE);

    CLKPR = (1 << CLKPS0);

    /* Turn an LED on. */

    DDRD |= (1 << 3);

    PORTD |= (1 << 3);

    uart_init();

    /* This will make things die! */

    //char *message = "Hello!";

    /* This works fine. */

    char message[7];

    message[0] = 'H';

    message[1] = 'e';

    message[2] = 'l';

    message[3] = 'l';

    message[4] = 'o';

    message[5] = '!';

    message[6] = 0;

    while (1) {

        _delay_ms(1000);

        my_puts(message);

    }

}

bobgardner

Posted: Jun 09, 2012 - 01:43 PM

Joined: Sep 04, 2002
Posts: 21396
Location: Orlando Florida

Well, how about commenting out the continue in the while in the putchar? (Once again, one of those gee I'll try this to see if it works test)

_________________
Imagecraft compiler user

benjwy

Posted: Jun 09, 2012 - 02:00 PM

Joined: Jun 07, 2012
Posts: 13

Removing the continue didn't change things. I really appreciate you all giving your time to this problem I must say.

This seems significant:

I created a dummy function (I tend to call these things "weasel")

Declaring (inside main):

Code:

char *message = "Hello!";

This outputs 'H' (as expected)

Code:

void weasel(char *str)

{

    uart_putchar(str[0]);

}

This however, crashes things.

Code:

void weasel(char *str)

{

    uart_putchar(str[1]);

}

sternst

Posted: Jun 09, 2012 - 02:13 PM

Joined: Jul 23, 2001
Posts: 2472
Location: Osnabrueck, Germany

Please check the address where the global data is stored. Perhaps this is one of the cases where it accidentally starts at 0x60 instead of 0x100.

_________________
Stefan Ernst

david.prentice

Posted: Jun 09, 2012 - 02:21 PM

Joined: Feb 12, 2005
Posts: 16547
Location: Wormshill, England

Your avr-gcc build looks fairly recent. Winavr-2010 is gcc version 4.3.3

However, Winavr builds are tried and tested.
Atmel Toolchain works fairly well.
Bingo's Linux builds are tried and tested.

I strongly suspect that your toolchain is seriously broken. And it is mystery to me why anyone should choose an 'unusual' build.

I will quite happily run Rowley Crossworks on Ubuntu and have a reliable compiler / debugger / IDE.
I don't mind compiling with avr-gcc on Ubuntu something that is already debugged.

Otherwise, why not use the best tools for the job. This probably means running or emulating Windows.

David.

benjwy

Posted: Jun 09, 2012 - 02:26 PM

Joined: Jun 07, 2012
Posts: 13

sternst wrote:

Please check the address where the global data is stored. Perhaps this is one of the cases where it accidentally starts at 0x60 instead of 0x100.

Code:

00800060 <__data_start>:

  800060:       48 65           ori     r20, 0x58       ; 88

  800062:       6c 6c           ori     r22, 0xCC       ; 204

  800064:       6f 21           and     r22, r15

        ...

Is this the info you're after? That's my string in there Wink

sternst

Posted: Jun 09, 2012 - 02:37 PM

Joined: Jul 23, 2001
Posts: 2472
Location: Osnabrueck, Germany

benjwy wrote:

Is this the info you're after?

Yes, and this

Code:

00800060 <__data_start>:

should be 00800100 instead.

Try to find a toolchain without that bug, or as a workaround put this into the command line:

Code:

-Wl,-Tdata=0x100

_________________
Stefan Ernst

benjwy

Posted: Jun 09, 2012 - 02:45 PM

Joined: Jun 07, 2012
Posts: 13

sternst wrote:

benjwy wrote:

Is this the info you're after?

Yes, and this

Code:

00800060 <__data_start>:

should be 00800100 instead.

Try to find a toolchain without that bug, or as a workaround put this into the command line:

Code:

-Wl,-Tdata=0x100

You. Are. The. Man. I can't thank you enough.

Just so I can learn something from this all, could you please tell me what tipped you off to this, and how you knew that the .data section should start at 0x0100, but might be at 0x0060 by mistake?

Thank-you very much to everyone who helped!

david.prentice

Posted: Jun 09, 2012 - 03:35 PM

Joined: Feb 12, 2005
Posts: 16547
Location: Wormshill, England

avr-gcc is often built by people who don't follow all the news / patches / bug reports.

Personally, I wait until a new build has 'settled down' before using it. i.e. let the others have the grief.

AFIK, the 0x0060 / 0x0100 feature is a build problem.
Most of the Atmel Toolchain features are down to Atmel putting typos in their typos into XML files. Then creating duff system header files.

David.

clawson

Posted: Jun 09, 2012 - 07:45 PM

Joined: Jul 18, 2005
Posts: 62944
Location: (using avr-gcc in) Finchingfield, Essex, England

Quote:

could you please tell me what tipped you off to this,

When you use code such as this:

Code:

    char message[7];

    message[0] = 'H';

    message[1] = 'e';

    message[2] = 'l';

    message[3] = 'l';

    message[4] = 'o';

    message[5] = '!';

    message[6] = 0;

the compiler generates code along the lines of:

Code:

  LDI YL, low(message)

  LDI YH, high(message)

  LDI R24, 'H'

  ST  Y+, R24

  LDI R24, 'e'

  ST  Y+, R24

  LDI R24, 'l'

  ST  Y+, R24

etc.

that is the actual data initialisers ('H', 'e', 'l'..) are built into the code image. When you use:

Code:

char message[] = "Hello!";

No code is immediately generated for this. Instead the string "Hello!" is tagged onto the end of the code image (after .text) and the C compiler simply relies on the fact that the C Run Time (CRT) code that runs before the entry to main() will have a _do_copy_data loop that knows the sizxe of all the initial values in the .data copy following .text in flash and it (in theory!) knows where .data will ultimately be located in RAM then it just does an LPM based copying loop to block copy the entire data from the flash copy and out to RAM. In your case to the wrong place.

Stefan would have recognised that:

Code:

char message[] = "Hello!";

is relying on _do_copy_data and you would not believe the number of thread we get here were someone's home brew makefile has an avr-obcopy command that does something like:

Code:

avr-obcopy -O ihex -j .text proj.elf proj.hex

when ,if -j's are to be used, it should be:

Code:

avr-obcopy -O ihex -j .text -j .data proj.elf proj.hex

as you also want a copy of .data in the .hex file. Both Stefan and I thought that's why your objcopy must be wrong and like him I was slightly surprised when you presented an avr-objcopy command that apparently does not suffer from the "missing .data syndrome" that we see so regularly.

But the reason why the .data stuff was missing in your case is far more pernicious. If your Linux can install .deb then go to my website and get one there:

www.wrightflyer.co.uk/avr-gcc/

By the way your avr-objcopy is NOT safe if you ever use <avr/eeprom.h> or <avr/fuse.h> or <avr/lock.h> so I'd suggest you also seek out "Mfile" and use it to generate your Makefile's (or at least just steal the "good stuff" from its makefile_template),

_________________

snigelen

Posted: Jun 09, 2012 - 08:09 PM

Joined: Jan 08, 2009
Posts: 1199
Location: Lund, Sweden

Since he's using a FreeBSD port of avr-gcc, I guess he's using FreeBSD. Isn't Jörg the maintainer of that port?

benjwy

Posted: Jun 10, 2012 - 01:21 AM

Joined: Jun 07, 2012
Posts: 13

Thanks for the in-depth reply clawson, it's appreciated. Now that I look in my datasheet, I see that RAM starts at 0x100. At least I learned something from you guys Smile

@snigelen, correct. I think he maintains all of the AVR-related ports. I was unsure whether to send a PR to him, or someone else.

Cheers.

engineer.pk

Posted: Jun 16, 2012 - 08:43 PM

Joined: Aug 12, 2011
Posts: 44
Location: Pakistan

The error may be in
void my_puts(char *str)
{
while (*str) {
uart_putchar(*str++);
}
}

change it to
{
while (*(str) != '\0') {
uart_putchar(*str++);
}
}

Last edited by engineer.pk on Jun 16, 2012 - 08:49 PM; edited 1 time in total

Jump to:

All times are GMT + 1 Hour

AVR Freaks Forum Index » AVR (8-bit) Technical Forums » AVR forum

AVRFreaks.net

Search