Built-in Analog Comparator Issue

Projects Programming
1

Hi everybody!

I'm designing a frequency detector for guitar.
It works great!
Now I'm trying to use the built-in analog comparator instead of using an external one to save even more PCB space!
The idea is to use the input capture feature on timer 1.
My problem is that for some reason the Comparator doesn't seem to handle frequencies higher than 120Hz or so. I'm using software to do different stuff if the comparator output is on a rising edge, or on a falling edge.

Here is a stripped down version of my code that I used for testing.
Maybe it's my way of adding software hysteresis that is weird?
My audio is biased around 2.5v going to the positive input, and 2.5v is applied on the negative input of the comparator.

byte Hysteresis=0;

void setup() {
  
  pinMode(6,INPUT);  //AIN0 positive input of comparator
  pinMode(7,INPUT); //AIN1 negative input of comparator
  pinMode(13,OUTPUT); //Comparator output visualisation for oscilloscope.
  
  ADCSRB |= (1<<ACME); //AIN1 to negative input
  
  ACSR &= ~(1<<ACD);  // No Disabling
  ACSR &= ~(1<<ACBG); //no bandgap
  ACSR |= (1<<ACIE); //enable interrupt
  //ACSR |= (1<<ACIC); //enable input capture from comp (+ICIE1)
  ACSR &= ~(1<<ACIS1); //both at 0: rising AND falling edge detector
  ACSR &= ~(1<<ACIS0); 
  
  DIDR1 &= ~(1<<AIN1D); // enables the digital input pins buffers
  DIDR1 &= ~(1<<AIN0D);

  //TIMSK1 |= (1<<ICIE1); // enable input capture
  
  
  
  TCCR2A &= ~(1 << COM2A1); //
  TCCR2A &= ~(1 << COM2A0); //these 2 bits disable the OC1A when not in PWM mode
  TCCR2A &= ~(1 << COM2B1); //
  TCCR2A &= ~(1 << COM2B0); //these 2 bits disable OC1B when not in PWM mode
  TCCR2A &= ~(1 << WGM21); //all 3 WGM bits to 0 is normal mode
  TCCR2A &= ~(1 << WGM20); 
  
 
  TCCR2B &= ~(1 << WGM22); 
  TCCR2B &= ~(1<< CS22); //  these 3 next bits set pre-scaler to 1
  TCCR2B &= ~(1 << CS21); //
  TCCR2B |= (1 << CS20); //
  
  TIMSK2 &= ~(1 << OCIE1B); //disable output compare B Interrupt
  TIMSK2 &= ~(1 << OCIE1A); //disable output compare A Interrupt
  TIMSK2 |= (1 << TOIE1); //enable overflow interrupt
  
}

ISR (ANALOG_COMP_vect) {
  if (Hysteresis==0) {
    if ((ACSR >> ACO) & 1) {
      Hysteresis=1;
      PORTB |= (1<<5); //  D13 high
    } else {
      Hysteresis=1;
      PORTB &= ~(1<<5); //   D13 low
    }
  } 
}

ISR (TIMER2_OVF_vect) {
  if (Hysteresis!=0) {
    Hysteresis++;
  }
  else if (Hysteresis==4) {
    Hysteresis==0;
  }
}

void loop() {
}

The comparator output visualized on pin D13 looks ok for the 5 or 6 first low notes on the guitar.
When I go higher, I either get a high state, a low state, or a frequency less than 120Hz.
I'd like the frequency range to go from 39Hz to 1300Hz.

What do you think is the problem?

2

Oups! Just spotted a mistake!
In the timer2 overflow Interrupt Service Routine, it shouldn't be "else if" but just another "if". I'll test it and let you know.

And it shouldn't be Hysteresis==0; but Hysteresis=0 of course!

3

I'm a bit confused by the lack of any Timer1 usage. Isn't "Input Capture" a feature of Timer1?

I also don't understand why hysteresis is needed. What problem does it solve?

4

Hi!
The code posted in the original post is a lighter version of my original code in order to test the analog comp interrupt. Therefore there is no use of Timer1 in this version of the code. I use the Timer2 to count a certain amount of overflows before allowing the analog comparator interrupt service routine to do anything, thus it does some debouncing.

Hysteresis (debounce) is needed because my analog comp interrupt would trigger many times each time my audio signal crosses the threshold defined by a 2 resistor divider on negative input.
Also I want my interrupt to do different stuff if it's a falling edge or a rising edge that is detected.

Now it works like a charm and you'll see the code below.
It is to be noted (while I am not sure it is always the case but it seems to be) that the Input Capture Interrupt Service Routine is being computed before the Analog Comparator Interrupt Service Routine.

Here are the new Analog Comparator and Timer2 overflow ISRs that work.
The Timer2 has to overflow 9 times (144 microseconds total) before the debounce time is over.

ISR (TIMER2_OVF_vect) {
  if (Hysteresis!=0) {
    Hysteresis++;
    if (Hysteresis==9) {
      Hysteresis=0;
    }
  }
}

ISR (ANALOG_COMP_vect) {
  if (Hysteresis==0) {
  Hysteresis=1;
    if ((ACSR >> ACO) & 1) {
      PORTB |= (1<<5); //  D13 high
    } else {
      PORTB &= ~(1<<5); //   D13 low
    }
  } 
}

Thanks for reading and sorry if I wasted anybody's time with my faulty code that I could clearly debug by my self.