Help with ISR's and Interrupts

I am working on a problem that I have only partially solved, and the solution generates more issues than it fixes, I suspect.

I am using Ben Buxton’s Rotary.H library for a rotary encoder, and have used two hardware interrupts for the encoder, rather than the Pin Change interrupts that seem to be the norm for this library-I got it working on my ATmega2561, with the following code:

#include <Rotary.h>

Rotary r = Rotary(20, 21);

const byte encoderbutton = 22;
const byte button2 = 23;
const byte button3 = 24;
const byte button4 = 25;

int Encoderbutton = 0;
int Button2 = 0;
int Button3 = 0;
int Button4 = 0;

void setup() {
  Serial.begin(9600);
  Serial.println("started the console");
  EICRA |= (1 << EICRA);
  EIMSK |= (1 << INT2) | (1 << INT3);
  sei();

  pinMode(encoderbutton, INPUT);
  pinMode(button2, INPUT_PULLUP);
  pinMode(button3, INPUT_PULLUP);
  pinMode(button4, INPUT_PULLUP);
}

void loop() {

Encoderbutton = digitalRead(encoderbutton);
Button2 = digitalRead(button2);
Button3 = digitalRead(button3);
Button4 = digitalRead(button4);

if (Encoderbutton == LOW) {
    // turn LED on:
    Serial.println("encoderbutton pushed!");
  } else {
    // turn LED off:
    //Serial.println("");
  }

if (Button2 == LOW) {
    // turn LED on:
    Serial.println("Button 2 pushed");
  } else {
    // turn LED off:
   
  }

  if (Button3 == LOW) {
    // turn LED on:
    Serial.println("Button 3 pushed");
  } else {
    // turn LED off:
    //Serial.println("");
  }

  if (Button4 == LOW) {
    // turn LED on:
    Serial.println("Button 4 pushed");
  } else {
    // turn LED off:
   // Serial.println("");
  }
}

ISR(INT2_vect) {
  unsigned char result = r.process();
  if (result == DIR_NONE) {
    // do nothing
  }
  else if (result == DIR_CW) {
    Serial.println("ClockWise");
  }
  else if (result == DIR_CCW) {
    Serial.println("CounterClockWise");
  }
}
 ISR(INT3_vect) {
  unsigned char result = r.process();
  if (result == DIR_NONE) {
    // do nothing
  }
  else if (result == DIR_CW) {
    Serial.println("ClockWise");
  }
  else if (result == DIR_CCW) {
    Serial.println("CounterClockWise");
  }
}

This actually works exactly as it’s expected to work, however if you remove one of the ISR’s, then turning the encoder will cause a reset of the Arduino and we go back to the top of the loop.

Is this normal for hardware interrupt pins? It would be very difficult to go back now and pick up a software interrupt, but I could if I absolutely had to do so.

I am only concerned about it due to the fact I plan to use the encoder to increment and deincrement a variable, but if I need two ISR’s I am not sure how it will impact the rest of the code?

if you remove one of the ISR's,

Why would you do that ?

By the way, Serial.print() uses interrupts and they are disabled when in an ISR. Can you see a problem ?

I note that none of your variables used in the ISRs are declared volatile but they are passed to other functions which can cause problems.

Are you sure about this:

EICRA |= (1 << EICRA);

I am using the same code for a rotary encoder in my UNO project (both hardware interrupts). It works great and, it’s fast! This is my ISR and the main line code. Maybe you can find something useful.

Based on my understanding, using Serial.print in the ISR violates the principle of keeping the ISR brief and uncomplicated.

// interrupt variables
int IRQActivity = 0;            //  IRQ activity flag
volatile int IRQcounter = 0;  //  Encoder direction value, set by 'rotate' ISR

/*
      After clearing any previous encoderTurned value, check
      the count value returned from the ISR to see if the
      rotary encoder has moved. If it has, then a new IRQ has
      been recognized and encoderTurned will be loaded with
      and hold the direction value for one scan only.
  */
  encoderTurned = 0;  // Reset previous turned value.
  if (IRQActivity != IRQcounter) {
    if (IRQcounter > IRQActivity)  encoderTurned = DIR_CW;
    if (IRQcounter < IRQActivity) encoderTurned = DIR_CCW;
    IRQActivity = IRQcounter; // reset one-shot value to be ready for next IRQ
  }
  /*

/* Interrupt Service Routine for rotary encoder:
   An interrupt is generated any time either of
   the rotary inputs change state.
*/
void rotate() {
  byte result = rotary.process();
  if (result == DIR_CW) {
    IRQcounter++;
  } else if (result == DIR_CCW) {
    IRQcounter--;
  }
}

using Serial.print in the ISR violates the principle of keeping the ISR brief and uncomplicated.

and is not guaranteed to work because interrupts are disabled.

s

however if you remove one of the ISR's, then turning the encoder will cause a reset of the Arduino and we go back to the top of the loop.

This is because you have enabled an interrupt without a corresponding interrupt service routine(ISR).

I plan to use the encoder to increment and deincrement a variable, but if I need two ISR's I am not sure how it will impact the rest of the code?

I do not use that library, so I'm not familiar with the details, and whether or not there is the option to use one interrupt. Using one interrupt instead of two will cut the resolution in half. For menus, this can be used to align the detents with the counts.

cattledog: Using one interrupt instead of two will cut the resolution in half.

I just tried it by commenting out one of the attach.interrupts. You won't like it, it only works infrequently. Totally unusable as a menu navigator. I think this is due to the concept behind it, it's not just counting pulses, it wants to see the transitions in a specific order to determine rotation.

Check out the PJRC Encoder Library.

I am aware of the serial.print() issues inside of ISRs-the above sketch is entirely designed to test the hardware and make sure it’s working before I start writing actual commands. I won’t be using serial or other interrupt-driven commands inside the ISR’s in the actual command code.

I’ve included the example code from the library here, so that people can see how the setup is supposed to look on the Uno, and why I was concerned about the twin ISR’s…as you can see, only a single ISR is used in the example, because PCINT2_vect covers PCINT18 AND PCINT19, so no need for two routines. Because I used hardware interrupts, I apparently need to cover both interrupts-so two ISR’s. At least, that’s what I’m picking up so far…hopefully correct?

/*
    Rotary Encoder - Interrupt Example
    
    The circuit:
    * encoder pin A to Arduino pin 2
    * encoder pin B to Arduino pin 3
    * encoder ground pin to ground (GND)
*/

#include <Rotary.h>

Rotary r = Rotary(2, 3);

void setup() {
  Serial.begin(9600);
  PCICR |= (1 << PCIE2);
  PCMSK2 |= (1 << PCINT18) | (1 << PCINT19);
  sei();
}

void loop() {

}

ISR(PCINT2_vect) {
  unsigned char result = r.process();
  if (result == DIR_NONE) {
    // do nothing
  }
  else if (result == DIR_CW) {
    Serial.println("ClockWise");
  }
  else if (result == DIR_CCW) {
    Serial.println("CounterClockWise");
  }
}

I am going to look at Paul’s library (it feels weird to see PJRC associated with stuff other than 8051’s these days, but awesome at the same time!) and see if I can get the library to work with my hardware too. If so, it won’t be the first library change in this project!

I am aware of the serial.print() issues inside of ISRs-the above sketch is entirely designed to test the hardware and make sure it's working before I start writing actual commands

So you are going to make sure that it works by using functions that are not guaranteed to work.

That sounds like a non sequitur to me

Yes. That is exactly what I did, because that's exactly how the examples are set up. I understand that it is not a suitable use of the ISR.

Xnke: Yes. That is exactly what I did, because that's exactly how the examples are set up. I understand that it is not a suitable use of the ISR.

Which examples are you referring to ?

The examples included with the library. I have been working with a different library this week, although it doesn't seem to be any better or worse. It does take care of all the interrupt issues internally-I don't have to think about them at all.

The examples included with the library.

Bear in mind that the majority of us will not have the examples to look at