Problems using millis() and extern interrupt

Hi,

the scope of my project is to measure the frequency of a square wave oszillator
by measuring the time between two RISING interrupts on digital pin 2 of the arduino uno.

The problem is that millis returns the same value on both interrupts.

My code is meant to do the following:

every interrupt increments Interrupt_flag
if Interrupt_flag is 1 the main loop gets a value from millis and writes it to tol_int
if Interrupt_flag is 2 the main loop calls get_Hertz
get_Hertz stops the interrupt to not be disturbed while working
now it gets a new value from millis to do the math for determining the frequency
after writing the frequency to a LCD it writes a 0 to Interrupt_flag and starts the Interrupts again

The problem is that both millis values are the same. As if main loop does both cases at once.

What is wrong with my code? Or is it a general design failure?

Best Regards

Basti

/* Frequenzmessung eines Rechtecksignals mittels externen Interrupts */

// The display communicates using SPI, so include the library:
#include <SPI.h>


// LCD Starting Byte:
const byte wcinit = 0b00011111;                                          // Initialisierungs-Byte zum Senden eines LCD-Kommandos.
const byte wzinit = 0b01011111;                                          // Initialisierungs-Byte zum Senden eines Zeichens an das LCD.

// Messwert in Millisekunden als globale Variable 
unsigned long tol_int = 0;

// Interrupt_flag
byte Interrupt_flag = 0;

void setup() {
  Serial.begin(9600);

  // Start the SPI library:
  SPI.begin();

  // Configure SPI
  SPI.setBitOrder(LSBFIRST);
  SPI.setClockDivider(SPI_CLOCK_DIV64);
  SPI.setDataMode(SPI_MODE3);
  
  // Configure LCD
  delay(20);
  // 8Bit Bus, Extended Function Set ein um auf 4 Zeilen Mode zu gehen
  LCD_write(0b00110100,0);
  delay(2);
  // 4 Zeilen 5 Dot Font Mode
  LCD_write(0b00001001,0);
  delay(2);
  // 8Bit Bus, Extended Fuction Set aus
  LCD_write(0b00110000,0);
  delay(2);
  // Display / Cursor on, Cursor blink
  LCD_write(0b00001111,0);
  delay(2);
  // Clear Display
  LCD_write(0b00000001,0);
  delay(2);
  // Cursor Auto-Increment
  LCD_write(0b00000110,0);
  
  //Configure Interrupt
  pinMode(2, INPUT);
  attachInterrupt(0, set_Interrupt_flag, RISING);
}

void loop() 
{
  switch (Interrupt_flag) {
    case 1:
      tol_int = millis();      //time of first interrupt
      break;
    case 2:
      get_Hertz();          //get time of second interrupt and calculate the frequency
      break;
    default:
      
    break;
  }

}

void get_Hertz()
{
  noInterrupts();                                                     //disable interrupts
  LCD_write(0b10000000,0);                                     //clear LCD
  unsigned long get_time = millis();                            //get time of second interrupt
  char dtostrfbuffer[4];                                                     
  unsigned long period = get_time - tol_int;                //do some math
  for (int i = 0; i <= String(tol_int).length() - 1 ; i++)   //write millis of first interrupt to LCD
    {
      LCD_write(String(tol_int)[i],1);
    }
  LCD_write(0b10100000,0);                                    
  for (int i = 0; i <= String(get_time).length() - 1 ; i++)   //write millis of second interrupt to LCD
    {
      LCD_write(String(get_time)[i],1);
    }
  LCD_write(0b10100000,0);                                      
  /*for (int i = 0; i <= String(period).length() - 1 ; i++)   //write period in ms to LCD
    {
      LCD_write(String(period)[i],1);
    }
  LCD_write(0b10100000,0);*/                          
  float frequ = 1000 / period;                              //some math
  dtostrf(frequ, 2, 1, dtostrfbuffer);                    //Float to char array
  String Sfrequ = String(dtostrfbuffer) + " Hz";            
  /*for (int i = 0; i <= Sfrequ.length() - 1 ; i++)    //write frequency to LCD
    {
      LCD_write(Sfrequ[i],1);
    }*/
  Interrupt_flag = 0;                                    //reset Interrupt_flag
  interrupts();                                             //enable interrupts again
}

void set_Interrupt_flag()
{
  Interrupt_flag += 1;
}

void LCD_write(byte data, byte RS)
{
  // Daten in lowdat und hidat aufteilen
  byte lowdat = data & 0b00001111;
  byte hidat = data >> 4 ;
  // initialize write RS = 0 Kommando, RS = 1 Zeichen
  if ( RS == 0) SPI.transfer(wcinit);
  else SPI.transfer(wzinit);
  SPI.transfer(lowdat);                                                      // write lower data 0 0 0 0 D3 D2 D1 D0
  SPI.transfer(hidat);                                                       // write higher data 0 0 0 0 D7 D6 D5 D4
  delay(2);
}

Your flag should be qualified as volatile, and you may want to use micros instead of millis.

Any variables shared between an interrupt service routine and other code should have "volatile" as part of their declaration.

What frequency is your square wave oscillator? millis() is only good up to about 1 KHz, and even at that frequency does not provide very good resolution. (1 millisecond => 1,000 Hz, 2 milliseconds => 500 Hz, 3 milliseconds => 333 Hz). You may want to consider using micros().

Hi Groove, hi vaj4088,

thanks for your fast response.

Qualifying Interrupt_flag as volatile lead to the following values:
tol_int get_time
522 522
1320 0320
...
9855 0855
10566 00566
...
13444 03444
...

Am I the only one seeing a strange pattern?

Edit: The oszillator has a frequency of about 10 Hz.

  for (int i = 0; i <= String(tol_int).length() - 1 ; i++)   //write millis of first interrupt to LCD
    {
      LCD_write(String(tol_int)[i],1);
    }

Why all this messing around ? Why not just print the tol_int value directly ?

Hi UKHeliBob,

I think the EA DIP204-4 LCD needs a byte like transmission.

Bastrator:
Hi UKHeliBob,

I think the EA DIP204-4 LCD needs a byte like transmission.

That sounds messy but I would still suggest not using Strings and using the arrays of chars directly.