Micros() value lower than a previously written variable with micros() ?!

Hi everyone:

Today I'm facing what must be a stupid problem but at this point I can't figure out why is it. I'm hooking a signal generator (5V square wave) to pin 2 of my Nano in order to measure frequency (it will be rpm later on) and display it by serial.

The question is: How can micros() value be lower than "t_pulse_started"? From time to time it enters inside the "if" and it shouldn't!. Note that as frequency goes up it enters more and more times, like if there's a chance of that happening every time the interrupt ocurrs...

Thank you guys.

const byte sensor = 2;

volatile unsigned long t_pulse_started = 0;
volatile unsigned long t_pulse_ended = 0;
volatile unsigned long t_pulse_duration = 0;

unsigned long t_last_print = 0;

int rpm = 0;
bool timeout = 1;

void setup() {

Serial.begin(9600);
pinMode(sensor, INPUT);
attachInterrupt(digitalPinToInterrupt(sensor), ISR_sensor, RISING);

}

void loop() {

if(((micros() - t_pulse_started) > 2000000) && timeout == 0){

timeout = 1;
rpm = 0;
Serial.println("Timeout");

};

}

void ISR_sensor(){

t_pulse_duration = micros() - t_pulse_started;
t_pulse_started = micros();
timeout = 0;

}

Roll over? unsigned long maxes out at 4294967295 where it starts from 0 again. You should save the value of micros() used in the interrupt as "t_pulse_micros" and use that instead.

I don't think it has to do with the rollover. It happens since the very beginning and randomly. If that was the case I guess it would happen after around 71 minutes...

Thank you anyway!

Anyone knows?

You need to read the long values with interrupts disabled. Take a copy.

TheMemberFormerlyKnownAsAWOL:
You need to read the long values with interrupts disabled. Take a copy.

What do you exactly mean? Should I detach the interrupt before the "if" and attach it again after that?

Could you tell me why is this happening? How could time go back? I really need to know the REAL micros time in order to calculate higher frequencies... Which one is the most accurate one?

Thank you!

Study Gammon Forum : Electronics : Microprocessors : Interrupts.

I have read the whole thing and I will try one of the examples that is almost what I'm trying to do. Anyway I still don't know why am I having this problem (from a logic perspective). I try to think what my program does step by step and I don't know why would micros() be able to record a certain value to a variable and then have a lower value after exiting the ISR...

pplg:
I have read the whole thing.....

Go back and read it again. Search for the word "atomic".

Also, what is the point of your variable 't_pulse_duration'? You don't do anything with it.

Why use intervals of 4 microseconds when you can use intervals of 1/16th microsecond? Connect your signal to Pin 8 and this sketch will give you periods, duty-cycle, and frequency:

// Measures the HIGH width, LOW width, frequency and duty-cycle of a pulse train
// on Arduino UNO Pin 8 (ICP1 pin).


// Note: Since this uses Timer1, Pin 9 and Pin 10 can't be used for
// analogWrite().


void setup()
{
  Serial.begin(115200);
  while (!Serial);


  // For testing, uncomment one of these lines and connect
  // Pin 3 or Pin 5 to Pin 8
  // analogWrite(3, 64);  // Pin 3: 512.00, 1528.00, 2040.00, 25.10%, 490.20 Hz
  // analogWrite(5, 64);  // Pin 5: 260.00, 764.00, 1024.00, 25.39%, 976.56 Hz


  noInterrupts ();  // protected code
  // reset Timer 1
  TCCR1A = 0;
  TCCR1B = 0;
  TCNT1 = 0;
  TIMSK1 = 0;


  TIFR1 |= (1 << ICF1); // clear Input Capture Flag so we don't get a bogus interrupt
  TIFR1 |= (1 << TOV1); // clear Overflow Flag so we don't get a bogus interrupt


  TCCR1B = _BV(CS10) | // start Timer 1, no prescaler
           _BV(ICES1); // Input Capture Edge Select (1=Rising, 0=Falling)


  TIMSK1 |= _BV(ICIE1); // Enable Timer 1 Input Capture Interrupt
  TIMSK1 |= _BV(TOIE1); // Enable Timer 1 Overflow Interrupt
  interrupts ();
}


volatile uint32_t PulseHighTime = 0;
volatile uint32_t PulseLowTime = 0;
volatile uint16_t Overflows = 0;


ISR(TIMER1_OVF_vect)
{
  Overflows++;
}


ISR(TIMER1_CAPT_vect)
{
  static uint32_t firstRisingEdgeTime = 0;
  static uint32_t fallingEdgeTime = 0;
  static uint32_t secondRisingEdgeTime = 0;


  uint16_t overflows = Overflows;


  // If an overflow happened but has not been handled yet
  // and the timer count was close to zero, count the
  // overflow as part of this time.
  if ((TIFR1 & _BV(TOV1)) && ICR1 < 1024)
    overflows++;


  if (PulseLowTime == 0)
  {
    if (TCCR1B & _BV(ICES1))
    {
      // Interrupted on Rising Edge
      if (firstRisingEdgeTime)  // Already have the first rising edge...
      {
        // ... so this is the second rising edge, ending the low part
        // of the cycle.
        secondRisingEdgeTime = overflows;
        secondRisingEdgeTime = (secondRisingEdgeTime << 16) | ICR1;
        PulseLowTime = secondRisingEdgeTime - fallingEdgeTime;
        firstRisingEdgeTime = 0;
      }
      else
      {
        firstRisingEdgeTime = overflows;
        firstRisingEdgeTime = (firstRisingEdgeTime << 16) | ICR1;
        TCCR1B &= ~_BV(ICES1); // Switch to Falling Edge
      }
    }
    else
    {
      // Interrupted on Falling Edge
      fallingEdgeTime = overflows;
      fallingEdgeTime = (fallingEdgeTime << 16) | ICR1;
      TCCR1B |= _BV(ICES1); // Switch to Rising Edge
      PulseHighTime = fallingEdgeTime - firstRisingEdgeTime;
    }
  }
}


void loop()
{
  noInterrupts();
  uint32_t pulseHighTime = PulseHighTime;
  uint32_t pulseLowTime = PulseLowTime;
  interrupts();


  // If a sample has been measured
  if (pulseLowTime)
  {
    // Display the pulse length in microseconds
    Serial.print("High time (microseconds): ");
    Serial.println(pulseHighTime / 16.0, 2);
    Serial.print("Low time (microseconds): ");
    Serial.println(pulseLowTime / 16.0, 2);


    uint32_t cycleTime = pulseHighTime + pulseLowTime;
    Serial.print("Cycle time (microseconds): ");
    Serial.println(cycleTime / 16.0, 2);


    float dutyCycle = pulseHighTime / (float)cycleTime;
    Serial.print("Duty cycle (%): ");
    Serial.println(dutyCycle * 100.0, 2);


    float frequency = (float)F_CPU / cycleTime;
    Serial.print("Frequency (Hz): ");
    Serial.println(frequency, 2);
    Serial.println();


    delay(1000);  // Slow down output


    // Request another sample
    noInterrupts();
    PulseLowTime = 0;
    interrupts();
  }
}

gfvalvo:
Go back and read it again. Search for the word "atomic".

Also, what is the point of your variable 't_pulse_duration'? You don't do anything with it.

After reading what you told me, I modified my void loop() like this:

void loop() {

noInterrupts();

if(((micros() - t_pulse_started) > 2000000) && timeout == 0){

timeout = 1;
rpm = 0;
Serial.println("Timeout");

};

interrupts();

}

Now it's working! But to be honest I think I understand the described problem in the manual but I don't see how could that affect my program. Is it because the program starts "checking" the micros() inside the "if" when, suddenly, the interrupt occurs modifying the t_pulse_started value which leads to a wrong result of the operation when the interrupt ends? (micros() would be "old" and t_pulse_started would be just updated with a higher value). I'm confused.

I don't get the point of using an interrupt then since I'll be forced to "atomize" the whole loop! After reading the manual it seemed to me that ONLY writing a variable "atomically" was needed in order to prevent an interruption messing the writing process...

Thank you in advance.

pplg:
... since I'll be forced to "atomize" the whole loop! After reading the manual it seemed to me that ONLY writing a variable "atomically" was needed in order to prevent an interruption messing the writing process...

You don't have to atomize the whole loop, just the access to multibyte volatile data.

void loop() {

   noInterrupts();
   uint32_t copyOfVolatileValue = t_pulse_started;
   interrupts();

   if(((micros() - copyOfVolatileValue ) > 2000000) && timeout == 0){
    timeout = 1;
    rpm = 0;
    Serial.println("Timeout");
  }    
}

You could consider this approach:

const byte sensor = 2;

volatile unsigned long t_pulse_started = 0;
volatile unsigned long t_pulse_duration = 0;

unsigned long t_last_print = 0;

int rpm = 0;
bool timeout = 1;

void setup() {
 
  Serial.begin(9600);
  pinMode(sensor, INPUT);
  attachInterrupt(digitalPinToInterrupt(sensor), ISR_sensor, RISING);
 
}

void loop() { 

   noInterrupts();
   bool trigger = (t_pulse_duration > 2000000) && (timeout == 0);
   interrupts();
          
   if(trigger){
           
           timeout = 1;
           rpm = 0;
           Serial.println("Timeout");

   };

}

void ISR_sensor(){
 
  unsigned long us = micros();  
  t_pulse_duration = us - t_pulse_started;
  t_pulse_started = us;
  timeout = 0;
 
}

Which would prevent interruption in the loop - but the usability depends on the missing code.

pplg:
I don't get the point of using an interrupt then since I'll be forced to "atomize" the whole loop!

You only have to disable interrupts long enough to make a local copy. You were told this back in Reply #3.

void loop() {
  uint32_t localT_Pulse_Started;

  noInterrupts();
  localT_Pulse_Started = t_pulse_started;
  interrupts();
  
  if (((micros() - localT_Pulse_Started) > 2000000) && timeout == 0) {
    .
    .
    .
  }
}

johnwasser:
Why use intervals of 4 microseconds when you can use intervals of 1/16th microsecond? Connect your signal to Pin 8 and this sketch will give you periods, duty-cycle, and frequency:

// Measures the HIGH width, LOW width, frequency and duty-cycle of a pulse train

// on Arduino UNO Pin 8 (ICP1 pin).

// Note: Since this uses Timer1, Pin 9 and Pin 10 can't be used for
// analogWrite().

void setup()
{
  Serial.begin(115200);
  while (!Serial);

// For testing, uncomment one of these lines and connect
  // Pin 3 or Pin 5 to Pin 8
  // analogWrite(3, 64);  // Pin 3: 512.00, 1528.00, 2040.00, 25.10%, 490.20 Hz
  // analogWrite(5, 64);  // Pin 5: 260.00, 764.00, 1024.00, 25.39%, 976.56 Hz

noInterrupts ();  // protected code
  // reset Timer 1
  TCCR1A = 0;
  TCCR1B = 0;
  TCNT1 = 0;
  TIMSK1 = 0;

TIFR1 |= (1 << ICF1); // clear Input Capture Flag so we don't get a bogus interrupt
  TIFR1 |= (1 << TOV1); // clear Overflow Flag so we don't get a bogus interrupt

TCCR1B = _BV(CS10) | // start Timer 1, no prescaler
          _BV(ICES1); // Input Capture Edge Select (1=Rising, 0=Falling)

TIMSK1 |= _BV(ICIE1); // Enable Timer 1 Input Capture Interrupt
  TIMSK1 |= _BV(TOIE1); // Enable Timer 1 Overflow Interrupt
  interrupts ();
}

volatile uint32_t PulseHighTime = 0;
volatile uint32_t PulseLowTime = 0;
volatile uint16_t Overflows = 0;

ISR(TIMER1_OVF_vect)
{
  Overflows++;
}

ISR(TIMER1_CAPT_vect)
{
  static uint32_t firstRisingEdgeTime = 0;
  static uint32_t fallingEdgeTime = 0;
  static uint32_t secondRisingEdgeTime = 0;

uint16_t overflows = Overflows;

// If an overflow happened but has not been handled yet
  // and the timer count was close to zero, count the
  // overflow as part of this time.
  if ((TIFR1 & _BV(TOV1)) && ICR1 < 1024)
    overflows++;

if (PulseLowTime == 0)
  {
    if (TCCR1B & _BV(ICES1))
    {
      // Interrupted on Rising Edge
      if (firstRisingEdgeTime)  // Already have the first rising edge...
      {
        // ... so this is the second rising edge, ending the low part
        // of the cycle.
        secondRisingEdgeTime = overflows;
        secondRisingEdgeTime = (secondRisingEdgeTime << 16) | ICR1;
        PulseLowTime = secondRisingEdgeTime - fallingEdgeTime;
        firstRisingEdgeTime = 0;
      }
      else
      {
        firstRisingEdgeTime = overflows;
        firstRisingEdgeTime = (firstRisingEdgeTime << 16) | ICR1;
        TCCR1B &= ~_BV(ICES1); // Switch to Falling Edge
      }
    }
    else
    {
      // Interrupted on Falling Edge
      fallingEdgeTime = overflows;
      fallingEdgeTime = (fallingEdgeTime << 16) | ICR1;
      TCCR1B |= _BV(ICES1); // Switch to Rising Edge
      PulseHighTime = fallingEdgeTime - firstRisingEdgeTime;
    }
  }
}

void loop()
{
  noInterrupts();
  uint32_t pulseHighTime = PulseHighTime;
  uint32_t pulseLowTime = PulseLowTime;
  interrupts();

// If a sample has been measured
  if (pulseLowTime)
  {
    // Display the pulse length in microseconds
    Serial.print("High time (microseconds): ");
    Serial.println(pulseHighTime / 16.0, 2);
    Serial.print("Low time (microseconds): ");
    Serial.println(pulseLowTime / 16.0, 2);

uint32_t cycleTime = pulseHighTime + pulseLowTime;
    Serial.print("Cycle time (microseconds): ");
    Serial.println(cycleTime / 16.0, 2);

float dutyCycle = pulseHighTime / (float)cycleTime;
    Serial.print("Duty cycle (%): ");
    Serial.println(dutyCycle * 100.0, 2);

float frequency = (float)F_CPU / cycleTime;
    Serial.print("Frequency (Hz): ");
    Serial.println(frequency, 2);
    Serial.println();

delay(1000);  // Slow down output

// Request another sample
    noInterrupts();
    PulseLowTime = 0;
    interrupts();
  }
}

Your code works flawlessly and it's so precise. Now I'll try to understand how it works. Thank you so much!

Thank you to everyone that helped. I understood why it's happening.

Have a great day!

Yes, interrupts can happen between any two machine instructions, potentially many times inside the same C assignment statement even, as C language operations are not machine instructions.