Global variable not working properly after removing printf

Go To Last Post
17 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello all,
I have started working again on a little project of mine involving an ATmega8. It's halfway done and using printf() and its brothers had already brought the flash usage to around 6500 bytes, so I decided that I can go with my own functions to print to USART. I wrote these:

void myputchar(char c)
{
	if (c == '\n')
		myputchar('\r');
	while ((UCSRA & (1 << UDRE)) == 0) {}; //loop_until_bit_is_set(UCSRA, UDRE);
	UDR = c;
}

/******************/

void myprint(char *s)
{
	char *t;
	t = s;
	
	while (*t)
	{
		myputchar(*t);
		t++;
	}
}

They seem to work fine, as I can do things such as:

myprint("Configuring Two-Wire Interface (I2C)...\n");

and even:

char s[8];
atoi(myint, s, 10);
myprint(s);
myputchar('\n');

and everything works (of course, it's not as flexible or elegant as printf(), but it's not a problem).

The issue I'm facing is probably not even linked to it, but since I did this, a global variable called "state" is not being properly initialized. It is declared like this in my main.c, *outside* the main() function:

uint8_t state = STATE_BOOTING;

(STATE_BOOTING, STATE_NORMAL and STATE_CONSOLE are #define'd as 0, 1 and 2 respectively)
This "state" variable is used at start-up, as I have a 3000 ms delay to allow the user to send ESC down the USART to switch to "console mode". The problem is that "state" is not initialized to 0, but rather to a random value usually around 26000, but right now it's *exactly* 8192.

Printing it out inside the ISR for the USART_RXC vector, I get this if I press ESC during the delay:

Quote:
Key pressed. Current state: 8192, key code: 27

Of course, as the "state" variable is not the same as STATE_BOOTING, it is not considered and after a few seconds (*not* 3 seconds as expected) it goes to "normal mode" (do_normal()), printing this:
Quote:
Starting normal operation.
configuration
Reading thermometer configuration

which is weird, because the code generating it is:

void do_normal(void)
{
	uint8_t i = 0;
	uint8_t conf;
	int8_t temperature;
	
	myprint("Writing thermometer configuration\n");
	thermo_writeconf(0x0b);
	myprint("Reading thermometer configuration\n");
	conf = thermo_readconf();
/* ... */
}

The funny thing is that if I press any key at this point, "state" is actually incremented by 1, as if the "state = STATE_NORMAL" statement which is carried out right before calling do_normal() worked... albeit the zero is not really zero, but is the original value of "state":

Quote:
Key pressed. Current state: 8193, key code: 97

I even tried moving the original assignment of state to main(), as in "state = 0", but it doesn't seem to be considered at all. I attempted to run avr-objdump with -d -S to get inline source code, and even though I'm completely ignorant when it comes to avr assembly, it seems to me that it's completely dismissed:

int main(void)
{
  8e:	10 92 62 04 	sts	0x0462, r1
	state = 0;
	USART_init();
  92:	4c d0       	rcall	.+152    	; 0x12c 
	//stdout = &mystdout;
	myprint("\x1b[2J");		// clear screen
  94:	81 ec       	ldi	r24, 0xC1	; 193
  96:	90 e0       	ldi	r25, 0x00	; 0
  98:	5c d0       	rcall	.+184    	; 0x152 
	myprint("\x1b[0;0H");	// go home
  9a:	86 ec       	ldi	r24, 0xC6	; 198
  9c:	90 e0       	ldi	r25, 0x00	; 0
  9e:	59 d0       	rcall	.+178    	; 0x152 

I am extremely confused. Could someone please help shed some light on the matter? :)

Thank you in advance

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Make state volatile:

volatile uint8_t state = STATE_BOOTING;

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Surely that "sts 0x0462,r1" is the equivalent of "state = 0;" ? The compiler almost always keeps 0 in r1 (except when MUL is being used) and I'm guessing your .map file shows that 'state' is held in location 0x462.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The code that shows weird values for state isn't by any chance in another source file than main.c? And might you have another definition of state in that file?

Sorry if this is below your level, but when I see weird stuff I try to look for a straight-forward explanation before deeming it to be a tool-chain bug or some such.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Koshchi: I just tried declaring it volatile, but it doesn't seem to work (I even changed "extern state" to "volatile extern state" in the only other source file that accesses it, but to no avail).
On the other hand the value changed again, it is now 1024 (or 1025).

clawson: thanks for clarifying that. I should definitely read something about avr asm. :)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Johan: it is indeed in another file, and it's declared as "extern state" (now I added volatile to it).
It is not below my level, I not that great C programmer. ;)

Indeed, in main() the state value is correctly defined. How is one supposed to have it working properly? :)

Thanks

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It seems I got it to work with this in main.c:

volatile uint8_t state = STATE_BOOTING;

and this in usart.c:

volatile extern uint8_t state;

(I wonder: why did a simple "extern state" work before?)

Now I just have to understand why it takes much longer than the expected three seconds to actually get to the console menu, but I think I may have just messed up some other part of the code with all this testing.

Thank you all very much!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You've got a few strings playing around in there... Are you possibly running out of memory or encountering stack-collision problems?

An ATmega8 has 1 kilobyte of SRAM available, in addresses from 0x0060 through 0x045F. If 'state' is indeed being held in location 0x0462 as Cliff observed, then that would place it beyond the end of available memory.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

lfmorrison: I might be, I'm not sure. Certaily I am having strange behaviors even though "state" has been fixed at that point. It seems that it easily changes to weird values that I have never coded, so something must be wrong. Perhaps printf() took care of that? I am using the very same strings, just without the built-in conversion of integers.
Maybe I should move the strings to program space?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Maybe I should move the strings to program space?

Umm, yes.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

jollino wrote:
Maybe I should move the strings to program space?
And in swoops... Tutorial Man!! :lol:

Have no fear! Try reading:

[TUT] [C] GCC and the PROGMEM Attribute

Stu

(Wow, great tutorial! Who was that masked man? :wink:)

Engineering seems to boil down to: Cheap. Fast. Good. Choose two. Sometimes choose only one.

Newbie? Be sure to read the thread Newbie? Start here!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you for the link, but the avr-libc was enough to write my own myprint_P and to get an acquaintance with the PSTR macro. I brought the .data section down to 264 bytes and all the strings are being displayed as expected now, I just have some logic flaw somewhere and now it clearly has nothing to do with . I'm going to investigate that.

Speaking of which, would someone be so kind to explain what __clz_tab is about? From what I found, it seems to be something related to PROGMEM stuff.

Disassembly of section .data:

00800060 <__data_start>:
	...

00800068 <__clz_tab>:
  800068:	00 01 02 02 03 03 03 03 04 04 04 04 04 04 04 04     ................
  800078:	05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05     ................
  800088:	06 06 06 06 06 06 06 06 06 06 06 06 06 06 06 06     ................
  800098:	06 06 06 06 06 06 06 06 06 06 06 06 06 06 06 06     ................
  8000a8:	07 07 07 07 07 07 07 07 07 07 07 07 07 07 07 07     ................
  8000b8:	07 07 07 07 07 07 07 07 07 07 07 07 07 07 07 07     ................
  8000c8:	07 07 07 07 07 07 07 07 07 07 07 07 07 07 07 07     ................
  8000d8:	07 07 07 07 07 07 07 07 07 07 07 07 07 07 07 07     ................
  8000e8:	08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08     ................
  8000f8:	08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08     ................
  800108:	08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08     ................
  800118:	08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08     ................
  800128:	08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08     ................
  800138:	08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08     ................
  800148:	08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08     ................
  800158:	08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08     ................
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The __clz_tab thing is an old bug:

http://gcc.gnu.org/bugzilla/show...

which version of GCC are you using? I thought this was fixed in 20080610 ?

Cliff

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avr-gcc -v tells me I'm using version 4.3.0.
I installed the whole thing using AVR MacPack, but that was some time ago. I guess I could upgrade. ;)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Jollino,

If you haven't solved this already, think about the range of values that a uint8_t can hold (0 to 255), since it is only 8 bits. It can't possibly hold 8192, 1024 or 26000ish. Strangle enough the low 8 bits of all the values you mention is "0" as it should be. The problem must be with how you are printing the value. You are most likely printing a 16 bit value which is getting the other 8 bits from a variable next to the state variable.

Hope this helps.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes, that was the problem, "state" was declared as an int while it is in fact an uint8_t:

Quote:
(I wonder: why did a simple "extern state" work before?)
In C anything declared without a type will be treated as an int.
/Lars

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

jollino wrote:
Maybe I should move the strings to program space?

As others have exclaimed, "Yes!". But if/when you do, you begin encountering the untidy detail of not being able to simply ask C to dereference your string pointer (you have to explicitly use a "read-from-FLASH" function instead). If all you're doing is generating "program debugging" messages, it can be awkward.

With trepidation, I offer an amusing alternative to the more rigorous ways of declaring and using strings in program space, inspired from some old Z80 practices. It comes with a LOT of caveats, but within limitations, you might find it more convenient for what you're doing. I'm attaching a .zip set of three files that you can load into AStudio, but I'll include the "main" module here to give the gist of the approach:

#include 
#include "squirt.h"

// Here's a dummy example of a "character consumer":
//
void zogSnuffle(uint8_t arg)
{
    PORTB = arg;
}


void main(void)
{
    // Demonstrate a use of the character-squirter:
    //
    SquirtTo(zogSnuffle, "How do you do?");

    // Prove that execution continues normally
    //
    PORTA = 99;
}

The "zogSnuffle" routine here is just any arbitrary byte-handling routine. You'd probably want to use your "myputchar" routine instead.

I mentioned caveats. They are:

    The strings are read by an "LPM" instruction, and therefore have to reside in the lower 0xffff bytes of program memory.

    The implementation uses some instructions not found on all AVRs: IJMP, ICALL, LPM Rn,Z+, and MOVW. It could be adjusted to work on less-capable cores, but at the cost of extra size.

Attachment(s):