does too many interrupt make arduino stop?

i'm testing with my encoder which have 6000 ppr.
i was recommened encoder library GitHub - PaulStoffregen/Encoder: Quadrature Encoder Library for Arduino
and now i want to analyze and extract partial code.

here is my code

int pin1 = 2;
int pin2 = 5;

#define IO_REG_TYPE      uint8_t
#define PIN_TO_BASEREG(pin)             (portInputRegister(digitalPinToPort(pin)))
#define PIN_TO_BITMASK(pin)             (digitalPinToBitMask(pin))
#define DIRECT_PIN_READ(base, mask)     (((*(base)) & (mask)) ? 1 : 0)

  volatile IO_REG_TYPE * pin1_register;
  volatile IO_REG_TYPE * pin2_register;
  IO_REG_TYPE            pin1_bitmask;
  IO_REG_TYPE            pin2_bitmask;
  uint8_t                state;
  int32_t                position;


void setup()
{
  Serial.begin(115200);
  
  pinMode(pin1, INPUT_PULLUP);
  pinMode(pin2, INPUT_PULLUP);

  pin1_register = PIN_TO_BASEREG(pin1);
  pin1_bitmask = PIN_TO_BITMASK(pin1);
  pin2_register = PIN_TO_BASEREG(pin2);
  pin2_bitmask = PIN_TO_BITMASK(pin2);
  position = 0;

  delayMicroseconds(2000);
  uint8_t s = 0;
  if (DIRECT_PIN_READ(pin1_register, pin1_bitmask)) s |= 1;
  if (DIRECT_PIN_READ(pin2_register, pin2_bitmask)) s |= 2;
  state = s;

  EICRA = (EICRA & 0xFC) | 0x01;
  EIMSK |= 0x01;
  EICRA = (EICRA & 0xF3) | 0x04;
  EIMSK |= 0x02;
}

unsigned long ptime = 0;

void loop()
{
  //if(millis() - ptime > 1000)
  {
    noInterrupts();
    Serial.println(position);
    interrupts();
    ptime = millis();
  }
}

ISR(INT0_vect) { encoderUpdate(); }
ISR(INT1_vect) { encoderUpdate(); }

void encoderUpdate()
{
    /*uint8_t p1val = DIRECT_PIN_READ(pin1_register, pin1_bitmask);
    uint8_t p2val = DIRECT_PIN_READ(pin2_register, pin2_bitmask);
    uint8_t s = state & 3;
    if (p1val) s |= 4;
    if (p2val) s |= 8;
    state = (s >> 2);
    noInterrupts();
    switch (s) {
      case 1: case 7: case 8: case 14:
        position++;
        return;
      case 2: case 4: case 11: case 13:
        position--;
        return;
      case 3: case 12:
        position += 2;
        return;
      case 6: case 9:
        position -= 2;
        return;
    }
    interrupts();*/

    noInterrupts();
    position++;
    interrupts();
}

the problem is when i rotate my encoder very fast speed, serial print doesn't work. i think arduino goes all stop.
i heard that if access too many byte in interrupt, it can cause corruption so i need to stop interrupt, but i don't think that my code too long to make corruption.

i think there is something fundamental problem that make stop arduino.
what's wrong with my code?

I suspect the main problem with your program is that you are trying to print much too frequently because you have commented out the 1 second time check.

Also you must have interrupts enabled for printing. Change your code so it takes a copy of the position and uses the copy for printing. Like this

    noInterrupts();
       copyOfPosition = position;
    interrupts();
    Serial.println(copyOfPosition);

And there is no need for noInterrupts() inside the ISR because interrupts are automatically paused while the ISR executes.

Finally (for now) you have not told us what you mean by "very fast speed"

...R

ok i change code like this.

change code to print when i stop rotating and send any word to get rotation position, so it doesn't matter with print frequency.

int pin1 = 2;
int pin2 = 5;

#define IO_REG_TYPE      uint8_t
#define PIN_TO_BASEREG(pin)             (portInputRegister(digitalPinToPort(pin)))
#define PIN_TO_BITMASK(pin)             (digitalPinToBitMask(pin))
#define DIRECT_PIN_READ(base, mask)     (((*(base)) & (mask)) ? 1 : 0)

  volatile IO_REG_TYPE * pin1_register;
  volatile IO_REG_TYPE * pin2_register;
  IO_REG_TYPE            pin1_bitmask;
  IO_REG_TYPE            pin2_bitmask;
  uint8_t                state;
  volatile int32_t                position;


void setup()
{
  Serial.begin(115200);
  
  pinMode(pin1, INPUT_PULLUP);
  pinMode(pin2, INPUT_PULLUP);

  pin1_register = PIN_TO_BASEREG(pin1);
  pin1_bitmask = PIN_TO_BITMASK(pin1);
  pin2_register = PIN_TO_BASEREG(pin2);
  pin2_bitmask = PIN_TO_BITMASK(pin2);
  position = 0;

  delayMicroseconds(2000);
  uint8_t s = 0;
  if (DIRECT_PIN_READ(pin1_register, pin1_bitmask)) s |= 1;
  if (DIRECT_PIN_READ(pin2_register, pin2_bitmask)) s |= 2;
  state = s;

  EICRA = (EICRA & 0xFC) | 0x01;
  EIMSK |= 0x01;
  EICRA = (EICRA & 0xF3) | 0x04;
  EIMSK |= 0x02;
}

void loop()
{
  if(Serial.available())
  {
    noInterrupts();
    char c = Serial.read();
    int32_t pos = position;
    interrupts();
    Serial.println(pos);
  }
}

ISR(INT0_vect) { encoderUpdate(); }
ISR(INT1_vect) { encoderUpdate(); }

void encoderUpdate()
{
    uint8_t p1val = DIRECT_PIN_READ(pin1_register, pin1_bitmask);
    uint8_t p2val = DIRECT_PIN_READ(pin2_register, pin2_bitmask);
    uint8_t s = state & 3;
    if (p1val) s |= 4;
    if (p2val) s |= 8;
    state = (s >> 2);
    switch (s) {
      case 1: case 7: case 8: case 14:
        position++;
        return;
      case 2: case 4: case 11: case 13:
        position--;
        return;
      case 3: case 12:
        position += 2;
        return;
      case 6: case 9:
        position -= 2;
        return;
    }
}

and very fast speed... may be 4 rotation per second, then it will make 4 * 6000 * 2 = 48000 pulse per second i think.

Why have you got Serial.read() inside noInterrupts() ?

And you have now completely changed the ISR code from what it was in your Original Post - why?

At 48,000 pulses per second there will be about 20 microsecs between pulses which does not leave a lot of time for a 16MHz Arduino to do other things. It will also be important to make the ISR code as short as possible.

...R

Robin2:
Why have you got Serial.read() inside noInterrupts() ?

that's because to remove Serial.available().

Robin2:
And you have now completely changed the ISR code from what it was in your Original Post - why?

not fully changed. i just remove comment and restore my origin code. but sorry for it.

then you mean 48,000 pulse is too short to do complete code?

sinsin63:
then you mean 48,000 pulse is too short to do complete code?

well at 50k ticks per seconds, your code has ~ 20 micro seconds to handle one tick.

with an Arduino at 16Mhz, a one clock cycle is 62,5 nano second, so in 20 micro seconds you can perform 320 "basic" low level assembly code instructions. that's what you have to play with.

sinsin63:
that's because to remove Serial.available().

Don't place it inside the noInterrupts() section

...R

Only explicitly disable interrupts to read or write volatile variables used by an ISR.
Always do this unless only a single byte is involved.

Never use serial or delay inside an ISR or with interrupts disabled.

All variables used to communicate between ISR and the rest of the program need to
be declared volatile.

This code:

  if(Serial.available())
  {
    noInterrupts();
    char c = Serial.read();
    int32_t pos = position;
    interrupts();
    Serial.println(pos);
  }

is confusing serial code with interrupt code, it should be more like:

  if(Serial.available())
  {
    Serial.read();  // discard the input, its just to trigger a response

    noInterrupts(); // read position in a critical section
    int32_t pos = position;
    interrupts();

    Serial.println(pos);  // respond
  }

Even though the guard with Serial.available() probably avoids deadlock, you should be
careful to keep your critical sections minimal - probably good to define each as a function
so the business logic of your sketch doesn't have to be scattered with irrelevancies like
"noInterrupts"

So I'd suggest:

int32_t read_position()
{
  noInterrupts(); // read position in a critical section
  int32_t pos = position;
  interrupts();
  return pos ;
}

bool triggered ()
{
  if (Serial.available())
  {
    Serial.read() ;
    return true ;
  }
  else
    return false ;
}

...


  // Nice clean business logic, using well-named functions,
  // implementation detail is hidden
  if (triggered ())
    Serial.println(read_position());  // respond

...

And yes, too many interrupts will jam the processor as any interrupt has higher priority than
the main body of the code.

wow, i think this code works fine right now, but i think may be i should be check more faster interrupt situtation in my project, but thanks for you guys!

i will remember your advice!

Instead of testing Serial.available(), just do a Serial.read() and compare with -1 it will be faster as available() requires some maths with modulo to count the number of bytes in the buffer and you don’t care about that number, you just want to read one byte if one is available. The function would look like this

inline bool triggered()
{
  return (Serial.read() != -1);
}

J-M-L:
Instead of testing Serial.available(), just do a Serial.read() and compare with -1 it will be faster as available() requires some maths with modulo to count the number of bytes in the buffer and you don’t care about that number, you just want to read one byte if one is available. The function would look like this

inline bool triggered()

{
  return (Serial.read() != -1);
}

ah! thanks! i can't imagine about this.

MarkT:
Only explicitly disable interrupts to read or write volatile variables used by an ISR.
Always do this.

There. Fixed it for you.