Need help coding a frequency detector

I have built a frequency detector that prints the incoming frequency to the serial monitor. I made it using the following instructables:

  1. Arduino Audio Input
  2. Arduino Frequency Detection

I have uploaded the following code to my arduino:

//clipping indicator variables
boolean clipping = 0;
int low = 11;
int middle = 10;
int high = 9;
//data storage variables
byte newData = 0;
byte prevData = 0;
unsigned int time = 0;//keeps time and sends vales to store in timer[] occasionally
int timer[10];//sstorage for timing of events
int slope[10];//storage for slope of events
unsigned int totalTimer;//used to calculate period
unsigned int period;//storage for period of wave
byte index = 0;//current storage index
float frequency;//storage for frequency calculations
int maxSlope = 0;//used to calculate max slope as trigger point
int newSlope;//storage for incoming slope data

//variables for decided whether you have a match
byte noMatch = 0;//counts how many non-matches you've received to reset variables if it's been too long
byte slopeTol = 3;//slope tolerance- adjust this if you need
int timerTol = 10;//timer tolerance- adjust this if you need

//variables for amp detection
unsigned int ampTimer = 0;
byte maxAmp = 0;
byte checkMaxAmp;
byte ampThreshold = 30;//raise if you have a very noisy signal

void setup(){
  
  Serial.begin(9600);
  
  pinMode(13,OUTPUT);//led indicator pin
  pinMode(12,OUTPUT);//output pin
  pinMode(11,OUTPUT);//output pin
  pinMode(10,OUTPUT);//output pin
  pinMode(9,OUTPUT);//output pin
  
  cli();//diable interrupts
  
  //set up continuous sampling of analog pin 0 at 38.5kHz
 
  //clear ADCSRA and ADCSRB registers
  ADCSRA = 0;
  ADCSRB = 0;
  
  ADMUX |= (1 << REFS0); //set reference voltage
  ADMUX |= (1 << ADLAR); //left align the ADC value- so we can read highest 8 bits from ADCH register only
  
  ADCSRA |= (1 << ADPS2) | (1 << ADPS0); //set ADC clock with 32 prescaler- 16mHz/32=500kHz
  ADCSRA |= (1 << ADATE); //enabble auto trigger
  ADCSRA |= (1 << ADIE); //enable interrupts when measurement complete
  ADCSRA |= (1 << ADEN); //enable ADC
  ADCSRA |= (1 << ADSC); //start ADC measurements
  
  sei();//enable interrupts
}

ISR(ADC_vect) {//when new ADC value ready
  
  PORTB &= B11101111;//set pin 12 low
  prevData = newData;//store previous value
  newData = ADCH;//get value from A0
  if (prevData < 127 && newData >=127){//if increasing and crossing midpoint
    newSlope = newData - prevData;//calculate slope
    if (abs(newSlope-maxSlope)<slopeTol){//if slopes are ==
      //record new data and reset time
      slope[index] = newSlope;
      timer[index] = time;
      time = 0;
      if (index == 0){//new max slope just reset
        PORTB |= B00010000;//set pin 12 high
        noMatch = 0;
        index++;//increment index
      }
      else if (abs(timer[0]-timer[index])<timerTol && abs(slope[0]-newSlope)<slopeTol){//if timer duration and slopes match
        //sum timer values
        totalTimer = 0;
        for (byte i=0;i<index;i++){
          totalTimer+=timer[i];
        }
        period = totalTimer;//set period
        //reset new zero index values to compare with
        timer[0] = timer[index];
        slope[0] = slope[index];
        index = 1;//set index to 1
        PORTB |= B00010000;//set pin 12 high
        noMatch = 0;
      }
      else{//crossing midpoint but not match
        index++;//increment index
        if (index > 9){
          reset();
        }
      }
    }
    else if (newSlope>maxSlope){//if new slope is much larger than max slope
      maxSlope = newSlope;
      time = 0;//reset clock
      noMatch = 0;
      index = 0;//reset index
    }
    else{//slope not steep enough
      noMatch++;//increment no match counter
      if (noMatch>9){
        reset();
      }
    }
  }
    
  if (newData == 0 || newData == 1023){//if clipping
    PORTB |= B00100000;//set pin 13 high- turn on clipping indicator led
    clipping = 1;//currently clipping
  }
  
  time++;//increment timer at rate of 38.5kHz
  
  ampTimer++;//increment amplitude timer
  if (abs(127-ADCH)>maxAmp){
    maxAmp = abs(127-ADCH);
  }
  if (ampTimer==1000){
    ampTimer = 0;
    checkMaxAmp = maxAmp;
    maxAmp = 0;
  }
  
}

void reset(){//clea out some variables
  index = 0;//reset index
  noMatch = 0;//reset match couner
  maxSlope = 0;//reset slope
}


void checkClipping(){//manage clipping indicator LED
  if (clipping){//if currently clipping
    PORTB &= B11011111;//turn off clipping indicator led
    clipping = 0;
  }
}


void loop(){
  
  checkClipping();
  
  if (checkMaxAmp>ampThreshold){
    frequency = 38462/float(period);//calculate frequency timer rate/period
  
    //print results
    Serial.print(frequency);
    Serial.println(" hz");
  
  delay(100);//delete this if you want
}

The detector is mainly accurate, however there are sometimes occasional misreadings. The detector has around a 90% success rate. The following readings were produced after playing B♭3 (233.08 hz) 8 times. The bold readings are those that are not close enough to the true frequency to be read as that note.

234.52 hz
233.1 hz
231.7 hz
231.7 hz
1831.52 hz
231.7 hz
231.7 hz
233.1 hz
233.1 hz
2136.78 hz
233.1 hz
231.7 hz
230.31 hz
116.2 hz
230.31 hz
233.1 hz
233.1 hz
231.7 hz
234.52 hz
231.7 hz
231.7 hz
231.7 hz
231.7 hz
233.1 hz
231.7 hz
23.16 hz
233.1 hz
233.1 hz
231.7 hz
228.94 hz
230.31 hz
230.31 hz
230.31 hz

I want to make an output that reacts to different frequencies (hopefully not just B♭3). For simplicity’s sake I’ll say it’s 3 different leds. Is there a way to use coding to weed out all the inaccurate readings and make the output react to a specific note/frequency?

Even though this is not a question about hardware and wiring, I have included the schematic of the audio input circuit:

i don't understand why your code is looking at the slope of the signal.

of course, signals can be composed of several wavelengths and presumably you're just interested in the dominant frequency.

i thought a common approach is to look at zero crossings (and by zero, your midpoint). One way to mitigate some noise is to look for the signal to be above zero by some threshold and then below zero by some threshold. I don't believe this should be a problem since you can control the amplitude of the input by adjusting the gain of the op-amp.

it's not obvious to me how an analog input is used to trigger an ISR. Perhaps you would need to reset the threshold and whether to interrupt on the input being above or below the threshold after each valid detection (i.e. above/below)

it seems inevitable that other wavelengths can cause zero crossings when the dominant frequency is near zero. If these are harmonic (i.e. integer multiples of some dominant tone), then reject any zero crossing that occurs in slightly more that half the period (~55%) of the previous measurement.

run-time averaging with leaky integration may also improve accuracy. s is the current measurement and R a rate of integration (integer)
A += (s - A) / R

it seems your code has a excessive amount of logic. if you really think it's appropriate, have you considered testing the algorithm with synthesized signals and noise on a laptop (e.g. mathLab)

Two things come to mind, variables used in interrupt service routines need to be declared as volatile, and interrupts need to be disabled while the other parts of the code access a variable that is used in an ISR (at least for variables that take more than one byte of memory), this is because multi-byte variables are accessed a byte at a time, and an interrupt can occur between bytes and change the value.

Thank you both for your help. I have now got my readings up to a 95% success rate, with only the occasional misreading.