Problem understanding interrupts and timers

Hello everyone, I am having a hard time trying to understand how interrupts and timers work on the Arduino Uno R3 (Atmega328p). Basically, I am using two microphone modules (https://www.amazon.com/Sensitivity-Microphone-Detection-Arduino-Raspberry/dp/B06WLHK6BY) with their digital pins and trying to detect when each one is triggered and figure out the time delay between the two using pin change interrupts and timers. The problem is that I have a variable mic_state which I am using in a switch statement inside an ISR(PCINT0_vect) function and according to the case I am starting a timer TCNT2 and later taking its value. However, it appears to me that I am unable to enter case ‘1’ of the switch statement, because I never get the “case 1, mic 8” string printed to the serial monitor. Is this normal behavior and is there something that looks completely wrong and out of place? I have attached the complete code to this post.

Sorry for the incompetent post, what I can say is that I usually read the forums more and rarely open topics. Any help would be greatly appreciated. :slight_smile:

ISR Function:

ISR (PCINT0_vect)
{
    uint8_t changedbits;


    changedbits = PINB ^ portbhistory;
    portbhistory = PINB;

    
    if(changedbits & (1 << PINB0))
    {
        /* PCINT0 changed */        
      switch (mic_state) {
			  case '0':
			    TCNT2 = 0;           //restart timer
			    mic_state = '1';       //update state
			    mic_order[0] = '8';    //update mic order
			    //Serial.println("case 0, mic 8");
			  break;

			  case '1':
			    sound_time1 = TCNT2;   //save timer value1
			    mic_state = '2';         //update state
			    mic_order[1] = '8';    //update mic order
			    Serial.println("case 1, mic 8");
			  break;

			  case '2':
			    sound_time2 = TCNT2; //save timer value2
			    mic_state = '3';
			    mic_order[2] = '8';    //update mic order
			    //vector_update_flag=1;       //UPDATE VECTOR!!
			    //PORTD &= ~(1<<PIND2);
			  break;

			  default:
			    mic_state = '-1';
			  break;
		  }
    }
    else{
      mic_state = '0';
    }

setup() function:

void setup() {
  cli();
  
    Serial.begin(9600);
    mic_state = '0';
    
    DDRB &= ~((1 << DDB0) | (1 << DDB1) | (1 << DDB2)); // Clear the PB0, PB1, PB2 pin
    // PB0,PB1,PB2 (PCINT0, PCINT1, PCINT2 pin) are now inputs

    PORTB |= ((1 << PORTB0) | (1 << PORTB1) | (1 << PORTB2)); // turn On the Pull-up
    // PB0, PB1 and PB2 are now inputs with pull-up enabled
    
    PCICR |= (1 << PCIE0);     // set PCIE0 to enable PCMSK0 scan
    PCMSK0 |= (1 << PCINT0);   // set PCINT0 (digital pin 8) to trigger an interrupt on state change
    PCMSK0 |= (1 << PCINT1);   //digital pin 9
    PCMSK0 |= (1 << PCINT2);   //digital pin 10
    
    //SOUND TIMER2 SETUP
	  //TCCR2B = 4;              //timer2 divide by 64
	  TCCR2B |= (1 << CS22);
	  TIMSK2 = (1<<TOIE2);     //enable overflow ISR

    sei();

}

Uno_pin_registers.zip (5.96 KB)

Variables that can be changed inside an ISR should be declared as volatile. Maybe that’s your issue. Have a look at this page in the reference section... https://www.arduino.cc/reference/en/language/variables/variable-scope--qualifiers/volatile/

Also, serial.println() inside an ISR isn't a good idea. Even if it works it'll take too long. So change global variables in the ISR and do serial.println in a main loop.

Never (!) use any method if the Serial object in an ISR. The Serial object depends on interrupts to do it's job. But inside an ISR interrupts are disabled so it cannot do it's job correctly.

In the Arduino world you should never use the String class as it rapidly fragments the available memory (RAM) and sooner than you like it you'll be out of memory although there's still enough of it, just unavailable.

   mic_state = '-1';

I don't know what the compiler does with that, for sure it's not what you probably expected. mic_state is either '-' or '1' after that but it's definitely not -1 (or 255).

Also I'm quite sure that the state flow is not implemented correctly as mic_state is really handled only if all three pins have changed at exactly the same time (check the else clauses).

Don't use characters for state values.

    mic_state = '0';

Use integers instead:

    mic_state = 0;

Use byte, they are 8 bit, ints are 16

Thank you all for your input.

As advised I have declared all my variables as volatile and made the mic_state variable byte.

volatile uint8_t portbhistory = 0xFF;
volatile byte mic_state;
volatile char mic_order [3];
volatile boolean isMic8InCase0 = false;
volatile boolean isMic8InCase1 = false;
volatile boolean isMic9InCase0 = false;
volatile boolean isMic9InCase1 = false;
volatile unsigned int sound_time1; //sound time of first triggered microphone
volatile unsigned int sound_time2; //sound time of second triggered microphone

I should have mentioned in the beginning that I have both of my two microphones connected on PORTB, meaning digital pins 8 and 9 and because of this I am using only one ISR() function which takes the PCINT0_vect, therefore the whole function looks like this:

ISR (PCINT0_vect)
{
    uint8_t changedbits;


    changedbits = PINB ^ portbhistory;
    portbhistory = PINB;

    
    if(changedbits & (1 << PINB0))
    {
        /* PCINT0 changed */        
      switch (mic_state) {
			  case 0:
			    TCNT2 = 0;           //restart timer
			    mic_state = 1;       //update state
			    mic_order[0] = '8';    //update mic order
			    isMic8InCase0 = true;
			  break;

			  case 1:
			    sound_time1 = TCNT2;   //save timer value1
			    mic_state = 2;         //update state
			    mic_order[1] = '8';    //update mic order
			    isMic8InCase1 = true;
			  break;

			  case 2:
			    sound_time2 = TCNT2; //save timer value2
			    mic_state = 3;
			    mic_order[2] = '8';    //update mic order
			    //vector_update_flag=1;       //UPDATE VECTOR!!
			    //PORTD &= ~(1<<PIND2);
			  break;

			  default:
			    mic_state = -1;
			  break;
		  }
    }
    else{
      mic_state = 0;
    }
    
    if(changedbits & (1 << PINB1))
    {
        /* PCINT1 changed */
        switch (mic_state) {
			  case 0:
			    TCNT2 = 0;           //restart timer
			    mic_state = 1;       //update state
			    mic_order[0] = '9';    //update mic order
			    isMic9InCase0 = true;
			  break;

			  case 1:
			    sound_time1 = TCNT2;   //save timer value1
			    mic_state = 2;         //update state
			    mic_order[1] = '9';    //update mic order
			    isMic9InCase1 = true;
			  break;

			  case 2:
			    sound_time2 = TCNT2; //save timer value2
			    mic_state = 3;
			    mic_order[2] = '9';    //update mic order
			    //vector_update_flag=1;       //UPDATE VECTOR!!
			    //PORTD &= ~(1<<PIND2);
			  break;

			  default:
			    mic_state = -1;
			  break;
		  }
    }
    else{
      mic_state = 0;
    }

    /*if(changedbits & (1 << PINB2))
    {
        /* PCINT2 changed */
        switch (mic_state) {
			  case 0:
			    TCNT2 = 0;           //restart timer
			    mic_state = 1;       //update state
			    mic_order[0] = '10';    //update mic order
			  break;

			  case 1:
			    sound_time1 = TCNT2; //save timer value1
			    mic_state = 2;       //update state
			    mic_order[1] = '10';    //update mic order
			  break;

			  case 2:
			    sound_time2 = TCNT2; //save timer value2
			    mic_state = 3;
			    mic_order[2] = '10';    //update mic order
			    //vector_update_flag=1;       //UPDATE VECTOR!!
			    //PORTD &= ~(1<<PIND2);
			  break;

			  default:
			    mic_state = -1;
			  break;
		  }
    }
    else{
      mic_state = 0;
    }*/
}

I am wondering if the fact that both the microphones are on PORTB, might be the cause for the program not to enter “case 1” of the switch statement?

I have added volatile booleans which I set to true inside the switch in the ISR function and I am checking their state in the loop() function simply like this:

void loop() {
  if(isMic8InCase0){
    Serial.println("case 0 mic 8");
    isMic8InCase0 = false;
  }
  if(isMic8InCase1){
    Serial.println("case 1 mic 8");
    isMic8InCase1 = false;
  }
  if(isMic9InCase0){
    Serial.println("case 0 mic 9");
    isMic9InCase0 = false;
  }
  if(isMic9InCase1){
    Serial.println("case 1 mic 9");
    isMic9InCase1 = false;
  }
}

However, again I get only “case 0 mic 8” printed to the Serial Monitor, meaning that mic 8 was triggered first. “case 1 mic 8” should mean that mic 8 was triggered second, after mic 9, which I cannot get printed out. :confused: I have attached the current complete code to this post as well. :slight_smile:

Uno_pin_registers.zip (6.32 KB)

I am wondering if the fact that both the microphones are on PORTB, might be the cause for the program not to enter "case 1" of the switch statement?

No, as I already wrote: it's your else block.