Go Down

Topic: Writing to SD after interrupt, need help optimizing (Read 6107 times) previous topic - next topic

i_like_cheese

Hello all,
My hardware: Adafruit's Datalogging shield on an Uno and a shaft Encoder on an interrupt.

I am in need of some help! This is bits of code cobbled together from others' and I am realizing that I have little idea as to what I am doing.

Without writing to the SD card I am able to catch every "tick" of the encoder (via the Serial Monitor) and the time of each tick. When I start writing to the Datalogging shield it slows down considerably and I can miss 20 or so ticks when the encoder spins quickly. I'm sure there is a more efficient way of doing this, I just don't know what it is!

Any help will be appreciated! Here is my code:
Code: [Select]
/*
Read encoder by interrupts using logic to deduce direction of spin
AMT102 Encoder - resolution 100 reads 200 ticks.
Write to Adafruit Datalogger SD card shield.
*/

#include <SD.h>

#define encoder1PinA 2
#define encoder1PinB 3
#define chipSelect   10

volatile int encoder1Pos = 0;
volatile int encoder1Val = 0;
volatile int long encoder1Time = millis();
volatile int long oldEncoder1Time = millis();

File dataFile;

void setup() {
  pinMode (encoder1PinA,INPUT);
  pinMode (encoder1PinB,INPUT);
    Serial.begin (115200);
    Serial.println("Reading one encoder...");
    Serial.println("encoder1Val, encoder1Time");
      Serial.print(encoder1Val);
      Serial.print(",");
      Serial.println(encoder1Time);

  Serial.print("Initializing SD card...");
  pinMode(10,OUTPUT);
  digitalWrite(10,HIGH);
  if(!SD.begin(chipSelect)) // if the statement !SD.begin(SS) comes back as FALSE execute the "if" statement
  {
    Serial.println("Card failed or not present"); // print to screen the SD card status
    return; // start the loop again and check if the SD is faulty or not installed
  }
  else
  {
    Serial.println("Card initialized"); // else everything is all good
  }
  if(SD.exists("1enc.txt"))
  {
    Serial.println("File exists and now being removed");
    SD.remove("1enc.txt");
  }
  else
  {
    Serial.println("No file could be found");
  }

  dataFile=SD.open("1enc.txt",FILE_WRITE);
    dataFile.println("encoder1Val, encoder1Time");
    dataFile.print(encoder1Val);
    dataFile.print(",");
    dataFile.println(encoder1Time);
  dataFile.close();
  Serial.println("Ready...");
 
  attachInterrupt(0, aChange, RISING);     
  attachInterrupt(1, bChange, RISING);
}

void loop() {

  dataFile=SD.open("1enc.txt",FILE_WRITE);
 
  if (encoder1Val != encoder1Pos) {
     
    //encoder1Val,millis 
      Serial.print(encoder1Val);
      Serial.print(",");
      Serial.println(encoder1Time);

    //encoder1Val, encoder1Time
      dataFile.print(encoder1Val);
      dataFile.print(",");
      dataFile.println(encoder1Time);
     
    //set Pos to Val
    encoder1Pos = encoder1Val;
  }
    dataFile.close();

}


//*********************************************************
void aChange() {         // when the encoder pinA changes
  encoder1Time=millis();
  if (digitalRead(encoder1PinB) == HIGH) {                 
    encoder1Val--;
  }
  else {
    encoder1Val++;
  }
}
//*********************************************************
//*********************************************************
void bChange() {         // when the encoder pinB changes
  encoder1Time=millis();
  if (digitalRead(encoder1PinA) == HIGH) {                 
    encoder1Val++;
  }
  else {
    encoder1Val--;
  }
}
//*********************************************************


marco_c

One thing that comes to mind is opening the file once only rather than each time through loop(). The file variable is global so the scope is already ok. I would also probably either write to the file or the Serial port (I assume the latter is for debugging). Serial comms may de disabling interrupts (not sure here...).
Arduino Libraries https://github.com/MajicDesigns?tab=Repositories
Parola for Arduino https://github.com/MajicDesigns/Parola
Arduino++ blog https://arduinoplusplus.wordpress.com

i_like_cheese

Thanks for the response. Yes, the Serial is for debugging - it does work better without the serial open, but not as fast as I'd like.
I don't like having to open and close the file all the time either, but I can't get it to work with various other ways I have tried. Could you give me some insight? Where would you put "open" and "close"?

marco_c

#3
Oct 28, 2013, 03:57 am Last Edit: Oct 28, 2013, 08:10 am by marco_c Reason: 1
Put the open in setup() and rather than close, make sure that you flush the data (there will be a function for this) every time you write. This will make sure that the data gets pushed out of the buffer to the SD card. Alternatively you can decide to flush every 10 (or any number you pick) so that you minimise the data loss.

This solution is not robust in case someone pulls the card out, but that requires a whole different approach. Unfortunately, unless you know that the Arduino is shutting down and can 'close' at that time, everything other than what you have already done is a compromise on data integrity.
Arduino Libraries https://github.com/MajicDesigns?tab=Repositories
Parola for Arduino https://github.com/MajicDesigns/Parola
Arduino++ blog https://arduinoplusplus.wordpress.com

i_like_cheese

Thanks! Yes, "flush()" is quite nice! I also like the trick of writing every after every 10th! I'll have to play around with that, to see what is optimal for losing the fewest ticks of the encoder.

I'm not concerned about someone pulling the card out or the arduino shutting down - it's not that critical of data. The important part is getting all of the ticks from the encoder.

CatweazleNZ

Why don't you implement a circular FIFO queue using an array of say 100 elements. That would allow allow your interrupt to record up to 100 readings in your queue (cache) before having to stop. When the interrupt is not running a procedure called from loop() could write all the queued records from the array to your log file. Even when the write function is running your interrupt could jump in and add more records into the queue (cache).

This technology is not hard to implement. You just need the array and two incrementing indexes into it. One to indicate the next array element to write the the next reading into and one to record the last array element written to the file. For a 100 array element you need to reset the two indexes back to zero when they get up to 100 - i.e. you just go around like in a circle. The two incrementing indexes simply chase each other and so long at you do not let them cross over each other everything is fine.

If you think you might capture more than 100 readings before they can be written to the SD card file then make your circular FIFO queue 200 elements, other 500, or 1,000 - whatever memory you have allowing for other application memory needs.

Cheers

Catweazle NZ

i_like_cheese

CatweazleNZ,
Thanks for the thoughts. Now that you've given me an idea of "what" to do I am still at a loss of "how" to do it.  I'm not asking anyone to write the code for me, but some bits of code to illustrate your idea sure would help!
Thanks!

Mr_arduino

Replace SD.h with sdfatlib https://code.google.com/p/sdfatlib/ then you will see a performance gain also I do not see the need for calling flush. Just don't do an unexpected power-off while the file is open and close the file when you are done with it and you will be fine.

i_like_cheese

Thanks Mr. Arduino, I'll try the fat lib and see how it goes.  As far as Flush() goes, I was under the impression that nothing actually gets written unless you close() or flush(). I'll do a test and find out.

Thanks again!

CatweazleNZ


CatweazleNZ,
Thanks for the thoughts. Now that you've given me an idea of "what" to do I am still at a loss of "how" to do it.  I'm not asking anyone to write the code for me, but some bits of code to illustrate your idea sure would help!
Thanks!

Code: [Select]
const int G_ReadingCacheCount = 100;
int G_Readings[G_ReadingCacheCount]; //Use float if required
int G_NextReading = 0;
int G_NextFileReading = 0;

void setup() {
  //Initialise if you want
  for (int l_index = 1; l_index < G_ReadingCacheCount; l_index++)
    G_Readings[l_index] = 0;
  //
}

void loop () {
  //Apart from everything else that the application is doing
  //make frequent calls to this procedure to write out the reading cache
  WriteSensorQueue();
 
}

void WriteSensorQueue() {
  //write out the cached readings (there may be none)
  //more might arrive while we are writing - that is OK
  //Open your file here (if not already open)
  while (true) {
    if (G_NextReading == G_NextFileReading)
      //we have caught up - no more cached records
      return;
    //
   G_NextFileReading = (G_NextFileReading + 1) % G_ReadingCacheCount;
   //file write G_Readings[G_NextReading] here
  } 
  //close your file here if you do not want it open all the time
}

void SensorInterrupt() {
if (((G_NextReading + 1) % G_ReadingCacheCount) == (G_NextFileReading % G_ReadingCacheCount))
   //array cache is full - ignore the readings
   return;
//
G_NextReading = (G_NextReading + 1) % G_ReadingCacheCount;
G_Readings[G_NextReading] = 12345; //"Sensor Reading Value";
}

i_like_cheese

Catweazle, this is awesome! Thanks for the help. I'll give it a whirl and see how it goes!

Mr_arduino


Thanks Mr. Arduino, I'll try the fat lib and see how it goes.  As far as Flush() goes, I was under the impression that nothing actually gets written unless you close() or flush(). I'll do a test and find out.

Thanks again!

You are correct to some extent however remember that sd card works in 512 byte sectors so it would be logical to assume that
Code: [Select]

Total-(MOD(Total bytes;512))

has been written however if whatever sd card library you are using has a larger buffer replace that with 512. I just close the file when finished with it and let the library take care of when to write to the sd card that way you don't do more writes than you need to.

marco_c

Quote
Just don't do an unexpected power-off while the file is open and close the file when you are done with it and you will be fine.


I agree that everything is fine if everyhting is perfect, but how can you guarantee that there will be no power loss? And if the application is meant to run forever, when do you close the file?

The issue with embedded programming and the style of this application (continuous data logging) is that there is no really good time to close files. A simplistic approach is to open/write/close, but the open and clkose take time. Slightly less inefficient is open/flush and never close. Flushing less or waiting until the buffer auto-flushes are variations on the same thing, basically postponing the inevitable 'actual' write to the SD card, which takes longer than a write to memory.

As the OP has said the data is not critical this is an academic exercise, but I would be interested in your views as to how you can assure data integrity in this sort of environment.
Arduino Libraries https://github.com/MajicDesigns?tab=Repositories
Parola for Arduino https://github.com/MajicDesigns/Parola
Arduino++ blog https://arduinoplusplus.wordpress.com

CatweazleNZ


Quote
Just don't do an unexpected power-off while the file is open and close the file when you are done with it and you will be fine.


I agree that everything is fine if everyhting is perfect, but how can you guarantee that there will be no power loss? And if the application is meant to run forever, when do you close the file?

The issue with embedded programming and the style of this application (continuous data logging) is that there is no really good time to close files. A simplistic approach is to open/write/close, but the open and clkose take time. Slightly less inefficient is open/flush and never close. Flushing less or waiting until the buffer auto-flushes are variations on the same thing, basically postponing the inevitable 'actual' write to the SD card, which takes longer than a write to memory.

As the OP has said the data is not critical this is an academic exercise, but I would be interested in your views as to how you can assure data integrity in this sort of environment.

The circular queue/cache solution that I suggested provides a mechanism to cache interrupt data readings during any delay caused by file open, flush and close and other application operations. You just need to be sure that the speed of incoming data (sensor reads) does not overflow the cache during worst case delays of file opening, closing and other application processing. If you have the memory you can make the cache (circular array) bigger.

Nothing will help if the speed of incoming data (sensor reads) is faster than an Arduino's ability to write data to an SD card - though you could concatenate multiple readings onto a single line and do one write operation to the SD card for every ten readings to improve write performance. (Perhaps just write in 512K data blocks for optimal performance.)

Cheers

Catweazle NZ

Mr_arduino

@marco_c there is no such thing as forever at some point there will be an end to the datalogging. What I do is just have a pushbutton to stop the datalogging. This will close the file and then blink an led in an infinite loop. To resume/restart data logging simply press the reset button. You need to have a way to stop the datalogging don't just power off when you feel done also turning off the sd card while flushing data is just as bad as not closing a file.

Go Up