ATMega328p internal comparator problem - ISR(ANALOG_COMP_vect) not firing

Question

After being introduced to this code on github, and exploring electronoobs tutorial I was compelled to try and recreate it with the Arduino Nano, and using the A0-A3 analog comparator inputs. Currently, I am just trying to get the rising edge of the receiver transducer to be recognized in the interrupt. Thus, I think my problem resides in ISR(ANALOG_COMP_vect)

Using an oscilloscope confirms that the ultrasonic transducer is firing every 100 ms, and I can detect the ping with the receiver as a small voltage above GND, which should trigger the ISR interrupt with DIO6 connected to GND... but that is not happening...

Here is my code:

#include <avr/interrupt.h>
#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>
#include <stdbool.h>
#include <Arduino.h>
#include "fastio.h"
#define F_CPU 16000000UL
// pins for TX driver
#define PING0 PB1
#define PING1 PB2
#define LED DIO13
#ifndef MASK
 #define MASK(m) (1UL << m)
#endif
#define DEBUG 0
#define INTERVAL 100
#define diff(a,b) (((a) >= (b))?((a)-(b)):((b)-(a)))
inline bool ge(uint32_t a, uint32_t b) {
 return ((a - b) & 0x80000000UL) == 0;
}
inline bool lt(uint32_t a, uint32_t b) {
 return ((a - b) & 0x80000000UL) != 0;
}
volatile uint32_t milliseconds = 0;
uint16_t starttime = 0;
volatile uint16_t envelopetime = 0;
volatile uint16_t phasetime = 0;
#define PULSECOUNT_X2 16
volatile uint8_t edges = 0;
void pins_init(void) {
 WRITE(PING0, 0);
 SET_OUTPUT(PING0);
 WRITE(PING1, 0);
 SET_OUTPUT(PING1);
 WRITE(LED, 0);
 SET_OUTPUT(LED);
}
void timer_init(void) {
 // init timers
 GTCCR = MASK(TSM) | MASK(PSRSYNC); // pause clocks
 PRR &= ~MASK(PRTIM1) & ~MASK(PRTIM0) & ~MASK(PRADC); // enable timers 0 and 1 and ADC
 // use timer1 for output waveform generation and analog comparator timing
 TCCR1A = 0; // normal mode, no match behaviour
 TCCR1B = MASK(CS10); // maximum speed (no prescale)
 // use timer0 for our millisecond clock
 TCCR0A = MASK(WGM01); // CTC mode
 TCCR0B = MASK(CS00) | MASK(CS01); // prescaler=64, ie clock is 250kHz
 OCR0A = 250; // interrupt at 1kHz, ie every 1ms
 TIMSK0 = MASK(OCIE0A); // interrupt on compare
 // set up analog comparator
 ADCSRA = (0 << ADEN);
 ADCSRB = (1 << ACME);
 ADMUX = 0;
 ACSR |= 0x03;
 
 GTCCR = 0; // re-enable clocks
}
void setup(){
 pins_init();
 Serial.begin(115200);
}
void loop() {
 timer_init();
 sei();
 Serial.println("Start");
 uint32_t pingtime = INTERVAL;
 for (;;) {
 if (ge(milliseconds, pingtime) && (edges == 0)) {
 // note when we should ping next
 pingtime += INTERVAL;
 // start in a millisecond from now to avoid race conditions
 starttime = TCNT1 + (F_CPU / 1000);
 OCR1A = OCR1B = starttime;
 // reset edge counter
 edges = PULSECOUNT_X2;
 // set A and clear B on match
 TCCR1A = MASK(COM1A0) | MASK(COM1A1) | MASK(COM1B1);
 // disable interrupt on comparator
 TIMSK1 &= ~MASK(ICIE1);
 // clear flag in case it triggered during startup
 TIFR1 = MASK(OCF1A);
 // enable interrupt
 TIMSK1 |= MASK(OCIE1A);
 }
 if (phasetime != 0) {
 uint16_t eoff = envelopetime - starttime; // time when Rx signal became loud enough to consider, ie start of envelope
 uint16_t poff = phasetime - starttime; // time of first zero crossing after envelope, use for calculating phase offset
 //printf_P(PSTR("envelope +%u (%uus) phase +%u (%uus)\n"), eoff, eoff >> 4, poff, poff >> 4);
 Serial.println(eoff);
 //Serial.println(poff);
 // zero so we only print once, process data before this
 phasetime = 0;
 }
 }
}
ISR(TIMER1_COMPA_vect) {
 if (edges == PULSECOUNT_X2) {
 TCCR1B |= MASK(ICES1); // listen for rising edge
 TIFR1 = MASK(ICF1); // clear flags in case it triggered during startup
 TIMSK1 |= MASK(ICIE1); // interrupt on comparator
 TCCR1A = MASK(COM1A0) | MASK(COM1B0); // toggle on match
 //Serial.println("PULSE");
 WRITE(LED, 1);
 }
 else if (edges == 1) {
 TCCR1A = MASK(COM1A1) | MASK(COM1B1); // clear both on match
 }
 else if (edges == 0) {
 //putc('>');
 TIMSK1 &= ~MASK(OCIE1A); // disable interrupt
 return;
 }
 edges--;
 OCR1A = OCR1B += F_CPU / 80000; // set next timeout
}
ISR (ANALOG_COMP_vect)
 {
 phasetime = ICR1; 
 if((ACSR & B00100000)){
 envelopetime = ICR1; // store timestamp
 WRITE(LED, 0);
 TIMSK1 &= ~MASK(ICIE1); // disable interrupt on comparator
 }
}
 
ISR(TIMER0_COMPA_vect) {
 milliseconds++;
}

• Edgar Bonet: 1. What's the purpose of ge() and lt()? Why not >= and <? 2. There is a lot going on in this code. Could you reduce this to a minimal example that shows the problem you are facing and nothing more? 3. You don't explain enough how it is supposed to operate. Must we read the full tutorial in order to understand it? 4. Expressions such as ACSR & B00100000 are very hard to decipher. ACSR & MASK(ACO) would be more readable. 5. You may want to use the avr-libc macros such as _BV() (equivalent to your MASK()) and bit_is_set().
• Edgar Bonet: 6. ISR(ANALOG_COMP_vect) will never fire if you don't enable the interrupt (bit ACIE in ACSR).
• timemage: Everything Edgar Bonet said. If I were working on this I'd probably remove MASK for the same reason I wouldn't use _BV. Unless you're making extraordinary use of these, they exist mostly so other people reading the code can have a "WTF?.... oh is that all?" moment. It's difficult to tell if the same thing is happening with ge() and lt().

Answer 1

You have not enabled the comparator interrupt.

Just need to change the line where you set ACSR.

//Comparator interrupt on rising output edge.
ACSR |= (1 << ACIS0) | (1 << ACIS1); 
//Enable comparator interrupt
ACSR |= (1 << ACIE)

Can also be achieved with.

ACSR = 0x0B

If you want to change what triggers an interrupt. Bits 0 and 1 control how an interrupt is fired (Toggle, falling or rising).

//Toggle (Default, only required if another interrupt mode is already selected)
ACSR &= ~(0b11)
//Falling
ACSR &= ~(1 << ACIS0); //Only required if ACIS0 is set
ACSR |= (1 << ACIS1);
//Rising
ACSR |= (1 << ACIS0) | (1 << ACIS1);
//Or
ACSR | = 0x03;

You will also need to enable global interrupts. (Which you have done) Which can be done by calling...

interrupts(); //Arduino only
sei(); //AVR and Arduino

I see you are calling "sei" in the loop, it only needs to be called once (unless you have disabled them with "cli()," and I believe its not even required at all with Arduino as I think interrupts are already enabled.

The datasheet is your friend. Tutorials are a great way to learn, but if you really want to increase your skill set you should try to break free from tutorials as much as possible by reading the datasheet, and experimenting on your own. The comparator starts on page 202, and would be a great place to start as its an easy peripheral to get going.