Getting stuck in Timer1 interrupt loop

I am attempting to run 2 interrupts at the same time, one blinks an LED at 1Hz and another detects when an encoder is turned. On start up the LED blinks perfectly fine, but as soon as the encoder is turned, it falls into an infinite loop within the LED interrupt. Any help as to why this happens would be greatly appreciated.

int counter = 0;
int previous;
int current;
int DT;
bool led = 0;

void setup() {
  pinMode(9, OUTPUT);
  pinMode(2, INPUT);
  pinMode(7, INPUT);
  pinMode(5, OUTPUT);

  Serial.begin(9600);

  previous = digitalRead(2);

  digitalWrite(11, HIGH);
  digitalWrite(12, LOW);

  //Timer Setup
  
  attachInterrupt(digitalPinToInterrupt(2), KNOB, RISING);

  //set timer1 interrupt at 2Hz
  TCCR1A = 0;// set entire TCCR1A register to 0
  TCCR1B = 0;// same for TCCR1B
  TCNT1  = 0;//initialize counter value to 0
  // set compare match register for 1hz increments
  OCR1A = 31249;// = (16*10^6) / (1*1024) - 1 (must be <65536)
  // turn on CTC mode
  TCCR1B |= (1 << WGM12);

  // Set CS10 and CS12 bits for 1024 prescaler
  TCCR1B |= (1 << CS12);

  // enable timer compare interrupt
  TIMSK1 |= (1 << OCIE1A);

}

ISR(TIMER1_COMPA_vect) {
  TCNT1  = 0;
  led ^= 1;
  digitalWrite(5, led);
  
  Serial.println("LED");
  
}

void KNOB() {
  Serial.println("Knob");
  current = digitalRead(2);
  DT = digitalRead(7);

  if (digitalRead(7) != current & counter != 0) {
    counter += -10;
  }
  if (digitalRead(7) == current & counter != 250) {
    counter += 10;
  }

  previous = current;
  Serial.println(counter);
}

void loop() {

  analogWrite(9, counter);

}

Remove the Serial.prints from within the ISR.

You should never use Serial.print within an ISR. Serial.print itself uses interrupts, an when used within an ISR it can block the whole system.
Be aware that ISR'S are NOT normal functions and need special attention.

I do not believe this does what you think it does (note '&' operator and understand how it is different from '&&').

I believe that & and && is identical for two bool operands,
nevertheless it is better to use the boolean && if that is the intention.

& as well as && have a lower precedence than comparison operators,
so it does not change the meaning of the expression.

https://en.cppreference.com/w/cpp/language/operator_precedence

const uint8_t val1_targetValue = 1;
const uint8_t val2_targetValue = 2;

void setup() {
  Serial.begin(115200);
  for (uint8_t i = 1; i < 3; i++) {
    for (uint8_t j = 1; j < 3; j++) {
      if (checkFor(i, j)) {
        Serial.println(F("different result"));
      } else {
        Serial.println(F("same result"));
      }
    }
  }
}

bool checkFor(uint8_t v1, uint8_t v2) {
  bool var1 = (v1 == val1_targetValue && v2 == val2_targetValue);
  bool var2 = (v1 == val1_targetValue & v2 == val2_targetValue);
  Serial.print(v1);
  Serial.print(F(" == "));
  Serial.print(val1_targetValue);
  Serial.print(F(" && "));
  Serial.print(v2);
  Serial.print(F(" == "));
  Serial.print(val2_targetValue);
  Serial.print(F(" gives "));
  Serial.println(var1 ? "true" : "false");
  Serial.print(v1);
  Serial.print(F(" == "));
  Serial.print(val1_targetValue);
  Serial.print(F(" &  "));
  Serial.print(v2);
  Serial.print(F(" == "));
  Serial.print(val2_targetValue);
  Serial.print(F(" gives "));
  Serial.println(var2 ? "true" : "false");
  return var1 != var2;
}

void loop() {}
1 == 1 && 1 == 2 gives false
1 == 1 &  1 == 2 gives false
same result
1 == 1 && 2 == 2 gives true
1 == 1 &  2 == 2 gives true
same result
2 == 1 && 1 == 2 gives false
2 == 1 &  1 == 2 gives false
same result
2 == 1 && 2 == 2 gives false
2 == 1 &  2 == 2 gives false
same result

The usage of & like this will trigger a warning:

Somewhere\oneortwoampersands.ino:19:19: warning: suggest parentheses around comparison in operand of '&' [-Wparentheses]
   bool var2 = (v1 == val1_targetValue & v2 == val2_targetValue);
                ~~~^~~~~~~~~~~~~~~~~~~

The removal of the Serial.prints doesnt solve the problem. Anytime the encoder interrupt occurs, the Timer interrupt gets stuck in a loop.

Not posting the new code, does not allow any comments on that.

analogWrite on pin 9 uses timer 1. This interferes with your usage of timer 1.
As soon as counter becomes greater than 0, it crashes.

And I told you already that ISR's are not normal functions. This is also important if you use variables inside AND outside the ISR. You have to deal with atomic reading and volatile variables.

Thank you this makes sense. Is there a way to obtain a 2Hz Frequency using Timer 0 and Timer 2 instead of Timer 1 for the interrupts? Or am I constrained to Timer 0 and Timer 2 (in other words higher frequencies) when analogWrite is necessary to be used?

To obtain a 2Hz frequency no timer is necessary in my opinion. This can be done with millis in loop().
Or you use the MoToTimebase class of my MobaTools lib, if you don't want to bother with millis directly.

1 Like

I have found in the past the Timer2 library to work well MsTimer2 - Arduino Reference.