File being created but SD card not being written to even with flush()

dlab

Active member
I am trying to write joystick X & Y values to an SD card from within a timer interrupt using the SD library.

The file is being created, but whatever data is supposed to be written to it cannot be seen when I open it in a text editor.

The timer interrupt contains a lot of code (mostly just if statements, and a few Serial.print()s) so I will not post it here, but the part that writes to the SD card is small and here it is:

Code:
  if (writeFlag) {
    flushCounter++;
    
    // flush only once in a while (FLUSH_AFTER_TICKS = 1000)
    if (flushCounter == FLUSH_AFTER_TICKS) {
      // Serial.println("flushing");
      analogData.flush();
      flushCounter = 0;
    }

    // Serial.println("writing to SD");
    analogData.print(millisValue-stateMachineStartTime);

    for (int i = 0; i < numAnalogInputHDW; i++) {
      analogData.print(",");
      analogData.print(analogInHDW[i]->binValue);
    }

    analogData.println();
  }

This is the code that does the writing and the flushing in the timer interrupt. The timer interrupt runs every millisecond. I have confirmed that the flush() and print() lines are being executed by having Serial.print()s that showed me on the serial monitor that they are being executed.

I tried to run a simplified version of the timer interrupt that replicates most of the code that doesn't work as a sanity check. Here is the simplified sketch:

Code:
#include <SD.h>

const int DURATION = 2 * 60 * 1000;
const int FLUSH_AFTER_TICKS = 1000;

IntervalTimer timer;
File asdf;

volatile int flushCounter = 0;
int startTime = millis();

void readPinAndWrite() {
  if (flushCounter == FLUSH_AFTER_TICKS) {
    asdf.flush();
    flushCounter = 0;
  }

  flushCounter++;
  // write some arbitrary data of similar length 
  asdf.print("A,123456,"); asdf.print(analogRead(A0)); asdf.println(",1234");
}

void setup() {
  // put your setup code here, to run once:
  Serial.begin(250000);
  SD.begin(BUILTIN_SDCARD);

  asdf = SD.open("asdf.xd", FILE_WRITE);

  if (asdf) {
    Serial.println("created file");
    timer.begin(readPinAndWrite, 1000);
  } else {
    Serial.println("failed to create file");
    while (true) {}
  }
}

void loop() {
  // put your main code here, to run repeatedly:
  if ((millis() - startTime) > DURATION) {
    timer.end();
    asdf.flush();
    // asdf.close();
    Serial.println("file closed");
    while (true) {}
  }
}

This sketch works as expected, with data saved every millisecond, and flushing happening every second.

The differences between this simplified code and the code that doesn't work are that the timer interrupt is much larger, as in there's much more code being executed, but I don't think this would affect this behaviour as the code not shown just checks and sets flags along with running very few Serial.print() statements, and I think the 'type' of timer interrupt is different in the code that doesn't work. I say this because in the timer interrupt you can have Serial.print statements whereas in the simplified code, we cannot have them inside the timer interrupt.

I don't know how the usage of Serial.print() has been enabled in that timer interrupt (the code that doesn't work was written by someone else) and how it would affect this behaviour I am seeing.

What might be going wrong here? Could it have to do with the fact that the type of timer interrupt is different? Or could the Serial.print() statements take too much time and mess things up?
 
Don't do this. Just ... don't even try. Pretty much any complex library will not play nicely if you use it from within an interrupt, and the SdFat library (which SD is just a thin skin over) is a definite case in point. Believe me, I know this from bitter experience.

Any "application level" interrupt service routine (ISR) should only ever alter variables / data structures, which must be specified as volatile if they're shared with foreground code. So on timer, read the ADC (and hope that library is simple enough to be callable from interrupt...) and put the results into memory. In loop(), if there are new results (as signalled from the ISR using a volatile flag or counter), then write them from memory to the SD card. When you're done, close() the file, don't just flush() it.

If you think your loop() may be too slow to do an SD write every time one new datum is generated, which may well be true sometimes as SD writes can take many milliseconds, then use a std::queue or similar to hold your data until you're ready to deal with it.
 
So on timer, read the ADC and put the results into memory. In loop(), if there are new results (as signalled from the ISR using a volatile flag or counter), then write them from memory to the SD card. When you're done, close() the file, don't just flush() it.

See the TeensySdioLogger example in the SdFat library. It shows how to do what you're trying to do. It's not obvious, but while calls to SD write functions will often execute quickly, they will occasionally take a long time, up to ~40 ms, due to SD card "housekeeping". As h4yn0nnymOu5e said, you simply cannot expect every write to occur fast enough to do it inside a 1-ms ISR, or any ISR for that matter. The SdFat library includes a RingBuf object and the TeensySdioLogger example shows how to use it. You need to define a RingBuf large enough to hold ~50 ms of data so that during the worst-case write delays, the buffer does not overflow. The example also shows how to "pre-allocate" the file, which helps to minimize the longer write times.
 
It's not just about the speed, though. In the case of the SD(Fat) library, if you attempt card access from both ISR and foreground code, you will eventually come across the bug* whereby you're halfway through the foreground access, and the ISR triggers and starts another one. If the internal registers are in just the wrong state (and eventually they will be), the library locks up in an infinite** spin loop waiting for a hardware flag that will never be asserted. Result is your Teensy silently stops working. Intermittently. Possibly at long intervals.

* I call it a bug...
** ...because of this - Bill Greiman disagrees, and says "well, you shouldn't do it, then". Which is true, but he's happily put timeouts on pretty much every other similar wait loop.
 
Back
Top