GCC global variables

[Simon]

I have a problem, it was not there at first then I did something unrelated and it cropped up then removed that unrelated thing and it stayed.

i have a program, and most af the variables need to be accessible to all functions. In particular the main time counter variable that controls the timing of the program running. So at first I put all external variables declarations in the one header file that is included, but that caused issues like the variable could not be found or was declared more than once. So i tried putting the creating of variabres in the setup.c file where all the setup code is put. and i put in every other c file a declaration for this variable so that all files know that it exists: extern uint16_t timer;

but still no joy, so the program sets up and starts one of the timer/counters at 1KHz and every time the counter gets to the top an interrupt fires and increments the "timer" variable, the interupt routine is happening because I have verified it. but when i then test in my main while loop for this variable having exceeded 10 nothing happens so the while loop code seems to net be looking at this variable and maybe more than one has been created?

This is infuriating, I have done this before and it worked. Books don't help because I assume GCC is not C99!

what do i do? can it be something to do with Atmel Studio, who's is the preprocessor in atmel studio?

[Nusa]

Try using the "volatile" keyword for any global variables changed in interrupt routines. You may be fighting compiler optimizations.

[Simon]

Yes the variable is volatile

[SiliconWizard]

Try using the "volatile" keyword for any global variables changed in interrupt routines. You may be fighting compiler optimizations.

Not knowing anything more about the OP's source code, I'd vote for this one as well. Has been discussed a few times on this forum.

Typically the compiler has no way to know that interrupt routines are ever called, contrary to other functions in your code that are explicitely called, so it may assume that whatever is done inside them has no effect. The volatile keyword makes the compiler assume that the qualified variable is going to get modified outside of the function(s) using it (in "read-only" manner).

[SiliconWizard]

Yes the variable is volatile

Maybe post the relevant part(s) of your code then... If you can.

[Mechatrommer]

have you make guard ring in your header file? such as...

Code: [Select]

#ifndef MYFILE_HPP
#define MYFILE_HPP

extern int myvar;
// your codes

#endif // MYFILE_HPP

myvar must be defined in one of c files (only once!) such as

Code: [Select]

// myfile.c

int myvar;
// or volatile int myvar;

a snipped code from your actual code provided here may help..

[Andy Watson]

When you change something that is global - like say a common header file - you have to ensure that all the modules that are affected by the change are also re-compiled - this should be written into your make file, but it's not always the case. You might have to force a re-compile of the whole project for the changes to take effect.

[Simon]

When you change something that is global - like say a common header file - you have to ensure that all the modules that are affected by the change are also re-compiled - this should be written into your make file, but it's not always the case. You might have to force a re-compile of the whole project for the changes to take effect.

how do i do that?

[SiliconWizard]

Just a thought here. You said you used the volatile qualifier, but how?

If you're just using it at the declaration, such as: "volatile int myvar;" but fail to add it in the header file (which would be: "extern volatile int myvar;", but maybe you just wrote: "extern int myvar;"), I think it's going to be ignored by the compiler while compiling any other C source file that includes the header. SO it must be added to the extern declaration as well.

[brucehoult]

It's very simple.

Ideally you should put: "extern uint16_t timer;" in the header file, and then in *one* .c file put either "uint16_t timer;" to have it initialized to zero or else "uint16_t timer = 1234;" to initialize it to some other value (which could be zero, to be explicit).

As a special convenience, if you want it to be zero initialized then you can simply put "uint16_t timer;" in the header file and nothing in any .c file. If you do this then the .o from every .c file will contain a definition of timer but the linker will merge them all together,

But it's better to put "extern" in the header file and without "extern" in *one* .c file.

[Simon]

main.c

Code: [Select]

#include <inttypes.h>
#include <avr/io.h>
#include "main.h"

// System variables
extern uint16_t timer;



int main(void)
{
setup();

// test code
PORTA.DIRSET = 0x01 << PA7;

   
    while (1)
    {
if(timer > 10)
{
schedule_10ms();
timer = 0;
// test code
PORTA.OUTTGL = 0x01 << PA7;
}


    }
}

setup.c

Code: [Select]

#include "main.h"

// System variables
volatile uint16_t timer;
volatile uint16_t pulse0;
volatile uint16_t pulse1;

volatile uint8_t mode;
volatile uint8_t diagnostic = 0;
volatile uint16_t fan_speed;
volatile uint16_t scav_speed;

void setup(){

PORTF.DIRCLR = 0x01 << 4; // button input
PORTF.PIN4CTRL = 0x01 << 3; // enable button pin pullup

PORTD.DIRCLR = 0x01 << 3 || 0x01 << 4; //taco 0 & 1 input

PORTD.DIRSET = 0b11100000; // led outputs 0-2 PD5:7
PORTF.DIRSET = 0x1 << PF3;  // led output 3

PORTF.DIRSET = 0x1 << PF0; //power switch control

PORTC.DIRSET = 0x01 << PC0; // fan_pwm
PORTC.DIRSET = 0x01 << PC1; // scav_pwm
PORTC.DIRSET = 0x01 << PC2; // charge pump pwm
PORTMUX.TCAROUTEA = 0x02; // Set TCA output on portC
TCA0.SINGLE.INTCTRL = 0x01; //enable OFL interrupt
TCA0.SINGLE.CTRLB = 0b01110011; // single slope PWM with all 3 output channel enabled
TCA0.SINGLE.CMP0 = 0xF; // Channel 0 compare value
TCA0.SINGLE.CMP1 = 0xF; // Channel 1 compare value
TCA0.SINGLE.CMP2 = 0xF; // Channel 2 compare value
TCA0.SINGLE.PER = 3333; // Top value for PWM counter to give 1KHz with a (10/3)MHz CLK_PER
TCA0.SINGLE.CTRLA = 0x01; //enable counter with no clock scaler

TCB0.CTRLB = (( TCB0.CTRLB & ~( 0x07 )) | ( 0x05 << 0 ));  // Set counter mode to input capture frequency and pulse width measurement
TCB0.INTCTRL = ( TCB0.INTCTRL & ~0x01 ) | ( 0x01 << 0 ); // Enable capture compare interrupt

EVSYS.CHANNEL2 = 0b01001011; // event channel 2 connects to pin PD3
EVSYS.USERTCB0 = 3; // connect the counter to event channel 2 (3-1)

TCB0.CTRLA = (( TCB0.CTRLA & ~( 0x03 << 1 )) | ( 0x0 << 1 )); // Set clock to main clock (CLK_PER)
TCB0.CTRLA = (( TCB0.CTRLA & ~0x01 ) | 0x01 );  // Enable the counter (connects the counter to the clock source)

TCB1.CTRLB = (( TCB1.CTRLB & ~( 0x07 )) | ( 0x05 << 0 ));  // Set counter mode to input capture frequency and pulse width measurement
TCB1.INTCTRL = ( TCB1.INTCTRL & ~0x01 ) | ( 0x01 << 0 ); // Enable capture compare interrupt

EVSYS.CHANNEL3 = 0b01001100; // event channel 2 connects to pin PD4
EVSYS.USERTCB1 = 4; // connect the counter to event channel 3 (4-1)

TCB1.CTRLA = (( TCB1.CTRLA & ~( 0x03 << 1 )) | ( 0x0 << 1 )); // Set clock to main clock (CLK_PER)
TCB1.CTRLA = (( TCB1.CTRLA & ~0x01 ) | 0x01 );  // Enable the counter (connects the counter to the clock source)

sei();
}

interrupts.c

Code: [Select]

#include "main.h"
#include <avr/interrupt.h>

// System variables
extern uint16_t timer;
extern uint16_t pulse0;
extern uint16_t pulse1;

ISR(TCA0_OVF_vect){
timer = timer + 1;
TCA0.SINGLE.INTFLAGS = 0x01;
}

ISR ( TCB0_INT_vect ){
pulse0 = TCB0.CCMP;
TCB0.INTFLAGS = 0x01;
}

ISR ( TCB1_INT_vect ){
pulse1 = TCB1.CCMP;
TCB1.INTFLAGS = 0x01;
}


[Simon]

Just a thought here. You said you used the volatile qualifier, but how?

If you're just using it at the declaration, such as: "volatile int myvar;" but fail to add it in the header file (which would be: "extern volatile int myvar;", but maybe you just wrote: "extern int myvar;"), I think it's going to be ignored by the compiler while compiling any other C source file that includes the header. SO it must be added to the extern declaration as well.

I think you hit the nail on the head. Ideally I wanted to put all external (and volatile) variables in the one header file so that they are universally available.

[djacobow]

I think the variable needs to be declared "volatile" in the header where it is "extern" as well as in the c file where the variable is actually declared.

Remember, C does compilation one file at a time. When the compiler is compiling a file that references an "extern" variable, everything it knows about that variable is in the header. In the example I just saw posted from Simon, any unit files that use "timer" will not know to treat it as volatile except the one unit file where the variable was actually instantiated.
 

[SiliconWizard]

Ideally I wanted to put all external (and volatile) variables in the one header file so that they are universally available.

That's fine. Just don't forget to make all your extern declarations fully qualified (ie. contain the same qualifiers as in the original declarations).

Not going to discuss the merits or drawbacks of using an excessive number of global variables here. I will just suggest limiting them when you can, because it can get messy real fast.
A few critical globals - that's fine.

One possible approach with your timer example, unless accessing (reading) the variable is extremely time-critical, you could use an accessor function instead of making the variable global. Said function is declared inside the same C file as the variable, and just returns its value. Just export the prototype of the function. Several added benefits of this (to name a few): it makes the possibility of modifying the variable outside of its intended scope impossible (it becomes effectively "read-only" to the outside source files), and it allows to add extra tests or guarding instructions when reading it: for instance, if the variable is too large to be read atomically on your target, your accessor function can include guarding instructions (for instance: disabling the timer interrupt, reading the variable in a local variable, re-enabling the timer interrupt, and finally returning the local variable...)

[Mechatrommer]

I think the variable needs to be declared "volatile" in the header where it is "extern" as well as in the c file where the variable is actually declared.

after looking at his code, this will be my thought too, but not 100% sure since i havent code for sometime. maybe..

volatile uint16_t timer; // <- definition
extern volatile uint16_t timer; // <- declaration

ps: definition is not declaration in computing science terminology

googling the net may give some light...
https://www.google.com/search?q=extern+volatile&ie=utf-8&oe=utf-8&client=firefox-b
https://www.avrfreaks.net/forum/extern-volatile-declaration

[Simon]

Ideally I wanted to put all external (and volatile) variables in the one header file so that they are universally available.

That's fine. Just don't forget to make all your extern declarations fully qualified (ie. contain the same qualifiers as in the original declarations).

Not going to discuss the merits or drawbacks of using an excessive number of global variables here. I will just suggest limiting them when you can, because it can get messy real fast.
A few critical globals - that's fine.

One possible approach with your timer example, unless accessing (reading) the variable is extremely time-critical, you could use an accessor function instead of making the variable global. Said function is declared inside the same C file as the variable, and just returns its value. Just export the prototype of the function. Several added benefits of this (to name a few): it makes the possibility of modifying the variable outside of its intended scope impossible (it becomes effectively "read-only" to the outside source files), and it allows to add extra tests or guarding instructions when reading it: for instance, if the variable is too large to be read atomically on your target, your accessor function can include guarding instructions (for instance: disabling the timer interrupt, reading the variable in a local variable, re-enabling the timer interrupt, and finally returning the local variable...)

Yes I know that in theory nothing should be external but I find that it helps greatly in structuring the program as it is far easier to then break the code into individual functions that will operate on the same variable set.

With regard to the timer variable it needs up-counting by the interrupt routine, testing by the main if statement which could be done with what yo describe but it needs zeroing again by the "if"s code.

[tggzzz]

Don't forget the differences between volatile and atomic declarations. Whether that makes any difference depends on the processor, the compiler, the compiler version, and the compilation switches.

Don't forget what volatile doesn't guarantee about, for example, re-ordering evaluation.

No, despite having used C since 1981, I am not a C expert anymore. If you want a full answer and understanding, consult a language lawyer

[Simon]

what is an atomic declaration?
what is re-ordering evaluation.

[coppice]

what is an atomic declaration?
what is re-ordering evaluation.

On an 8 bit machine accessing 16 or 32 bit variables is not atomic. That is, they will usually be accessed as two or four 8 bit chunks, and the variable might be changed by other code between these accesses.

The re-ordering issue is that you should not assume that things will be executed in the order they occur in your source code. Variables may be accessed in a very different order, typically to keep a better flow through the machine's pipelines.

[Simon]

Ah right. In actual fact i have no idea why i made it a 16 bit variable as I am only counting to 10. I'll be sorting that out when I make corrections later.

[SiliconWizard]

With regard to the timer variable it needs up-counting by the interrupt routine, testing by the main if statement which could be done with what yo describe but it needs zeroing again by the "if"s code.

Which in the same approach, can be perfectly done adding another function that resets the counter. And call that instead of directly assigning "0" to the variable.

[mikerj]

With regard to the timer variable it needs up-counting by the interrupt routine, testing by the main if statement which could be done with what yo describe but it needs zeroing again by the "if"s code.

Which in the same approach, can be perfectly done adding another function that resets the counter. And call that instead of directly assigning "0" to the variable.

  Using small functions to get/set the value of variables is a much better idea than having a huge clump of global variables.  Also I don't see any mention of shadowing, perhaps there is another variable of the same name which is closer in scope to the function using it, often caused by beginners using header files to define variables.

[rstofer]

Atomic access means that the entire store operation is completed without possible interruption.  This doesn't work when you have a multi-byte variable that is moving in and out of narrow memory.  In fact, since you don't look at the assembly code, you can't assume it works at all.

Inside the interrupt routine the handler 'owns' the variable.  Nothing in the mainline can influence what happens in the interrupt routine.  That isn't true for the mainline.  Consider what happens when the assembly code is halfway done modifying a multi-byte variable and the interrupt fires off and changes everything.  It creates a mess!

So, when your mainline code simply must modify a value that is manipulated in an interrupt handler, the only safe thing to do is shut down the interrupts before the mainline changes the value and re-enable the interrupts after the change.  It's called a 'critical region' in RTOS parlance.

Volatile doesn't handle this at all.  Volatile simply makes the compiler emit code to re-evaluate a variable every time through a loop.  Ordinarily, a variable would be considered 'loop invariant' if it isn't changed inside the loop and evaluated just once, probably before entering the loop.

[T3sl4co1l]

Another option, if a somewhat heavy-handed one, is to put all the variables into one .c file, and expose only accessor functions.  This is equivalent to the OOP practice of private variables, private helper functions, and public getters/setters.

Probably not be worthwhile on a small or embedded project, but then again, embedded CPUs are awfully powerful these days; a Cortex M0 at 160MHz doing the job of a PIC at 8MHz say?  Lots of room for verbosity.

If you have atomic access to worry about (the getters/setters may sometimes be called from interrupts?), you can set an atomic flag that another function is currently executing, and decide to pass on execution (the function would return an error code that it failed).  It would be very convenient to simply say, "go back to that other function and finish it up, then come back here" -- but that is actually terribly difficult to set up (such capability is one of the distinguishing features of a proper multitasking OS), so for ordinary C code it is much easier to fail out.

Or you can do atomic buffering operations, queuing the getting/setting operations so they are always done exactly in sequence.  But, such a getter needs a target pointer, or callback function, to know what to do with its returned data; and that operation in turn probably needs to be performed atomically, too.  So it's not that much easier, and still just as prone to bugs.

If you have a simple, special-case, one-way data flow, that doesn't necessarily need to be in sequence (like a graphical frame buffer, which is new every time): double buffering is an excellent way to handle it.  Set an atomic flag which indicates which buffer to read from, and write to the opposite buffer.  Toggle the flag once a read and write cycle has been completed.  That way, no reading or writing is ever trampled.

Tim

[brucehoult]

Probably not be worthwhile on a small or embedded project, but then again, embedded CPUs are awfully powerful these days; a Cortex M0 at 160MHz doing the job of a PIC at 8MHz say?  Lots of room for verbosity.

Where have you found a Cortex M0 at 160 MHz?

I've never seen an M0 offered at more than 50 MHz, or an M0+ at more than 75.