Is better to use ISR(PCINT2_vect) or attachInterrupt on pins 2, 3 ?

Projects Programming
1

Hello,

Is better to use ISR(PCINT2_vect) or attachInterrupt on pins 2, 3 on CHANGE event?
I need this to increase the resolution of an optical encoder
Is there any way to increase the speed of the code?
Here is the code with both variants in it

#define MASK_A1 0b100    //(1<<2)
#define MASK_A2 0b1000   //(1<<3)
#define MASK_B1 0b10000  //(1<<4)
#define MASK_B2 0b100000 //(1<<5)
volatile int PosA = 0;
volatile int PosB = 0;
volatile byte A1 = 0;//Status of pin 4 H or L
volatile byte A2 = 0;//Status of pin 5 H or L
volatile byte B1 = 0;//Status of pin 4 H or L
volatile byte B2 = 0;//Status of pin 5 H or L

void Setup45()
{ 
	if (PIND & MASK_A1)//replace attachInterrupt(0, UpdateA, CHANGE);
		A1 = 1;
	if (PIND & MASK_A2)//replace attachInterrupt(1, UpdateB, CHANGE);
		A2 = 1;	
	if (PIND & MASK_B1)
		B1 = 1;
	if (PIND & MASK_B2)
		B2 = 1;		
	PCICR |= (1 << PCIE2);//Interrupt on pin change FROM PCINT16 to PCINT23 Digital
	PCMSK2 = (1 << PCINT18) | (1 << PCINT19) | (1 << PCINT20) | (1 << PCINT21); // digital pin 4 and 5
}

void setup()
{
	//in this case myPinX is the same as Bit
	DDRD &= ~(1 << 2);//Set pin 2 as input
	DDRD &= ~(1 << 3);//Set pin 3 as input
	DDRD &= ~(1 << 4);//Set pin 4 as input
	DDRD &= ~(1 << 5);//Set pin 5 as input
	DDRD |=  (1 << 6); //Set pin 6 as output
	PORTD |= B01111100;//Turn On 20KOhm resistors on 2,3,4,5,6 (HIGH)
	//Is better to use ISR(PCINT2_vect) or attachInterrupt on pins 2, 3 ?
	// attachInterrupt(0, UpdateA, CHANGE);// encoder pin on interrupt 0 - pin 2
	// attachInterrupt(1, UpdateB, CHANGE);// encoder pin on interrupt 1 - pin 3
	
	Serial.begin (115200);
	Serial.println("start");

}

void loop()
{
	Serial.println(PosA, DEC);// debug - remember to comment out
	Serial.println(PosB, DEC);// debug - remember to comment out
}

ISR(PCINT2_vect)//is the function called by the interrupt on PCINT2
{
	if ((PIND & MASK_A1) != A1)//if digital PIN2 has changed
	{
		A1 = PIND & MASK_A1;//Set new status
		if (!((PIND & MASK_B1) ^ A1))                       
			PosA++;//If (A1 = 0 and B1 = 1) or (A1 = 1 and B1 = 0)
		else
			PosA--;//If (A1 = 0 and B1 = 0) or (A1 = 1 and B1 = 1)
	}
	if ((PIND & MASK_A2) != A2)//if digital PIN3 has changed
	{
		A2 = PIND & MASK_A2;//Set new status
		if (!((PIND & MASK_B2) ^ A2))                       
			PosB++;//If (A2 = 0 and B2 = 1) or (A2 = 1 and B2 = 0)
		else
			PosB--;//If (A2 = 0 and B2 = 0) or (A2 = 1 and B2 = 1)
	}
	
	if ((PIND & MASK_B1) != B1)//if digital PIN4 has changed
	{
		B1 = PIND & MASK_B1;//Set new status
		if (!((PIND & MASK_A1) ^ B1))                       
			PosA++;//If (A1 = 0 and B1 = 1) or (A1 = 1 and B1 = 0)
		else
			PosA--;//If (A1 = 0 and B1 = 0) or (A1 = 1 and B1 = 1)
	}
	if ((PIND & MASK_B2) != B2)//if digital PIN5 has changed
	{
		B2 = PIND & MASK_B2;//Set new status
		if (!((PIND & MASK_A2) ^ B2))                       
			PosB++;//If (A2 = 0 and B2 = 1) or (A2 = 1 and B2 = 0)
		else
			PosB--;//If (A2 = 0 and B2 = 0) or (A2 = 1 and B2 = 1)
	}
}

// void UpdateA()
// {
	// if (!((PIND & MASK_A1) ^ (PIND & MASK_B1)))                       
		// PosA++;//If (A1 = 0 and B1 = 1) or (A1 = 1 and B1 = 0)
	// else
		// PosA--;//If (A1 = 0 and B1 = 0) or (A1 = 1 and B1 = 1)
// }

// void UpdateB()
// {
	// if (!((PIND & MASK_A2) ^ (PIND & MASK_B2)))
		// PosB++;//If (A2 = 0 and B2 = 1) or (A2 = 1 and B2 = 0)
	// else
		// PosB--;//If (A2 = 0 and B2 = 0) or (A2 = 1 and B2 = 1)
// }
2

The attachInterrupt() function assigns the function to the interrupt vector, so, in the end the same function is called either way.

Is there any way to increase the speed of the code?

Get rid of the Serial.print() calls.

3

Actually attachInterrupt moves the function address to an array. Like so:

intFunc[interruptNum] = userFunc;

Note, it also turns on the appropriate interrupt (which I don't see you doing in setup when you "manually" do the interrupts).

Then the generated code for the interrupt checks to see if the function is in the table, and if so, calls it, like this:

SIGNAL(INT1_vect) {
  if(intFunc[EXTERNAL_INT_1])
    intFunc[EXTERNAL_INT_1]();
}

So, using attachInterrupt would be slightly slower, because of the overhead of the table lookup. However the overhead is small.

I need this to increase the resolution of an optical encoder
Is there any way to increase the speed of the code?

What speed are you getting, and what speed do you hope to get?

As PaulS says, doing Serial.print is hardly the way to time interrupts. The overhead of printing is too high.

4

Note, it also turns on the appropriate interrupt (which I don't see you doing in setup when you "manually" do the interrupts).

Dosen't the code below turn on interrupt on pin change on digital pins 2,3,4,5?

PCICR |= (1 << PCIE2);//Interrupt on pin change FROM PCINT16 to PCINT23 Digital
	PCMSK2 = (1 << PCINT18) | (1 << PCINT19) | (1 << PCINT20) | (1 << PCINT21); // digital pin 4 and 5

Forgot to add 2 and 3 pins

By faster i was thinking Is there another way to write the if statements from the ISR(PCINT2_vect) in order to have less cpu cycles?

I'm trying to use this to read the optical encoders of two servos
62.5rpm that will give me about 150 interrupts /second

PS: Here is the code that compiles

#define MASK_A_1 (1<<2)
#define MASK_A_2 (1<<3)
#define MASK_B_1 (1<<4)
#define MASK_B_2 (1<<5)
volatile int PosA = 0;
volatile int PosB = 0;
volatile byte A_1 = 0;//Status of pin 4 H or L
volatile byte A_2 = 0;//Status of pin 5 H or L
volatile byte B_1 = 0;//Status of pin 4 H or L
volatile byte B_2 = 0;//Status of pin 5 H or L

void Setup45()
{ 
	if (PIND & MASK_A_1)//replace attachInterrupt(0, UpdateA, CHANGE);
		A_1 = 1;
	if (PIND & MASK_A_2)//replace attachInterrupt(1, UpdateB, CHANGE);
		A_2 = 1;	
	if (PIND & MASK_B_1)
		B_1 = 1;
	if (PIND & MASK_B_2)
		B_2 = 1;		
	PCICR |= (1 << PCIE2);//Interrupt on pin change FROM PCINT16 to PCINT23 Digital
	PCMSK2 = (1 << PCINT18) | (1 << PCINT19) | (1 << PCINT20) | (1 << PCINT21); // digital pin 4 and 5
}

void setup()
{
	//in this case myPinX is the same as Bit
	DDRD &= ~(1 << 2);//Set pin 2 as input
	DDRD &= ~(1 << 3);//Set pin 3 as input
	DDRD &= ~(1 << 4);//Set pin 4 as input
	DDRD &= ~(1 << 5);//Set pin 5 as input
	DDRD |=  (1 << 6); //Set pin 6 as output
	PORTD |= B01111100;//Turn On 20KOhm resistors on 2,3,4,5,6 (HIGH)
	//Is better to use ISR(PCINT2_vect) or attachInterrupt on pins 2, 3 ?
	// attachInterrupt(0, UpdateA, CHANGE);// encoder pin on interrupt 0 - pin 2
	// attachInterrupt(1, UpdateB, CHANGE);// encoder pin on interrupt 1 - pin 3
	
	Serial.begin (115200);
	Serial.println("start");

}

void loop()
{
	Serial.println(PosA, DEC);// debug - remember to comment out
	Serial.println(PosB, DEC);// debug - remember to comment out
}

ISR(PCINT2_vect)//is the function called by the interrupt on PCINT2
{
	if ((PIND & MASK_A_1) != A_1)//if digital PIN2 has changed
	{
		A_1 = PIND & MASK_A_1;//Set new status
		if (!((PIND & MASK_B_1) ^ A_1))                       
			PosA++;//If (A_1 = 0 and B_1 = 1) or (A_1 = 1 and B_1 = 0)
		else
			PosA--;//If (A_1 = 0 and B_1 = 0) or (A_1 = 1 and B_1 = 1)
	}
	if ((PIND & MASK_A_2) != A_2)//if digital PIN3 has changed
	{
		A_2 = PIND & MASK_A_2;//Set new status
		if (!((PIND & MASK_B_2) ^ A_2))                       
			PosB++;//If (A_2 = 0 and B_2 = 1) or (A_2 = 1 and B_2 = 0)
		else
			PosB--;//If (A_2 = 0 and B_2 = 0) or (A_2 = 1 and B_2 = 1)
	}
	
	if ((PIND & MASK_B_1) != B_1)//if digital PIN4 has changed
	{
		B_1 = PIND & MASK_B_1;//Set new status
		if (!((PIND & MASK_A_1) ^ B_1))                       
			PosA++;//If (A_1 = 0 and B_1 = 1) or (A_1 = 1 and B_1 = 0)
		else
			PosA--;//If (A_1 = 0 and B_1 = 0) or (A_1 = 1 and B_1 = 1)
	}
	if ((PIND & MASK_B_2) != B_2)//if digital PIN5 has changed
	{
		B_2 = PIND & MASK_B_2;//Set new status
		if (!((PIND & MASK_A_2) ^ B_2))                       
			PosB++;//If (A_2 = 0 and B_2 = 1) or (A_2 = 1 and B_2 = 0)
		else
			PosB--;//If (A_2 = 0 and B_2 = 0) or (A_2 = 1 and B_2 = 1)
	}
}

// void UpdateA()
// {
	// if (!((PIND & MASK_A_1) ^ (PIND & MASK_B_1)))                       
		// PosA++;//If (A_1 = 0 and B_1 = 1) or (A_1 = 1 and B_1 = 0)
	// else
		// PosA--;//If (A_1 = 0 and B_1 = 0) or (A_1 = 1 and B_1 = 1)
// }

// void UpdateB()
// {
	// if (!((PIND & MASK_A_2) ^ (PIND & MASK_B_2)))
		// PosB++;//If (A_2 = 0 and B_2 = 1) or (A_2 = 1 and B_2 = 0)
	// else
		// PosB--;//If (A_2 = 0 and B_2 = 0) or (A_2 = 1 and B_2 = 1)
// }
5

My version:

 void UpdateA()
 {
	 if ((PIND & MASK_A_1) ^ (PIND & MASK_B_1))   
              --PosA;//If (A_1 = 0 and B_1 = 0) or (A_1 = 1 and B_1 = 1)                    
	  else  
              PosA++;//If (A_1 = 0 and B_1 = 1) or (A_1 = 1 and B_1 = 0)		
 }

Pre decrement is faster then post decrement and post increment is faster than pre increment on AVRs. Small difference but always. I also removed negation as it was unnecessary operation. If you will use PosA variable only in one ISR and not in any other function you can declare it inside ISR as "static" and not volatile it will speed things up. If PosA will be > 0 and < 256 than use type "byte" not "int".

Other optimization ideas you can find on my site : OSKAR WICHA'S DIGITAL CONNECTION: Source code optimization tips for Atmel AVR programmers also Arduino and clones

Hope that helps.

Oskar Wicha

6

ok

how about replacing the volatile variables wit just one?

#define MASK_A_1 (1<<2)
#define MASK_A_2 (1<<3)
#define MASK_B_1 (1<<4)
#define MASK_B_2 (1<<5)
volatile int PosA = 0;
volatile int PosB = 0;
volatile byte Status = 0;
//volatile byte A_1 = 0;//Status of pin 4 H or L
//volatile byte A_2 = 0;//Status of pin 5 H or L
//volatile byte B_1 = 0;//Status of pin 4 H or L
//volatile byte B_2 = 0;//Status of pin 5 H or L

void Setup45()
{ 
//	if (PIND & MASK_A_1)//replace attachInterrupt(0, UpdateA, CHANGE);
//		A_1 = 1;
//	if (PIND & MASK_A_2)//replace attachInterrupt(1, UpdateB, CHANGE);
//		A_2 = 1;	
//	if (PIND & MASK_B_1)
//		B_1 = 1;
//	if (PIND & MASK_B_2)
//		B_2 = 1;
        Status = PIND;		
	PCICR |= (1 << PCIE2);//Interrupt on pin change FROM PCINT16 to PCINT23 Digital
	PCMSK2 = (1 << PCINT18) | (1 << PCINT19) | (1 << PCINT20) | (1 << PCINT21); // digital pin 4 and 5
}

void setup()
{
	//in this case myPinX is the same as Bit
	DDRD &= ~(1 << 2);//Set pin 2 as input
	DDRD &= ~(1 << 3);//Set pin 3 as input
	DDRD &= ~(1 << 4);//Set pin 4 as input
	DDRD &= ~(1 << 5);//Set pin 5 as input
	DDRD |=  (1 << 6); //Set pin 6 as output
	PORTD |= B01111100;//Turn On 20KOhm resistors on 2,3,4,5,6 (HIGH)
        Setup45();
	//Is better to use ISR(PCINT2_vect) or attachInterrupt on pins 2, 3 ?
	// attachInterrupt(0, UpdateA, CHANGE);// encoder pin on interrupt 0 - pin 2
	// attachInterrupt(1, UpdateB, CHANGE);// encoder pin on interrupt 1 - pin 3
	
//	Serial.begin (115200);
//	Serial.println("start");

}

void loop()
{
//	Serial.println(PosA, DEC);// debug - remember to comment out
//	Serial.println(PosB, DEC);// debug - remember to comment out
}

ISR(PCINT2_vect)//is the function called by the interrupt on PCINT2
{
	if ((PIND & MASK_A_1) != (Status & MASK_A_1))//if digital PIN2 has changed
	{
		Status |= MASK_A_1;//Set new status without changing other bits
		if ((PIND & MASK_B_1) ^ (Status & MASK_B_1))                       
			--PosA;//If (A_1 = 0 and B_1 = 0) or (A_1 = 1 and B_1 = 1)
		else
			PosA++;//If (A_1 = 0 and B_1 = 1) or (A_1 = 1 and B_1 = 0)
	}
	if ((PIND & MASK_A_2) != (Status & MASK_A_2))//if digital PIN3 has changed
	{
		Status |= MASK_A_2;//Set new status without changing other bits
		if ((PIND & MASK_B_2) ^ (Status & MASK_B_2))                       
			--PosB;//If (A_2 = 0 and B_2 = 0) or (A_2 = 1 and B_2 = 1)
		else
			PosB++;//If (A_2 = 0 and B_2 = 1) or (A_2 = 1 and B_2 = 0)
	}
	
	if ((PIND & MASK_B_1) != (Status & MASK_B_1))//if digital PIN4 has changed
	{
		Status |= MASK_B_1;//Set new status without changing other bits
		if ((PIND & MASK_A_1) ^ (Status & MASK_A_1))                       
			--PosA;//If (A_1 = 0 and B_1 = 0) or (A_1 = 1 and B_1 = 1)
		else
			PosA++;//If (A_1 = 0 and B_1 = 1) or (A_1 = 1 and B_1 = 0)
	}
	if ((PIND & MASK_B_2) != (Status & MASK_B_2))//if digital PIN5 has changed
	{
		Status |= MASK_B_2;//Set new status without changing other bits
		if ((PIND & MASK_A_2) ^ (Status & MASK_A_2))                       
			--PosB;//If (A_2 = 0 and B_2 = 0) or (A_2 = 1 and B_2 = 1)
		else
			PosB++;//If (A_2 = 0 and B_2 = 1) or (A_2 = 1 and B_2 = 0)
	}
}

// void UpdateA()
// {
	// if ((PIND & MASK_A_1) ^ (PIND & MASK_B_1))                       
		// --PosA;//If (A_1 = 0 and B_1 = 0) or (A_1 = 1 and B_1 = 1)
	// else
		// PosA++;//If (A_1 = 0 and B_1 = 1) or (A_1 = 1 and B_1 = 0)
// }

// void UpdateB()
// {
	// if ((PIND & MASK_A_2) ^ (PIND & MASK_B_2))
		// --PosB;//If (A_2 = 0 and B_2 = 0) or (A_2 = 1 and B_2 = 1)
	// else
		// PosB++;//If (A_2 = 0 and B_2 = 1) or (A_2 = 1 and B_2 = 0)
// }
7

gvi70000:
Dosen't the code below turn on interrupt on pin change on digital pins 2,3,4,5?

PCICR |= (1 << PCIE2);//Interrupt on pin change FROM PCINT16 to PCINT23 Digital

PCMSK2 = (1 << PCINT18) | (1 << PCINT19) | (1 << PCINT20) | (1 << PCINT21); // digital pin 4 and 5


Forgot to add 2 and 3 pins

Yessss. But you never call the function Setup45, so what it does isn't really relevant.

I'm trying to use this to read the optical encoders of two servos
62.5rpm that will give me about 150 interrupts /second

It should easily handle 150 interrupts a second (ie. one every 6.66 mS). I think we worked out recently that the overhead of servicing an interrupt is around 3.5 uS, so you are getting them at about 2000 times more slowly than that.

By faster i was thinking Is there another way to write the if statements from the ISR(PCINT2_vect) in order to have less cpu cycles?

Not necessary at that rate of interrupts. Keep the code simple and readable. If it isn't working as you expect, the rate at which interrupts are serviced isn't the issue.

ISR(PCINT2_vect)//is the function called by the interrupt on PCINT2

{
if ((PIND & MASK_A_1) != (Status & MASK_A_1))//if digital PIN2 has changed

Why would you be in an interrupt routine if the pin hasn't changed?

8

OscylO:
Pre decrement is faster then post decrement and post increment is faster than pre increment on AVRs. Small difference but always.

Why do you say that?

This test sketch:

volatile int PosA = 0;

void test1 ()
  {
  PosA++;
  }
  
void test2 ()
  {
  ++PosA;
  }
 
void test3 ()
  {
  PosA--;
  }
  
void test4 ()
  {
  --PosA;
  }
 
void setup ()
{
  test1 ();
  test2 ();
  test3 ();
  test4 ();
}

void loop () {}

Generates this code for both additions:

void test1 ()
  {
  PosA++;
  a6:	80 91 00 01 	lds	r24, 0x0100
  aa:	90 91 01 01 	lds	r25, 0x0101
  ae:	01 96       	adiw	r24, 0x01	; 1
  b0:	90 93 01 01 	sts	0x0101, r25
  b4:	80 93 00 01 	sts	0x0100, r24
  }
  
void test2 ()
  {
  ++PosA;
  b8:	80 91 00 01 	lds	r24, 0x0100
  bc:	90 91 01 01 	lds	r25, 0x0101
  c0:	01 96       	adiw	r24, 0x01	; 1
  c2:	90 93 01 01 	sts	0x0101, r25
  c6:	80 93 00 01 	sts	0x0100, r24
  }

Absolutely the same machine code.

And for the subtractions:

void test3 ()
  {
  PosA--;
  ca:	80 91 00 01 	lds	r24, 0x0100
  ce:	90 91 01 01 	lds	r25, 0x0101
  d2:	01 97       	sbiw	r24, 0x01	; 1
  d4:	90 93 01 01 	sts	0x0101, r25
  d8:	80 93 00 01 	sts	0x0100, r24
  }
  
void test4 ()
  {
  --PosA;
  dc:	80 91 00 01 	lds	r24, 0x0100
  e0:	90 91 01 01 	lds	r25, 0x0101
  e4:	01 97       	sbiw	r24, 0x01	; 1
  e6:	90 93 01 01 	sts	0x0101, r25
  ea:	80 93 00 01 	sts	0x0100, r24
{
  test1 ();
  test2 ();
  test3 ();
  test4 ();
}
  ee:	08 95       	ret

Again, the same.

It makes a difference for more complex situations, but not for simple adding or subtracting of simple variables outside an expression.

So you don't need to trouble your mind about remembering which one to use in situations like that.

9

In this simple example you are right. But when incrementing or decrementing variable as loop counter,
pre-decrementing of variables gives usually the most efficient code. Predecrement and post-increment is more efficient because branches are depending on the flags after decrement.

char counter = 100; /* Declare loop counter variable*/
// LDI  R16,100 ; Init variable
do 
{
} while(--counter); /* Decrement counter and test for zero*/
?0004:DEC   R16 ; Decrement
// BRNE  ?0004 ; Branch if not equal

When using pointers it is also better to use predecrement and postincrement performance wise :

char *pointer1 = &table[0];
char *pointer2 = &table[49];
*pointer1++ = *--pointer2;

This generates the following Assembly code:

LD R16,-Z ; Pre-decrement Z pointer and load data 
ST X+,R16 ; Store data and post increment
10

Why would you be in an interrupt routine if the pin hasn't changed?

Is the another way of knowing which pin has change?

11

OscylO:
In this simple example you are right. But when incrementing or decrementing variable as loop counter,
pre-decrementing of variables gives usually the most efficient code. Predecrement and post-increment is more efficient because branches are depending on the flags after decrement.

Yes, but these do different things:

 do 
  {
  }
  while(--counter); /* Decrement counter and test for zero*/

 do 
  {
  }
  while(counter--); /* Decrement counter and test for zero*/

It may seem on the face of it that you have generated more efficient code, but you are doing different things. Try this sketch:

volatile byte counter; /* Declare loop counter variable*/

void setup ()
{

  Serial.begin (115200);
  
  counter = 100;
  do 
  {
  }
  while(--counter); /* Decrement counter and test for zero*/
  Serial.println (counter, DEC);

  counter = 100; 
  do 
  {
  }
  while(counter--); /* Decrement counter and test for zero*/
  Serial.println (counter, DEC);
}

void loop () {}

The output is:

0
255

So, in chasing efficiency, you are getting different results. Is that what you want?

OscylO:
When using pointers it is also better to use predecrement and postincrement performance wise :

char *pointer1 = &table[0];

char *pointer2 = &table[49];
*pointer1++ = *--pointer2;




This generates the following Assembly code:

LD R16,-Z ; Pre-decrement Z pointer and load data
ST X+,R16 ; Store data and post increment

It doesn't generate any such thing. Are you actually testing this before posting?

This sketch:

void setup ()
{
volatile char table [100];

  volatile char *pointer1 = &table[0];
  volatile char *pointer2 = &table[49];
  *pointer1++ = *--pointer2;

  volatile char a, b;
  
  a = *pointer1;
  b = *pointer2;
}

void loop () { }

Generates (for setup):

void setup ()
  a6:	df 93       	push	r29
  a8:	cf 93       	push	r28
  aa:	cd b7       	in	r28, 0x3d	; 61
  ac:	de b7       	in	r29, 0x3e	; 62
  ae:	c6 56       	subi	r28, 0x66	; 102
  b0:	d0 40       	sbci	r29, 0x00	; 0
  b2:	0f b6       	in	r0, 0x3f	; 63
  b4:	f8 94       	cli
  b6:	de bf       	out	0x3e, r29	; 62
  b8:	0f be       	out	0x3f, r0	; 63
  ba:	cd bf       	out	0x3d, r28	; 61
{
volatile char table [100];

  volatile char *pointer1 = &table[0];
  volatile char *pointer2 = &table[49];
  *pointer1++ = *--pointer2;
  bc:	8b a9       	ldd	r24, Y+51	; 0x33
  be:	8b 83       	std	Y+3, r24	; 0x03

  volatile char a, b;
  
  a = *pointer1;
  c0:	8c 81       	ldd	r24, Y+4	; 0x04
  c2:	89 83       	std	Y+1, r24	; 0x01
  b = *pointer2;
  c4:	8b a9       	ldd	r24, Y+51	; 0x33
  c6:	8a 83       	std	Y+2, r24	; 0x02
}
  c8:	ca 59       	subi	r28, 0x9A	; 154
  ca:	df 4f       	sbci	r29, 0xFF	; 255
  cc:	0f b6       	in	r0, 0x3f	; 63
  ce:	f8 94       	cli
  d0:	de bf       	out	0x3e, r29	; 62
  d2:	0f be       	out	0x3f, r0	; 63
  d4:	cd bf       	out	0x3d, r28	; 61
  d6:	cf 91       	pop	r28
  d8:	df 91       	pop	r29
  da:	08 95       	ret

There is nothing like this in that:

LD R16,-Z ; Pre-decrement Z pointer and load data 
ST X+,R16 ; Store data and post increment

Look, as a general rule (and this applies to the original poster too): don't try to outsmart the compiler. Keep your code simple and readable. Don't randomly change post-increment to pre-increment just to squeeze out another machine cycle. The compiler is written by smart people. If it is sensible to do so, it will probably do it. Changing around the way you increment things can introduce subtle bugs (like I illustrated above, where the number of iterations in the loop changes).

Unless you are writing highly time-critical code, the time you save (if any) is going to be wasted by the exta debugging time you spend.

12

gvi70000:

Why would you be in an interrupt routine if the pin hasn't changed?

Is the another way of knowing which pin has change?

Have one ISR per pin? How many pins do you have generating interrupts? You are trying to save time, right? So why add an extra test in when you don't have to?

13

I know how post and pre operators work and that those loops will execute different number off times (first 100 times and second 101 times). I do not say that who ever reads this thread should change all their loops to use predecrement. When postdecrement is changed to preincrement in expressions without any other modifications to code it will often introduce logical changes to application. When I write code I try to use predecrement and post increment when I can and it is not complicating my code in comparison to code using postdecrement or preincrement.

My code examples are from Atmel document named : AVR035: Efficient C Coding for AVR.
It contains a lot of good tips for efficient code.

Oskar Wicha

14

Pins 2, 3, 4 and 5 will generate interrupts
i use if ((PIND & MASK_A_1) != (Status & MASK_A_1))//if digital PIN2 has changed in order to determine witch pin has changed
from the last call of ISR

15

OscylO:
My code examples are from Atmel document named : AVR035: Efficient C Coding for AVR.
It contains a lot of good tips for efficient code.

And I'll read that soon, looks interesting, but... The native code generated by the compiler is going to be largely dependent upon which compiler is selected first and then which optimization options are selected. The standard Arduino environment does not let you change it, but there are many, many very specific optimizations which can be employed to make code more efficient, often at the cost of code compactness. Just saying a pre vs post increment or decrement is faster under all circumstances is not accurate.

16

The native code generated by the compiler is going to be largely dependent upon which compiler is selected first and then which optimization options are selected. The standard Arduino environment does not let you change it, but there are many, many very specific optimizations which can be employed to make code more efficient, often at the cost of code compactness.

I agree with that 100%. Because of that I use by own build process with compiler and linker options that I want.

Just saying a pre vs post increment or decrement is faster under all circumstances is not accurate.

All depends as with almost everything. But I never found reliable source saying that post decerement is faster than pre decrement or that pre increment is faster than post increment. There are cases that assembler code is that same so the performance also. Of course this is all on AVRs and not on different uC.

Oskar Wicha

17

gvi70000:
Pins 2, 3, 4 and 5 will generate interrupts
i use if ((PIND & MASK_A_1) != (Status & MASK_A_1))//if digital PIN2 has changed in order to determine witch pin has changed from the last call of ISR

I've used the following approach to count rising edges for 5 pins on Port C:

int pulseCount[5] = 0;

ISR(PCINT1_vect) {
  static byte oldPins = 0;                       // Saves pin status between interrupts
  byte newPins, setPins;
  newPins = PINC & 0x1F;                         // Get current status of all 5 pins
  setPins = (oldPins ^ newPins) & newPins;       // Bitmask of newly set pins (rising edge)
  for (byte n = 0; n < 5; n++) {                 // For each pin ...
    if (setPins & _BV(n))                        //   if pin is newly set...
      pulseCount[n]++;                           //   increment pulse count
  }
  oldPins = newPins;                             // Update saved pin status
}
18

This is my version of your code cjands40.

Changes made:
unsigned int instead of int - if not using negative values why use type that supports it
variable changed to constants when posible - if value will not change why use variable
loop unwinding - bigger code size but faster code execution

I didn't compile this code but it should by ok.

unsigned int pulseCount[5] = 0;

ISR(PCINT1_vect) {
  static byte oldPins = 0;                                 // Saves pin status between interrupts
  const byte newPins = PINC & 0x1F;                         // Get current status of all 5 pins
  const byte setPins = (oldPins ^ newPins) & newPins;   // Bitmask of newly set pins (rising edge)
  
  //   if pin is newly set...
  //   increment pulse count
  if (setPins & _BV(0)) pulseCount[0]++;       
  if (setPins & _BV(1)) pulseCount[1]++;       
  if (setPins & _BV(2)) pulseCount[2]++;       
  if (setPins & _BV(3)) pulseCount[3]++;                               
  if (setPins & _BV(4)) pulseCount[4]++;                           
 
  oldPins = newPins;                             // Update saved pin status
}

Cheers