detachInterrupt may be messing up my timer reading

Hey guys, I am making a frequency counter. For some reason my code works perfectly when I don't use the "detachInterrupt" command. Whenever I try to detach the interrupt in the ISR my frequency reading start giving randim values. I suspect that the detachInterrupt is messing with my timerRead. could you guys help clarify.

[code]
const byte        interruptPin = 26;              // Assign the interrupt pin
volatile uint64_t StartValue;                     // First interrupt value
volatile uint64_t PeriodCount;                    // period in counts of 0.000001 of a second
float             Freg;                           // frequency
char              str[21];                        // for printing uint64_t values

hw_timer_t * timer = NULL;                        // pointer to a variable of type hw_timer_t
portMUX_TYPE mux = portMUX_INITIALIZER_UNLOCKED;  // synchs between maon cose and interrupt?


// Digital Event Interrupt
// Enters on falling edge in this example
//=======================================
void IRAM_ATTR handleInterrupt()
{
  //detachInterrupt(digitalPinToInterrupt(interruptPin));
  uint64_t TempVal = timerRead(timer);        // value of timer at interrupt
  portENTER_CRITICAL_ISR(&mux);
  PeriodCount = TempVal - StartValue;         // period count between rising edges in 0.000001 of a second
  portEXIT_CRITICAL_ISR(&mux);
  StartValue = TempVal;                       // puts latest reading as start for next calculation

}




// Converts unit64_t to char for printing
// Serial.println(uintToStr( num, str ));
//================================================
char * uintToStr( const uint64_t num, char *str )
{
  uint8_t i = 0;
  uint64_t n = num;
  do
    i++;
  while ( n /= 10 );

  str[i] = '\0';
  n = num;

  do
    str[--i] = ( n % 10 ) + '0';
  while ( n /= 10 );

  return str;
}

// SetUp
//======================================
void setup()
{
  Serial.begin(115200);

  timer = timerBegin(2, 80, true);                                                // this returns a pointer to the hw_timer_t global variable
  // 0 = first timer
  // 80 is prescaler so 80MHZ divided by 80 = 1MHZ signal ie 0.000001 of a second
  // true - counts up
  timerStart(timer);  // starts the timer
  pinMode(interruptPin, INPUT_PULLUP);                                            // sets pin high
  attachInterrupt(digitalPinToInterrupt(interruptPin), handleInterrupt, FALLING); // attaches pin to interrupt on Falling Edge
}

void loop()
{
  portENTER_CRITICAL(&mux);
  Freg = 1000000.00 / PeriodCount;                    // PeriodCount in 0.000001 of a second
  portEXIT_CRITICAL(&mux);
  Serial.print("Frequency   "); Serial.println(Freg, 2);
  delay(200);

}
[/code]

Why are you doing that?

BTW, your indenting is atrocious. Please auto-format with ctrl-t.

I fixed the Indentation. I want to detach the interrupt as soon as it enters the ISR because later on I plan to reattach it after some time has passed (10 seconds) using timerAttachInterrupt. So I will be periodically checking the frequency every 10 seconds or so.

Easier to just have a bool flag. Set it to false when you don't want interrupt processing.

Check the flag upon entering the ISR. If it's false, return immediately.

1 Like

I did what you said and made the following changes to the main loop and the ISR but my output is returning "inf".

[code]
const byte        interruptPin = 26;              // Assign the interrupt pin
volatile uint64_t StartValue;                     // First interrupt value
volatile uint64_t PeriodCount;                    // period in counts of 0.000001 of a second
float             Freg;                           // frequency
char              str[21];                        // for printing uint64_t values
bool              flag = true;
hw_timer_t * timer = NULL;                        // pointer to a variable of type hw_timer_t
portMUX_TYPE mux = portMUX_INITIALIZER_UNLOCKED;  // synchs between maon cose and interrupt?


// Digital Event Interrupt
// Enters on falling edge in this example
//=======================================
void IRAM_ATTR handleInterrupt()
{
  portENTER_CRITICAL_ISR(&mux);
  if (flag == true) {
    uint64_t TempVal = timerRead(timer);        // value of timer at interrupt
    PeriodCount = TempVal - StartValue;         // period count between rising edges in 0.000001 of a second
    StartValue = TempVal;                       // puts latest reading as start for next calculation
  }
  portEXIT_CRITICAL_ISR(&mux);
  //detachInterrupt(digitalPinToInterrupt(interruptPin));
}


void loop()
{
  portENTER_CRITICAL(&mux);
  if (flag == true) {
    Freg = 1000000.00 / PeriodCount;                    // PeriodCount in 0.000001 of a second
    Serial.print("Frequency   "); Serial.println(Freg);
    flag = false;
    delay(200);
  }
  portEXIT_CRITICAL(&mux);
}
[/code]

The flag should affect only the ISR. The flag should be set to false in the ISR so that it will only run once.

You will have to add code that sets the flag back to true to re-enable the ISR.

I think the ISR must run at least twice to measure the periode. The flag kann be set to true in loop() to start a measurement. The ISR must recognize the flag and start a measurement. The next ISR resets the flag and computes the periode.
Resetting the flag may than be recognized by loop() to see that the measurement is finished.

Division by zero!

There's quite a bit in your code that I'd change:

  • The second code you posted won't even compile since it has no setup() function.

  • If you're using the Arduino setup() / loop() paradigm, then you don't need to worry about the second core in the ESP32. So, get rid of the mutex stuff.

  • Make flag volatile. Set it in the main code, reset it in the ISR.

  • Initialize StartValue.

  • Only try to compute the frequency when there's actually data available to do so.

This compiles but is untested:

const byte interruptPin = 26;              // Assign the interrupt pin
volatile uint64_t StartValue;                     // First interrupt value
volatile uint64_t PeriodCount;                    // period in counts of 0.000001 of a second
float             Freg;                           // frequency
volatile bool flag = false;
volatile bool dataReady = false;
hw_timer_t * timer = NULL;                        // pointer to a variable of type hw_timer_t


// Digital Event Interrupt
// Enters on falling edge in this example
//=======================================
void IRAM_ATTR handleInterrupt() {
  if (flag == true) {
    uint64_t TempVal = timerRead(timer);        // value of timer at interrupt
    PeriodCount = TempVal - StartValue;         // period count between rising edges in 0.000001 of a second
    //StartValue = TempVal;                     // set StartValue again in main code when ready to do another count
    flag = false;                               // Set flag again in main code when ready to do another count
    dataReady = true;
  }
}

void setup() {
  Serial.begin(115200);
  pinMode(interruptPin, INPUT_PULLUP);

  timer = timerBegin(2, 80, true);                                                // this returns a pointer to the hw_timer_t global variable
  // 0 = first timer
  // 80 is prescaler so 80MHZ divided by 80 = 1MHZ signal ie 0.000001 of a second
  // true - counts up

  attachInterrupt(digitalPinToInterrupt(interruptPin), handleInterrupt, FALLING); // attaches pin to interrupt on Falling Edge
  timerStart(timer);  // starts the timer
  StartValue = timerRead(timer);                 
  flag  = true;
}

void loop() {
  if (dataReady == true) {
    noInterrupts();
    Freg = 1000000.00 / PeriodCount;                    // PeriodCount in 0.000001 of a second
    Serial.print("Frequency   "); Serial.println(Freg);
    dataReady = false;
    interrupts();
  }
}

A few minor tweaks:

const byte interruptPin = 26;              // Assign the interrupt pin
volatile uint64_t StartValue;                     // First interrupt value
volatile uint64_t PeriodCount;                    // period in counts of 0.000001 of a second
float Freg;                           // frequency
volatile bool flag = false;
volatile bool dataReady = false;
hw_timer_t * timer = NULL;                        // pointer to a variable of type hw_timer_t

// Digital Event Interrupt
// Enters on falling edge in this example
//=======================================
void IRAM_ATTR handleInterrupt() {
  if (flag == true) {
    uint64_t TempVal = timerRead(timer);        // value of timer at interrupt
    PeriodCount = TempVal - StartValue;         // period count between rising edges in 0.000001 of a second
    //StartValue = TempVal;                     // set StartValue again in main code when ready to do another count
    flag = false;                               // Set flag again in main code when ready to do another count
    dataReady = true;
  }
}

void setup() {
  Serial.begin(115200);
  pinMode(interruptPin, INPUT_PULLUP);

  timer = timerBegin(2, 80, true);                                                // this returns a pointer to the hw_timer_t global variable
  // 0 = first timer
  // 80 is prescaler so 80MHZ divided by 80 = 1MHZ signal ie 0.000001 of a second
  // true - counts up

  attachInterrupt(digitalPinToInterrupt(interruptPin), handleInterrupt, FALLING); // attaches pin to interrupt on Falling Edge
  timerStart(timer);  // starts the timer
  StartValue = timerRead(timer);
  flag  = true;
}

void loop() {
  uint64_t localPeriodCount;
  if (dataReady == true) {
    noInterrupts();
    localPeriodCount = PeriodCount;
    dataReady = false;
    interrupts();
    Freg = 1000000.00 / localPeriodCount;                    // PeriodCount in 0.000001 of a second
    Serial.print("Frequency   "); Serial.println(Freg);
  }
}

Thanks for your help @gfvalvo. Unfortunately the frequency output isn't right. @MicroBahner is right about the fact that I have to run the ISR at least twice. In my code I added a counter to make sure the frequency is only calculated when the ISR runs twice and it worked :slight_smile:

[code]
const byte        interruptPin = 26;              // Assign the interrupt pin
volatile uint64_t StartValue;                     // First interrupt value
volatile uint64_t PeriodCount;                    // period in counts of 0.000001 of a second
float             Freg;                           // frequency
char              str[21];                        // for printing uint64_t values
bool              flag = true;
volatile uint64_t count;
hw_timer_t * timer = NULL;                        // pointer to a variable of type hw_timer_t
portMUX_TYPE mux = portMUX_INITIALIZER_UNLOCKED;  // synchs between maon cose and interrupt?


// Digital Event Interrupt
// Enters on falling edge in this example
//=======================================
void IRAM_ATTR handleInterrupt()
{
  portENTER_CRITICAL_ISR(&mux);
  if (flag == true) {
    uint64_t TempVal = timerRead(timer);        // value of timer at interrupt
    PeriodCount = TempVal - StartValue;         // period count between rising edges in 0.000001 of a second
    StartValue = TempVal;                       // puts latest reading as start for next calculation
    count++;
  }
  portEXIT_CRITICAL_ISR(&mux);
  //detachInterrupt(digitalPinToInterrupt(interruptPin));
}

void IRAM_ATTR onTime()
{
  portENTER_CRITICAL_ISR(&mux);
  flag = true;
  count = 0;
  portEXIT_CRITICAL_ISR(&mux);
}




// Converts unit64_t to char for printing
// Serial.println(uintToStr( num, str ));
//================================================
char * uintToStr( const uint64_t num, char *str )
{
  uint8_t i = 0;
  uint64_t n = num;
  do
    i++;
  while ( n /= 10 );

  str[i] = '\0';
  n = num;

  do
    str[--i] = ( n % 10 ) + '0';
  while ( n /= 10 );

  return str;
}

// SetUp
//======================================
void setup()
{
  Serial.begin(115200);

  timer = timerBegin(2, 80, true);                                                // this returns a pointer to the hw_timer_t global variable
  // 0 = first timer
  // 80 is prescaler so 80MHZ divided by 80 = 1MHZ signal ie 0.000001 of a second
  // true - counts up
  timerStart(timer);  // starts the timer
  timerAttachInterrupt(timer, &onTime, true);
  timerAlarmWrite(timer, 5000000, true);
  timerAlarmEnable(timer);
  pinMode(interruptPin, INPUT_PULLUP);                                            // sets pin high
  attachInterrupt(digitalPinToInterrupt(interruptPin), handleInterrupt, FALLING); // attaches pin to interrupt on Falling Edge
}

void loop()
{
  portENTER_CRITICAL(&mux);
  if (flag == true && PeriodCount != 0 && count == 2) {
    Freg = 1000000.00 / PeriodCount;                    // PeriodCount in 0.000001 of a second
    Serial.print("Frequency   "); Serial.println(Freg);
    delay(200);
  }
  portEXIT_CRITICAL(&mux);
}


[/code]

the only problem comes up when I add another pin to measure two frequencies simultaneously. In this regard sometimes it prints both frequencies and sometimes it prints only one frequency.

Of course not. The input signal is asynchronous with the processing. The first reading will always be bad. I saw that right away. But, I'm not going to fix all the problems with your design. Some are left as an exercise to you.

I did fix many flaws in your code that you just re-introduced into your new code (for example, there's no need to use a mutex when doing all this on a single core). Go back, read my suggestions and study my code. It would be a trivial matter to add multiple readings to the code I posted. I'd probably do something like read take N readings. Throw out the first one and average the rest.

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.