Millis() isn't working? Standalone project sticks at 2558490 & 2558533

Hi guys

So I have a datalogging type project going on at the moment and it's a standalone device. I'm using millis() to keep track of the time then print several sensor values like:

millis, X1, Y1, Z1, X2, Y2, Z2, PIR1, PIR2, Photo,

Which is repeated.

I left the circuit running over night and when I checked the SD card the millis recorded seem to be incorrect. I have 8hours of data recording at 25Hz sampling using interrupts. I've checked the speed of interrupt using a oscilloscope and just switching a pin high/low inside the ISR so it's not like I've configured the interrupts wrong.

The millis() stopped changing at 2558533. More than half of the recordings have been timestamped at this value of millis and I can't figure out why.

ALSO at some points, there are two (or more) millis readings of the same time but different sensor values as though it gets stuck. For several thousand values, the millis is stuck at 2558490.

Any ideas? Thanks.

Below is my ISR code.

            myFile = SD.open("m03n17.txt", FILE_WRITE);
            
            Serial.println(millis());
            myFile.print(millis());    //millis
            myFile.print(", ");
            
            // read the values of x, y, z, and T 
            // this function needs to be modified later since the 
            // temperature isnt required
            xl.readXYZTData(XValue1, YValue1, ZValue1, Temperature1, acc_chip_select_1);  
            xl.readXYZTData(XValue2, YValue2, ZValue2, Temperature2, acc_chip_select_2); 

            myFile.print(XValue1);     
            myFile.print(", ");
            myFile.print(YValue1);  
            myFile.print(", ");    
            myFile.print(ZValue1);    
            myFile.print(", ");
            
            myFile.print(XValue2);     
            myFile.print(", ");
            myFile.print(YValue2);     
            myFile.print(", ");
            myFile.print(ZValue2);     
            myFile.print(", ");    

            myFile.print(digitalRead(pir_sensor_sm));  //pir slight motion                
            myFile.print(", ");    
            myFile.print(digitalRead(pir_sensor_spot)); //pir spot type
            myFile.print(", ");               
            myFile.print(analogRead(photo_sensor_pin)); //photo diode reading in volts
            myFile.print(", ");   
            myFile.println(" ");          
    
            myFile.close();

Weird symptoms could be an indication that you're out of memory. You might try seeking out the freeMemory() function and print that out too in case you have a space-leak.

"MemoryFree" library I believe.

Is that really in an ISR?

You should not be doing prints and writes in an ISR, you want to get out as quickly as possible. You can set a flag to indicate that a print is required that can be acted on by your main-nonISR code.

I would very much like to see your whole program but if what you posted is an ISR then you should not be printing in it because Serial.print() uses interrupts and they are automatically disabled in an ISR.

You have not said why you are using an interrupt in the first place. What triggers it ?

Thanks for the replies.

KeithRB:
Is that really in an ISR?

You should not be doing prints and writes in an ISR, you want to get out as quickly as possible. You can set a flag to indicate that a print is required that can be acted on by your main-nonISR code.

Initially I had the measurements taken during the ISR and saved to one of two arrays. Then all the writing was done in the main loop from whichever array was full. However, with this method the board didn’t have enough memory to sample at a decent speed.

UKHeliBob:
Serial.print() uses interrupts and they are automatically disabled in an ISR.

You have not said why you are using an interrupt in the first place. What triggers it ?

The interrupts are just set up at 25Hz.

Putting serial.print() in the ISR was just a new addition when I was messing about and I will remove it and retry the code tonight.
Essentially I have some sensors which I would like to read from at fixed intervals rather than using delay(). The time spent inside the ISR is significantly less than the period between two interrupts. 25Hz = 40ms, and the code is usually within the ISR for less than 10ms (I tested it with a scope, putting the digital pin to high when I enter, and low when I exit).

I’ve posted the full code below.

ALSO, the library “ADXL3623.h” is just a ADXL362.h library which I’ve modified to work with two accelerometers.

//  THIS Sketch uses interrupts but doesnt use the RTC
//  Writes to the SD card in the format of:
//  millis(), X1, Y1, Z1, X2, Y2, Z2, PIR1, PIR2, Photo

/********************************************************/
#include <SPI.h>
#include <SD.h>
#include <ADXL3623.h>
/********************************************************/
//Declaration of variables
File myFile;
ADXL3623 xl;


int photo_sensor_pin = A0;      //input from photodiode amplifier circuit

const int led_pin = 2;          //to positive end of RED LED
const int button_pin = 6;       //  output of the switch (middle pin)
const int pir_sensor_sm = 4;    //   slight motion PIR
const int pir_sensor_spot = 5;  //   spot type PIR

const int sd_chip_select = 7;  //  chip select on SD card
const int rtc_chip_select = 8;  //  chip select on SD card

int acc_chip_select_1 = 10 ;     //chip select for accelerometer 1
int acc_chip_select_2 = 9;      //chip select for accelerometer 2


//accelerometer 1 and 2 values
int16_t XValue1, YValue1, ZValue1, Temperature1; 
int16_t XValue2, YValue2, ZValue2, Temperature2;

int button_press = 0;

/********************************************************/

void setup()
{
    // Initiate an SPI communication with the board
    SPI.begin();
    // Configure the SPI connection for the ADXL362 
    // and SD card
    SPI.setDataMode(SPI_MODE0);
    // Reduce the speed of the SPI from 4MHz to 125kHz
    SPI.setClockDivider(SPI_CLOCK_DIV128);
    
    Serial.begin(9600);
    
    /*-------------------------------------------------*/
    // initialisation of pins to be in/outputs
    pinMode(led_pin, OUTPUT);
    pinMode(rtc_chip_select, OUTPUT);
    
    pinMode(button_pin, INPUT);

    pinMode(pir_sensor_sm, INPUT);
    pinMode(pir_sensor_spot, INPUT);
    pinMode(sd_chip_select, OUTPUT);
    
    digitalWrite(rtc_chip_select, HIGH);
    
    /*-------------------------------------------------*/
    Serial.print("Initializing SD card...");
    
    // checks the SD paramters and also initialises the
    // chip select to digital pin 7
    if (!SD.begin(sd_chip_select)) 
    {
        Serial.println("initialization failed!");
        digitalWrite(led_pin, HIGH);
        while(1);
    }
    
    Serial.println("initialization done.");
    
    
    // open the file
    myFile = SD.open("m03d17.txt", FILE_WRITE);
    
    // if the file DID NOT open properly, return an error 
    // message and light the LED
    if (!myFile) 
    {
        // if the file didn't open, print an error:
        Serial.println("Error opening file!");
        digitalWrite(led_pin, HIGH);
        while(1);
    }
    else
        {
          //make sure the LED is off
          digitalWrite(led_pin, LOW);
          myFile.println("RESTARTED");
        }

    
        
    /*-------------------------------------------------*/
    // Configure the SPI connection for the ADXL362 
    // and SD card
 
    xl.begin();
    xl.beginMeasure(acc_chip_select_1);              // Switch ADXL362 to measure mode  
    xl.beginMeasure(acc_chip_select_2);              // Switch ADXL362 to measure mode   
    
   //Set up the interrupts at 25Hz
    cli();//stop interrupts
    
        //set timer1 interrupt at 25Hz
        TCCR1A = 0;// set entire TCCR1A register to 0
        TCCR1B = 0;// same for TCCR1B
        TCNT1  = 0;//initialize counter value to 0
        // set compare match register for 25hz increments
        OCR1A = 624;
        // turn on CTC mode
        TCCR1B |= (1 << WGM12);
        // Set CS12 and CS10 bits for 1024 prescaler
        TCCR1B |= (1 << CS12) | (1 << CS10);  
        // enable timer compare interrupt
        TIMSK1 |= (1 << OCIE1A); 
    
    sei();//allow interrupts
}

/********************************************************/
//  TIMER 1 Interrupt Service Routine. 
ISR(TIMER1_COMPA_vect){//timer1 at 25Hz

            // open the file
            myFile = SD.open("m03n17.txt", FILE_WRITE);
    
            //Print to SD card using myFile.print
            myFile.print(millis());    //millis
            myFile.print(", ");
            
            // read the values of x, y, z, and T 
            // this function needs to be modified later since the 
            // temperature isnt required
            xl.readXYZTData(XValue1, YValue1, ZValue1, Temperature1, acc_chip_select_1);  
            xl.readXYZTData(XValue2, YValue2, ZValue2, Temperature2, acc_chip_select_2); 

            //write accelerometer 1 values to the card
            myFile.print(XValue1); 
            myFile.print(", ");
            myFile.print(YValue1);  
            myFile.print(", "); 
            myFile.print(ZValue1); 
            myFile.print(", ");
            
            //write accelerometer 2 values to the card
            myFile.print(XValue2); 
            myFile.print(", ");
            myFile.print(YValue2); 
            myFile.print(", ");
            myFile.print(ZValue2); 
            myFile.print(", "); 
         
            myFile.print(digitalRead(pir_sensor_sm));  //pir slight motion            
            myFile.print(", "); 
            myFile.print(digitalRead(pir_sensor_spot)); //pir spot type
            myFile.print(", "); 
            myFile.print(analogRead(photo_sensor_pin)); //photo diode reading in volts
            myFile.print(", "); 
            myFile.println(" ");          
            
            //close the file
            myFile.close();            

}
/********************************************************/

void loop()
{
      //continue the loop while the switch hasn't been flicked
      while(LOW == button_press)
      {      
  
            //check whether the switch has been flicked
            button_press = digitalRead(button_pin);
            
            //do nothing
            
      }
      
      cli();//stop interrupts
      
      // close the file:
      myFile.close();
      
      //turn on the LED to indicate that the SD card
      //can be removed safely after a small delay
      delay(500);
      digitalWrite(led_pin, HIGH);
  
      //end the program until a reset
      return;    
      while(1);
}
/********************************************************/

Essentially I have some sensors which I would like to read from at fixed intervals rather than using delay().

Why not read from them at fixed intervals using millis() or micros() ?

UKHeliBob: Why not read from them at fixed intervals using millis() or micros() ?

Well at the time I thought interrupts would be more accurate rather than comparing current and previous value of millis.

I could still implement it using millis timing, but there's some issue with the millis getting stuck and I'm not sure why that's happening..

millis() uses an ISR (so it is just as accurate as any other ISR!) but since you spend so much time with interrupts off it probably messes it up.

KeithRB: millis() uses an ISR (so it is just as accurate as any other ISR!) but since you spend so much time with interrupts off it probably messes it up.

Interesting. In that case I will try implementing it by comparing current and previous millis values and see what happens.

I just initially thought that when the comparison is occurring, the time between the comparison and registering a difference would lead to a loss in accuracy? What I'm trying to say with reference to the crude bit of code below is that there is some random amount of time spent doing the other bits of code, at which point the millis() could exceed the 40ms, while with ISR as soon as it hits 40ms it would take measurements.

while(1) {

if( (oldmillis - millis())>40) {

//do the code }

//other bits of code }

I guess I don't need it super accurate, but I thought since the interrupts were an option I would take it.

OK so I have an update.

I modified the above code slightly, just putting the ISR inside the main loop within an if statement

while(LOW == buttonPress) { if(dataCollectFlag) { dataCollectFlag = 0; //gather data and store it to SD card } }

and inside the ISR all that happens is the flag gets set.

So I've collected some data and it seems to be going OK at exactly 40ms intervals between the points. Except, every 10th point is separated by 160ms instead of 40ms. Any ideas on what is happening?

I am going to guess that the problem lies in line 42 of the code that you didn't show us.

I will point out that any variables that are shared between "the main loop" and "the ISR" (such as dataCollectFlag) MUST MUST MUST be declared volatile.

Please try again using the volatile keyword. If it still doesn't work, provide ALL of your code using code tags (the scroll symbol on the tool bar). If you don't want to show your real code, show something simplified that exhibits the same issue. Good Luck!

Below is my ISR code.

You most certainly should not be opening or closing files in an ISR.

Nor doing serial prints.

Fix that up and get back to us.

a8rakadabra: So I've collected some data and it seems to be going OK at exactly 40ms intervals between the points. Except, every 10th point is separated by 160ms instead of 40ms. Any ideas on what is happening?

Accessing the SD card sometimes takes longer than 40ms, especially when opening and closing the file each time. It would be more efficient to leave the file open until you're done collecting data, closing it upon a button push or keyboard entry. Even with this it's still possible that writing to the SD card could exceed 40ms. I think a better approach is to collect the data at 25Hz in your ISR and store it in a FIFO of some sort, writing to the SD in the main loop whenever data is in the FIFO. At least that's what I'd try.

Keep your ISR to memory read and writes (volatile variables), pin read and writes and reading the clock. Don't try to do anything more - general rule of thumb.

Things that hang the system:

Calling delay() in an ISR (delayMicroseconds () is OK, but keep the time smal! Calling Serial.xxxx()

Things that break the system if called from ISR:

Calling new/delete or malloc/free

There are lots of other pitfalls - you need to see what a library's methods do before calling its methods from an ISR - that's your responsibilty unless the author has documented that it can be used in an interrupt context.

vaj4088: I am going to guess that the problem lies in line 42 of the code that you didn't show us.

I will point out that any variables that are shared between "the main loop" and "the ISR" (such as dataCollectFlag) MUST MUST MUST be declared volatile.

Please try again using the volatile keyword. If it still doesn't work, provide ALL of your code using code tags (the scroll symbol on the tool bar). If you don't want to show your real code, show something simplified that exhibits the same issue. Good Luck!

Hi so I've just seen this and I have done most of what has been suggested (the ISR is just one line now) but I haven't tried declaring the variable as volatile. I'll have a go with that and get back to you with my full code. Thanks for the input. Nick seems to be saying a similar thing also.

jboyton: I think a better approach is to collect the data at 25Hz in your ISR and store it in a FIFO of some sort, writing to the SD in the main loop whenever data is in the FIFO.

I had tried something similar with two data arrays, but the rate at which I was collecting data was much faster than the amount of spare memory for storing the variables in.