I cannot understand why this code does not work. it flickers an LED on pin 8 when an interrupt occurs. When 10 interrupts have occurred i want the LED on pin9 to switch on. It seams like the counter in the ISR does not increment. i do not understand?
# include <avr/io.h>
# include <util/delay.h>
# include <avr/interrupt.h>
volatile int counter;
int pin9 = 9;
int main(void){
DDRB = 0b00000011; //set pins 0 and 1 to output on port B
DDRD = 0b00000000; //ser all pins on port D to input
PORTD = 0b00000100; //enable internal pulup resistor
EIMSK = 1 << INT0; //External interrupt mask register
//this is the interupts enable bit for INT0 pin
sei(); //enable global interupts, This is setting the I bit in the SREG to 1
EICRA = 0 << ISC01 | 1 << ISC00; //(EICRA) External interupt control register A for INT0 (ISC) Interupt sensor control bit,
// 4 bits (namely ISC00,ISC01,ISC10,ISC11) are set which control how interupts react, rising edge, falling edge, any logical change etc
//Note that only two are set per interupt pin
while (1)
{
if (counter > 10){ //When 10 interrupts have occured sitch LED on pin 9 on
digitalWrite(pin9,HIGH);
}
}
}
ISR(INT0_vect){
PORTB ^= 1 << PIN0;
counter = counter+ 1;
reti(); //Signal end interrupts
}
i Figured it out. The issue with the program was that when the while loop was getting the variable counter an interrupt could occur in the middle of retrieving the variable which was corrupting it. This was fixed by defining an atomic block of code to retrieve the counter variable. works like a dream now. The reti(); instruction was also causing issue's, as i understand now its used in assembly code to return interupts, that is enable global interupts bit once the ISR is done
# include <avr/io.h>
# include <util/delay.h>
# include <avr/interrupt.h>
# include <util/atomic.h> //This included the attomic header files which are used to specify a block of code that must ALL be executed. interupts are disabled in this block
volatile long int global_counter;
int pin9 = 9;
int main(void){
volatile long int local_counter;
DDRB = 0b00000011; //set pins 0 and 1 to output on port B
DDRD = 0b00000000; //ser all pins on port D to input
PORTD = 0b00000100; //enable internal pulup resistor
EIMSK = 1 << INT0; //External interrupt mask register
//this is the interupts enable bit for INT0 pin
sei(); //enable global interupts, This is setting the I bit in the SREG to 1
EICRA = 0 << ISC01 | 1 << ISC00; //(EICRA) External interupt control register A for INT0 (ISC) Interupt sensor control bit,
// 4 bits (namely ISC00,ISC01,ISC10,ISC11) are set which control how interupts react, rising edge, falling edge, any logical change etc
//Note that only two are set per interupt pin
while (1)
{
ATOMIC_BLOCK(ATOMIC_RESTORESTATE) //This block disables interupts to ensure the code is definately executed in full
{
local_counter = global_counter;
}
if (local_counter > 1000000){ //When 1000000 interrupts have occured sitch LED on pin 9 on
digitalWrite(pin9,HIGH);
}
}
}
ISR(INT0_vect){
PORTB ^= 1 << PIN0;
global_counter++;
//reti(); //Signal end interrupts seams to cause issue's, only for assembly?
}
The issue with the program was that when the while loop was getting the variable counter an interrupt could occur in the middle of retrieving the variable which was corrupting it. This was fixed by defining an atomic block of code to retrieve the counter variable.
Since the useful values that you want to store in the counter are 0 to 9 (or 10), why are you using a variable type that can not be copied/reset atomically? Why use a workaround instead of correcting the problem?
PaulS:
Since the useful values that you want to store in the counter are 0 to 9 (or 10), why are you using a variable type that can not be copied/reset atomically? Why use a workaround instead of correcting the problem?
what variable can i copy/reset atomically? i did not know u get such a variable. thank you for pointing that out to me
calvingloster:
This is a line to say the interrupt service routine is done? should it not be there? any idea's why the counter wont count?
The ISR macro generates the reti instruction so you don't need to put it there yourself.
I do not want to use arduino code cause it hides too much information. i prefer using AVR c code even though im a noob at it
If you say so, however I have to say your code isn't particularly easy to read. You will find that out next year when you review and and wonder what all those bits do.
PaulS:
Given your initial requirements, that makes no sense.
Yes your reply is totally valid. I am however asking what variable can I use if I need my counter to go up into the millions? Will an atomic block then be ok to use with a long int type?
calvingloster:
Yes your reply is totally valid. I am however asking what variable can I use if I need my counter to go up into the millions? Will an atomic block then be ok to use with a long int type?
I think his point is if you can use a data type that can be read/written as a single operation (for these processors, that would be a single byte) then do so and you won't need any special protections. The code you first posted is such an example since the threshold value is only 10.
If you must use a larger value, one that cannot be done as a single read/write operation, then you will need some special protection to make sure you aren't accessing a partially updated value.
One issue with using a protection method, such as turning off the interrupts during a multi-byte operation, is that it can cause timing or latency issues as the project gets more complex. It's best not to be turning interrupts on and off if you can avoid it.
To clarify (as is done in my link about interrupts) the issue is if you attempt to share a variable between an ISR and main (non-ISR) code. There is no problem with simply accessing multi-byte variables in general as, if an interrupt occurred, the code resumes where it was interrupted.
The problem is if the ISR changes the variable, half-way through reading it. Even single byte variables are not immune in this sort of scenario
* Read a variable
* Add one to it
* Write it back
If the interrupt occurs after reading but before writing it back, you still have a problem.
You will find, for example, that the code for millis() turns off interrupts, while it reads the unsigned long which is the current elapsed time.
It's best not to be turning interrupts on and off if you can avoid it.
Indeed, but every time you call millis() you are doing it. Ditto for micros(). And these are called a lot.