Abdo Saied Anwar
Abdo Saied Anwar

Reputation: 111

Global variables modified by main() and accessed by ISR()

Here is my c code

char global_variable = 0;
ISR(){
    PORTA = global_variable;
    toggle_led;//to make sure that the interrupt is triggered
}
int main(){
    while(1){
        _delay_ms(500);
        gobal_variable++;
        PORTB = global_variable;
    }
    return 0;
}

The bottom line is that I have a global variable modified by the main function and read by both the main and ISR -interrupt handler.
When the global variable is read by main I get the expected values, but in ISR I get the value that was first assigned to the global variable.
I know this is an optimization issue but I don't understand what makes the compiler see the right value in the main and the initial value in the ISR

Note: when I modified the variable in the ISR, I read it right in the ISR but in the main I got the initial value.

Upvotes: 0

Views: 3224

Answers (2)

too honest for this site
too honest for this site

Reputation: 12263

ISR has no correct declaration. You really should get used to use prototype-style declarations. Compile with C99 or C11 and you will get a warning. Similar for main:

void ISR(void)

int main(void)

Whereas the signature of main depends on your environment, assuming you are on a bare-metal embedded syetem, i.e. a freestanding environment.

Depending on the target, you have to att compiler-specific attributes to tag the function as interrupt handler.

Said that, you are missing to declare global_variable volatile. You should know, as you already added the tag.

The compiler cannot know the ISR is called anyway and that the variable is modified outside its control-flow. So it can presume it to have the default value, which is 0. Using the volatile qualifier tells it exactly that it cannot and the variable is in fact modified outside.

Note that due to all this, the compiler must not optimize acceses to volatile objects. Thus, you should limit accesses to such objects to the minimum. In main, it is better to use a helper variable to count and just write the updated value once to the volatile object and the counter otherwise. This way you avoid one read and one write:

volatile unsigned char global_variable;

...

int main(void)
{
    unsigned char counter;
    while ( 1 ) {
        _delay_ms(500);
        gobal_variable = counter++;
        PORTB = counter;
    }
    return 0;
}

Notice that I changed the type to unsigned char this is vital, as char can be signed or unsigned and signed integer overflow invokes undefined behaviour in C. Unsigned overflow is defined to simply wrap (i.e.: MAX+1 == 0). A better datatype to use would be uint8_t since C99 (strongly recommended) to explicitly state you are using an 8 bit variable (char does not guarantee this).

Note: According to your comment below, you use an AVR MCU. This is single-core and does not even support memory barriers. So there is absolutely no need for such. Also, as you have a single writer and the writes are atomic (i.e. all-or-nothing of the variable is updated), there is also no need for more complex synchronisation.

However, if you increase the size of the counter, you have to take provisions in ISR or main to ensure reading the value consistently. This because the AVR is an 8 bit machine, thus the update and reads are not atomic.

Note: Due to popular demand, you should check your target if it actually performs the writes atomic. This is also true for 8 bit values. If you are not sure, check the generated assembler code. However, for the AVR, PIC, MSP430, ARM-Cortex-M (iff the busses and registers support byte-writes), the code above is safe unless you use DMA on one of the variables.

Upvotes: 0

4pie0
4pie0

Reputation: 29724

In this case you should insert a memory barrier to assert all writes will be completed before you read. In user space you should declare this variable as volatile.

volatile char global_variable = 0;

Upvotes: 3

Related Questions