Go Down

Topic: How to create 3th interrupt (Read 5 times) previous topic - next topic

westfw

Code: [Select]
//pin change interrupt on digital pin 5
//this corresponds to PCINT0_vect
PCICR |= (1 << PCIE0);
PCMSK0 |= (1 << PCINT21);

//pin change interrupt on digital pin 6
//this corresponds to PCINT1_vect
PCICR |= (1 << PCIE1);
PCMSK1 |= (1 << PCINT22);

//pin change interrupt on digital pin 7
//this corresponds to PCINT2_vect
PCICR |= (1 << PCIE2);
PCMSK2 |= (1 << PCINT23);
I haven't used PC interrupts on AVR, but according to my reading, ALL of (arduino) pins 5, 6, and 7 are controlled by PCMSK2 and PCIE2, and interrupt via PCINT2_vector.
The advantage of the external interrupt pins is that you get one interrupt vector per pin, and have control of rising/falling/low causing the interrupt.  With the pin-change interrupt, you get one interrupt vector per port, and get interrupted on any change to any (un)masked bit in that port.

FusiveResonance

Westfw is definitely right. You're code is trying to do too many magical things

Try this first. Remember to implement small chunks of code first, whilst testing along the way. As slow as it seems, its actually faster than banging out a whackload of code and hoping for the best when you hit upload.

Code: [Select]

#include <avr/interrupt.h>
#include <avr/io.h>


void setup() {
 pinMode(5, INPUT);
 Serial.begin(9600);

//pin change interrupt on digital pin 5
//this corresponds to PCINT0_vect
PCICR |= (1 << PCIE2);
PCMSK2 |= (1 << PCINT21);

}

volatile bool print_flag = false;

ISR(PCINT2_vect)
{
     print_flag = true;
}

void loop()
{
     if(print_flag == true)
     {
           Serial.println("interrupt on pcint0");
           print_flag = false;
     }

}


Understand this code before you attempt to modify it (unless it doesn't work, in which case do write back). It seems like you've arbitrarily assigned the pcint vectors and the pcmask registers in your code, but it doesn't work that way.

Have you read: http://www.atmel.com/dyn/resources/prod_documents/doc8161.pdf
If not then begin reading. In particular, start reading section 12 on external interrupts.

If this all seems too intimidating, then do not be concerned, we are still here to help.

I hope that you won't shy away from asking more questions. On the flipside, do realize that I simply yearn for you to find the answer yourself, as that is the always the most rewarding route.

westfw

Code: [Select]
//this corresponds to PCINT0_vect
You left the comment wrong.  The code looks OK, though.  PCIE2, PCMSK2, PCINT2_vect...

(pet peeve against untrue comments!)

EriSan500

Thanks guy's, will give this a try this evening. Will report back.

Greetings,
EriSan500

FusiveResonance

Sorry westfw, that one slipped.

Erisan500, I failed to mention one item.

Notice how I displace the serial.print statement from the ISR into the loop(). This is because, as a general rule of thumb, you should always keep your ISR code short and fast. A rather common way of writing ISR's is to have them modify flags which are then polled by loop().

EriSan500

Untested Code !!!
Code: [Select]

#include <avr/interrupt.h>
#include <avr/io.h>


void setup() {
 pinMode(5, INPUT);
 Serial.begin(9600);

//pin change interrupt on digital pin 5
//this corresponds to PCINT2_vect
PCICR |= (1 << PCIE2);
PCMSK2 |= (1 << PCINT21);

}

volatile unsigned long counter = 0;
volatile bool print_flag = false;

ISR(PCINT2_vect)
{
     counter++;
       print_flag = true;
}

void loop()
{
     if(print_flag == true)
     {
           Serial.println(counter);
           print_flag = false;
     }

}




So this should increase the counter variable every-time the change interrupt fires (twice if we go from LOW to HIGH to LOW) if i understand this correctly?

Now how can i check fast if port 5 is HIGH or LOW, because i only want to increment the counter when the pin goes from LOW to HIGH?

I suppose DigitalRead in the ISR is not the way to go? Looking at the print_flag suggestion made by FusiveResonace, maybe I should also set a 2nd flag pinState = ???? inside the ISR, and check inside the loop if pinState is HIGH, only, what goes in place of the ???? ?

I know that's allot of questions, but I do wanna learn this stuff.

If anyone want's to know why I need this, I'll be happy to explain it.

Many greetings and thanks for the wonderful help so far.
EriSan500

PS: westfw, I corrected the comment  ;)

Mitch_CA

Read here for info on fast read of digital inputs...
http://arduino.cc/en/Reference/PortManipulation

You'll replace ???? with 'true'.  Also note that this...
   if (myValue == true)
when replaced by
   if (myValue)
will evaluate as true as long as myValue is != 0.

I thought pointing out this optimization might help a little with the boolean logic concept in discussion here.

EriSan500

What I was looking for is this:
Code: [Select]

ISR(PCINT2_vect)
{
     counter++;
       print_flag = true;
       pinState = ???? //faster alternative for digitalread(5)
}


So I think what you are saying

Quote

You'll replace ???? with 'true'.


is not correct because pinState will allways be TRUE.

EriSan500

Mitch_CA

Sorry, 'true' that wasn't meant to be a direct injection into your code.

Rather...
Code: [Select]
ISR(PCINT2_vect)
{
   if (PIND & B00100000) // evaluates to TRUE when PortD bit #5 (Arduino digital pin #5) is HIGH
   {
       counter++;
       print_flag = true;
   }
}


EriSan500

#24
Dec 16, 2009, 08:02 pm Last Edit: Dec 16, 2009, 08:05 pm by erisan500 Reason: 1
Thanks Mitch_CA,

that was exactly what i was looking for.

To check for LOW instead of HIGH, I do this right? (or is there a leaner way?)

Code: [Select]

   if (PIND & B00100000 == LOW) // evaluates to TRUE when PortD bit #5 (Arduino digital pin #5) is LOW
   {
       counter++;
       print_flag = true;
   }

Mitch_CA

Code: [Select]
if (!(PIND & B00100000))...

EriSan500

Ok, thanks to everyone who replied, i got the below code working:

Code: [Select]

#include <avr/interrupt.h>
#include <avr/io.h>

void setup() {
 pinMode(5, INPUT);
 Serial.begin(9600);

 //pin change interrupt on digital pin 5
 //this corresponds to PCINT2_vect
 PCICR |= (1 << PCIE2);
 PCMSK2 |= (1 << PCINT21);

}

volatile unsigned long counter1 = 0;

ISR(PCINT2_vect)
{
 if (PIND & B00100000) // evaluates to TRUE when PortD bit #5 (Arduino digital pin #5) is HIGH
 {
   counter1++;
 }
}

void loop()
{
 Serial.println(counter1);
 delay(1000);
}


Now, up to implement the other 2 interrupts. ;)

westfw

Interrupt service routines need to be as short as possible, but you don't need to make them obnoxiously painful.  Using digitialRead inside an ISR should be fine, unless your code is time critical for other reasons (for example, SoftwareSerial expects to be able to make a good effort at running LOTS of pins that do stuff at every bit time at rather high bit rates, so I'd expect it to use raw IO.  If you're going to add a couple of pin-change interrupts that handle relatively low datarates, you can use whatever makes more sense from a program readability point of view...

Coding Badly

#28
Dec 17, 2009, 06:41 am Last Edit: Dec 17, 2009, 06:43 am by bcook Reason: 1

You need to disable interrupts when accessing counter1 outside of the interrupt service routine...

Code: [Select]
void loop()
{
 unsigned long c;
 
 {
   uint8_t SaveSREG = SREG;
   cli();
   c = counter1;
   SREG = SaveSREG;
 }
 
 Serial.println(c);
 delay(1000);
}

FusiveResonance

@Coding Badly

Why bother saving SREG and then restoring it? Why not just use the matching sei() call with cli()

Secondly, isn't retrieving the value of the counter an indivisible call? It would matter if he were retrieving the value, modifying it, and then writing back, but in this case he is simply retrieving the value. I do not understand why that must portion of code must be atomic.  

Go Up