# Random and unpredictable analog comparator behaviour

I am working on a relatively "simple" project where I need to measure frequency of a sine wave that varies in amplitude and frequency. To simplify things, for now, I have only got a fixed frequency (27Hz) sine wave input (negative input of the comparator) which can only be varied in amplitude (using a potentiometer). The positive input of the comparator is set to Vcc/2. The output of the comparator is then fed into input capture register of the atmega2560 microcontroller to measure the frequency.

The problem is that at certain amplitudes of the input signal I get quite intense toggling (or sometimes dead bands) on the output which looks like this:

Where as the expected output should look something like this:

Things I have tried so far:

Using internal atmega2560's internal comparator. Using an external comparator. Introducing hysteresis using software and Schmitt trigger circuit. Tried various input setups, including fixed reference setup and data slicer setup. Trying different atmega2560's. Trying different clock speeds.

Some solutions were more stable than others, but none of them were anywhere near acceptable. I have settled with the most stable configuration so far:

With this setup, certain things improve/change the stability, however still nowhere near perfect:

Changing value of R5 to increase hysteresis. Removing C2 completely (no idea why). Touching wires on the breadboard (quite a few of them next to each other). Switching power supplies from external to USB and vice versa.

At this point, it's either noise, my DAC with which I am generating the sine wave or I am doing something very fundamental incorrectly. This circuit has worked for other people without any issues, so something must be wrong with my configuration or environment.

If anyone has any suggestions, I would greatly appreciate your time.

Here's my minimal source:

#include <avr/io.h>

void init(void);

void init(void) {
/* Setup comparator */
ACSR = (1 << ACIE) | (1 << ACIS1);
/* Initialize PORTD for PIND5 */
DDRD = 0x00;
PORTD = 0x00;
/* Enable global interrupts */
sei();
}

int main(void) {

init();

while (1) {}
}

ISR(ANALOG_COMP_vect) {

if (!(ACSR &  (1<<ACIS0))) { //comparator falling edge
/* Set PIND5 to 0V */
PORTD &= ~(1 << PIND5);

ACSR |= (1<<ACIS0); //set next comparator detection on rising edge
}
else  {
ACSR &= ~(1<<ACIS0); //set next comparator detection on falling edge
/* Set PIND5 to 5V */
PORTD |= (1 << PIND5);
}
}


Also, here's the link to the circuit diagram and the library itself:

UPDATE:

I have tried all of your suggestions, none of them worked but one. Clearing the interrupt flags or disabling the interrupts within or outside the ISR didn't really have any effect. I seem to misunderstand how the chip's comparator register actually works.

As I had mentioned initially, I was going to use input capture to measure the frequency of a square wave derived from a sine wave. Output of the comparator is fed into input capture pin, then use timers to measure the period, simple.

As you can see, the comparator has two outputs, ACO and ACIS0+ACIS1. ACO is set when + input > - input, cleared when + input < - input. ACIS0+ACIS1 are edge select bits.

I what I was doing initially was checking the edge type in my ISR. I changed the ISR to this instead:

    ISR(ANALOG_COMP_vect) {

if (!(ACSR &  (1<<ACO))) { // + < -
/* Set PIND5 to 0V */
PORTD &= ~(1 << PIND5);
}
else  {
/* Set PIND5 to 5V */
PORTD |= (1 << PIND5);
}
}


And the output behaved itself flawlessly (just like in the second picture). Then I proceeded to measure width of the pulses but the results weren't great. Intense toggling on my LCD display, numbers jumping to random values or staying at 0, despite having a clean signal. I rewrote my code many times using different conditions, the only semi-stable solution I have got so far is this:

#include <avr/io.h>
#include <util/delay.h>
#include "UART.h"

void init(void);

volatile uint16_t y = 0;
volatile uint16_t x = 0;
volatile uint16_t current_value = 0;
volatile uint16_t previous_value = 0;
volatile uint16_t total = 0;

void init(void) {
/* Normal mode, 64 prescaler, Rising Edge trigger, Input Capture */
TCCR1A = 0;
TCCR1B = (1 << CS10) | (1 << CS11) | (1 << ICES1);
TIMSK1 = (1 << ICIE1);

ACSR = (1 << ACIC);

/* This port is used for simulating comparator's output */
DDRC = 0xFF;
PORTC = 0xFF;

DDRD = 0x00;
PORTD = 0x00;

USART_Init(UBRR_VALUE);

sei();
}

int main(void) {

init();

while (1) {
if (TCNT1 == 60000) {
/* Display the values on the LCD */
USART_Transmit(0xFE);
USART_Transmit(0x01);

USART_Transmit_Double(x+y);
}
}
}

ISR(TIMER1_CAPT_vect) {

//ACSR &= ~(1<<ACIC);

if (!(ACSR & (1 << ACO))) {
if (!(TCCR1B & (1 << ICES1))) { // check for falling edge
PORTD |= (1 << PIND5);

PORTC &= ~(1 << PINC1);

TCCR1B |= (1 << ICES1);

current_value = ICR1;
x = current_value - previous_value;
previous_value = current_value;
}
}
else {
if (TCCR1B & (1 << ICES1)) { // check for rising edge
PORTD &= ~(1 << PIND5);

PORTC |= (1 << PINC1);

TCCR1B &= ~(1 << ICES1);

current_value = ICR1;
y = current_value - previous_value;
previous_value = current_value;
}
}

//ACSR |= (1<<ACIC);
}


By semi-stable I mean, I get the correct value 1/3 of the times. The other times 2/3 of the times it's either half of the correct value or a random value. I tried using timer's register bits for conditional statements as well as comparator's register bits in my ISR, this is the only configuration that sort of works.

What I did later in the day was use an external comparator instead with the identical setup and source (excluding all the lines related to the comparator). Its output was fed into input capture pin and it worked as intended (didn't even need any hysteresis).

At this point I can say I got it solved by using an external comparator however I have no idea why the internal one doesn't behave itself. I have read many posts and guides on this, read different libraries, tried to imitate them without any acceptable result. The datasheet only has 5 pages on the entire comparator unit, I re-read it many times and I don't see what I am doing wrong.

I would like to find out how to use it properly but if that fails I have got a backup. If you have any further input it's greatly appreciated.

• for starters... add a 1M resistor between the output and the +ve input. THIS is what creates hysteresis, not your R5... that just changes the reference – JonRB Feb 19 '18 at 16:40
• How can you produce scope pictures of the output from a comparator that is inside the chip and not accessible? – Andy aka Feb 19 '18 at 16:52
• Are you disabling further interrupts when you enter an ISR? You might need to - it could be that most ISRs are getting double hits. – Andy aka Feb 19 '18 at 17:05
• How are you toggling the hysteresis pin and are you qualifying it by the current value. The delay between the interrupt and the toggle may be screwing with you. – Trevor_G Feb 19 '18 at 17:38
• not shown in your schematic is the internal capacitance between pin5 and pin6, can you use the internal pull-up on pin7 to make your hysterisis instead? – Jasen Feb 19 '18 at 18:57

I read that you are using a DAC to generate the sine wave signal. DAC outputs can glitch at the output state changes so you should definitely apply some analogue filtering to the DAC output before feeding it into your comparator circuit. This can help with preventing some of the double interrupt triggers that are likely to be occurring.

I would also comment that you really want to be using an external comparator for this type of problem so that you can apply the hysteresis with resistors without the use of a software interaction. This will also allow better problem isolation since you can directly monitor the output of the comparator.

Last comment relates to the type of hysteresis you are using. It is a bit hard to see exactly what scheme you are using but note that what you want is behavior that does this: You want the hysteresis that pulls the threshold voltage in the OPPOSITE direction than the signal is transitioning. So for a rising edge you want the threshold to be a bit higher than the zero point and then when the state changes the threshold gets pulled to a lower level.

• +1 for the extra description of how the hysteresis direction should work. Paragraph 2 is good advice but doing it internally is also ok, provided it is done right, which in this example, does not seem to be the case. – Trevor_G Feb 19 '18 at 18:24
• @Trevor_G - :^) – Michael Karas Feb 19 '18 at 18:28
• @Hypomania - I know you can read the single timer in the ISR. But unless the timer is double buffered so that an output register holds the count from a trigger whilst the timer itself can continue to count then it becomes necessary to stop the timer so you can read it and then re-enable it after it has been read. Many MCU timers are not double buffered like that and thus the processing time to get into the ISR to when the timer is re-enabled is lost time on period time measurement for the next half cycle. It depends to some degree on how fast the timer is being clocked (continued) – Michael Karas Feb 21 '18 at 14:44
• (continued from above) but you never want to be in the situation that you are reading a count value when a clock could be coming at the same time to change the count. I have not researched the specific MCU you are using to see if your timer is double buffered on a trigger capture event or not. – Michael Karas Feb 21 '18 at 14:46
• @Hypomania - On a whim I looked at your linked AVR MCU data sheet and see that the timer input capture function is double buffered!! As a matter of fact the timer in these parts looks quite robust. It has been nearly 15 years since I used any AVR parts. – Michael Karas Feb 21 '18 at 14:56

The issues with this scenario is there is a time delay between the comparator switching and the interrupt being handled to the point where you switch the "hysteresis" pin.

Your hysteresis band is also rather small for that signal level considering what you are using it for. Especially when I see how much noise is on that square wave on your scope.

With both of those factors in mind, there is a high probability that at certain input levels you will get multiple edges from the comparator before you can handle the first one. Checking to see what the comparator state is during that interrupt handler wont help much since it can be in either state.

Unfortunately you have not detailed in the question how the handler works.

Your handler should however, work something like this.

1. When the hysteresis value in the high threshold state you should be waiting for a negative edge interrupt.

2. When said negative edge interrupt arrives, toggle the hysteresis to the low value, wait a few cycles, then clear any pending interrupt and begin waiting for a positive edge interrupt.

3. When said positive edge interrupt arrives, toggle the hysteresis pin back to the high value, wait a few cycles, clear any pending interrupt and begin waiting for a negative edge interrupt again.

4. Repeat from step 1.

BTW I'm not too keen on the way you are using the comparator reference as the bias for the signal. That results in a little cross-talk both from the signal to the reference and from the hysteresis to the signal, especially with low frequency signals. Granted with those values that effect should be small, but for purity, a separate bias on the signal would be better.

In the else statement you change the interrupt edge before you set the hysteresis.

In neither case do you pause and clear any pending interrupts before returning. (Note, changing the interrupt control register can create interrupts on it's own.)

I do not know if the Atmega does re-entrant interrupts, that is, if a subsequent edge will interrupt the still running handler from the previous edge. If so you need to handle concurrency appropriately.

Not sure what the PORTC part is for, but it probably needs to move into the qualified part.

• Thank you very much, I will try your suggestions tomorrow and give you an update. As for my ISR, I have if-else if statement for the exact scenario you have described excluding the waiting. – Shibalicious Feb 19 '18 at 18:09
• @Hypomania you should edit your question and post your interrupt handler code so folks can see if you are messing up somewhere. It may just be noise though, your scope traces look like there is way more than 50mV of noise on there. Still with noise I'd expect it to correct itself, all be it with occasional extra pulses at the transitions. – Trevor_G Feb 19 '18 at 18:11
• That's what I expected too. Will do it as soon as possible. – Shibalicious Feb 19 '18 at 18:17
• @Hypomania see edit – Trevor_G Feb 19 '18 at 19:36
• @Hypomania Because you could get another interrupt between those two commands. I also edited the edit... – Trevor_G Feb 19 '18 at 19:47

This effect is similar to contact bounce, and can be mitigated by the same debounce techniques you'd use for pushbuttons.

• Decide on debounce time Td
• keep the timestamp of the last edge interrupt in a variable
• if the time between the current interrupt and the last one is less than Td, ignore the current interrupt