Can only write limited number of lines to SD card - why?

Hello,

I'm using the Adafruit Data Logging Shield (Adafruit Data logging shield for Arduino [v1.0] : ID 243 : $24.80 : Adafruit Industries, Unique & fun DIY electronics and kits) to log the output of three temperature sensors and two LDRs to a 2GB SD card. However, I can only write a limited number of lines to the card. How many I can't predict, sometimes it's a few hundred, last night it was ~4700, sometimes it's only 20 or so. Normally the circuitry is not connected to a computer, but (from memory) I think that when it's hooked up to a computer the output to the serial display (temperatures, light info, time) continues but nothing is written any more, and I don't get any errors.

The sketch, as it si now, opens the file and then goes into an infinite while() loop in which it continuously writes to the file and flushes it, to avoid repeated open() and close() operations. But I have also used a variant of the code below in which for each writing operation the file is opened, a line is written, and then the file is closed again (this seemed to be the proper way of doing it). Unfortunately, this also didn't help.

What's going on? I don't even know how to debug this.

Any hints are much appreciated.

Enno

#include <SD.h>
#include <Wire.h>
#include "RTClib.h"
#include <stdlib.h> // provides flat to string conversion function dtostrf
#include <OneWire.h>
#include <DallasTemperature.h>

// needed for the RTC
RTC_DS1307 RTC;

// Data wire is plugged into port 2 on the Arduino
#define ONE_WIRE_BUS 2

int LDRwater=0;
int LDRheat=1;
double T;
int watersensor;
int heatsensor;
float temp1;
float temp2;
float temp3;
long unixtime;
String dataString;
char s[32];  // needed for float to string conversion using dtostrf
const int chipSelect = 10;

// Setup a oneWire instance to communicate with any OneWire devices (not just Maxim/Dallas temperature ICs)
OneWire oneWire(ONE_WIRE_BUS);

// Pass our oneWire reference to Dallas Temperature. 
DallasTemperature sensors(&oneWire);

DeviceAddress Thermometer1 = { 
  0x10, 0x18, 0x58, 0x7F, 0x02, 0x08, 0x00, 0xA1 };
DeviceAddress Thermometer2 = { 
  0x28, 0xD6, 0xAC, 0x18, 0x04, 0x00, 0x00, 0xC0 };
DeviceAddress Thermometer3 = { 
  0x10, 0xA6, 0x54, 0x7F, 0x02, 0x08, 0x00, 0x94 };


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

  // Start up the library
  sensors.begin();
  // locate devices on the bus
  Serial.print("Locating devices...");
  Serial.print("Found ");
  Serial.print(sensors.getDeviceCount(), DEC);
  Serial.println(" devices.");

  // set the resolution to 12 bit (Each Dallas/Maxim device is capable of several different resolutions)
  sensors.setResolution(Thermometer1, 12);
  sensors.setResolution(Thermometer2, 12);
  sensors.setResolution(Thermometer3, 12);

  Wire.begin();

  RTC.begin();

  Serial.print("Initializing SD card...");
  // make sure that the default chip select pin is set to
  // output, even if you don't use it:
  pinMode(10, OUTPUT);

  // see if the card is present and can be initialized:
  if (!SD.begin(chipSelect)) {
    Serial.println("Card failed, or not present");
    // don't do anything more:
    return;
  }
  Serial.println("card initialized.");
  // open file for writing and write a header line
  File dataFile = SD.open("datalog.txt", FILE_WRITE);
  // if the file is available, write to it:
  if (dataFile) {
    dataFile.println("#hh mm ss unixtime LDR0 LDR1 T1 T2 T3");
    dataFile.close();
    // print to the serial port too:
    Serial.println(dataString);
  }  

}


void loop(){
  
  // open the file for writing
  File dataFile = SD.open("datalog.txt", FILE_WRITE);
  
  while(1){
  watersensor=analogRead(LDRwater); 
  heatsensor=analogRead(LDRheat);

  sensors.requestTemperatures(); // Send the command to get temperatures
  temp1 = sensors.getTempC(Thermometer1);
  temp2 = sensors.getTempC(Thermometer2);
  temp3 = sensors.getTempC(Thermometer3);

  Serial.println("--------------------------------------------------------");
  Serial.print("Warmwasser: ");
  Serial.println(watersensor);
  Serial.print("Heizung: ");
  Serial.println(heatsensor);
  Serial.print("T1, T2, T3: ");
  Serial.print(temp1);
  Serial.print(", ");
  Serial.print(temp2);
  Serial.print(", ");
  Serial.println(temp3);

  DateTime now = RTC.now();
  unixtime=now.unixtime();

  dataString=String(now.hour())+" "+String(now.minute())+" "+String(now.second())+" "+String(unixtime)+" "+\
  String(watersensor)+" "+String(heatsensor)+" "+\
  dtostrf(temp1, 7, 4, s)+" "+dtostrf(temp2, 7, 4, s)+" "+dtostrf(temp3, 7, 4, s);

  //File dataFile = SD.open("datalog.txt", FILE_WRITE);
  // if the file is available, write to it:
  if (dataFile) {
    dataFile.println(dataString);
    dataFile.flush();
    // print to the serial port too:
    Serial.println(dataString);
  }  
  // if the file isn't open, pop up an error:
  else {
    Serial.println("error opening datalog.txt");
    while(1);
  } 

  delay(10000);

  }

}
  dataString=String(now.hour())+" "+String(now.minute())+" "+String(now.second())+" "+String(unixtime)+" "+\
  String(watersensor)+" "+String(heatsensor)+" "+\
  dtostrf(temp1, 7, 4, s)+" "+dtostrf(temp2, 7, 4, s)+" "+dtostrf(temp3, 7, 4, s);

Completely and utterly unnecessary waste of resources. The File class knows how to write strings, ints, and floats to the file. Writes to the file are buffered, anyway, so pissing away resources to get all the data into one collection is not going to make the process faster anyway.

  delay(10000);

And, beside, you don't seem to give a fuck about speed anyway.

PaulS:
Completely and utterly unnecessary waste of resources. The File class knows how to write strings, ints, and floats to the file. Writes to the file are buffered, anyway, so pissing away resources to get all the data into one collection is not going to make the process faster anyway.

And, beside, you don't seem to give a fuck about speed anyway.

Why would I if I want to log temperature and light every 10s or (ultimately) every 60s?

Do your answers have anything to do with the problem I described?

Enno

Do your answers have anything to do with the problem I described?

Quite possibly - you're using a fair bit of SRAM and then compounding your problems with the String objects. Your issue may simply be that you're running out of memory and/or fragmenting what you have until it's unusable.

All those serial prints of string literals can be wrapped in the F macro to save some space and as suggested, you needn't use String objects at all - just write the data to the file and let the SD library take care of buffering. It has already reserved a bunch of your memory for the purpose - let it do its job.

I think he is struggling to tell you not to use strings as it is clogging up your code, and this probably is relevant to your problem. It may not be the whole problem. If you are using a Uno, you could be sailing a bit too close to the wind as far as memory is concerned anyway, but what you are doing is making a bad situation worse.

The aquisition part of your code looks OK, much the same as mine and I assume it is from Hacktronics. The filing section is utter junk. All you need do is send the data to the card as it is received from the sensors, indeed in the same way you send it to serial. With the amount of data you are collecting and the ten second cycle, I believe your 2gb card should be good for continuous operation for at least a year.

EDIT:
This is no longer relevant, but your line

#include <stdlib.h> // provides flat to string conversion function dtostrf

suggests that, not only is your string circus a bad idea, but also a badly executed bad idea. You may be excused if you are using a very outdated IDE, but that is a bad idea too!

I raise this because there are times when you are obliged to use strings. I'm not very experienced with them, but I don't think you're on the right tram.

Nick_Pyner:
I think he is struggling to tell you not to use strings as it is clogging up your code, and this probably is relevant to your problem. It may not be the whole problem. If you are using a Uno, you could be sailing a bit too close to the wind as far as memory is concerned anyway, but what you are doing is making a bad situation worse.

Ok, there seems to be a consensus here that this is a bad idea - but why? I'm simply defining a string, which gets used over and over again. Why is it that in some situations I can write tens of lines, sometimes a few hundred? Does the way I use the strings consume increasing amounts of memory?

The aquisition part of your code looks OK, much the same as mine and I assume it is from Hacktronics. The filing section is utter junk.

I pieced together the reading of the sensors from examples, but AFAIR I wanted to write the temperatures to the SD card with more significant digits. I know they provide 12 bits of resolution only anyway, but I was trying to understand string formatting and was playing around with it. Seems it didn't go very well!

All you need do is send the data to the card as it is received from the sensors, indeed in the same way you send it to serial.

That's what I'm doing at the moment (it's sitting at home, collecting data and I will know tonight if it worked). But as I said that's only writing the temperatures, which are floats, to the CD card with 4 significant digits. What to do if I wanted more precision?

EDIT:
This is no longer relevant, but your line

#include <stdlib.h> // provides flat to string conversion function dtostrf

suggests that, not only is your string circus a bad idea, but also a badly executed bad idea. You may be excused if you are using a very outdated IDE, but that is a bad idea too!

Uh-oh....

Many thanks for your hints!

Enno

I'm simply defining a string, which gets used over and over again.

There are two things wrong with this statement. First, you are not creating a string. You are creating Strings - 6 of them using explicit constructor calls plus many more as a result of the concatenation operators being used, and another one as a result of the equal operator being called.

If you don't believe that, modify the String class to add Serial.println() statements to the constructors (all of them).

Then, when you are done with them, the destructors need to be called. Are they called in the correct order, so that all the memory allocated becomes freed, in a big block? Or, are you fragmenting memory? Dynamic memory allocation on a system with a limited amount of memory is a bad idea, to be avoided if at all possible.

And, in your case, its not only possible, its easy.

Why is it that in some situations I can write tens of lines, sometimes a few hundred? Does the way I use the strings consume increasing amounts of memory?

No, but it's like rolling out cookie dough and using a cookie cutter to cut out a dozen cookies. Do it right, and you get a dozen cookies. Do it wrong, and you don't. You have plenty of dough left, but there is no piece big enough to cut another cookies.

but I was trying to understand string formatting

String and string are not the same thing. Careful use of terminology IS important.

But as I said that's only writing the temperatures, which are floats, to the CD card with 4 significant digits. What to do if I wanted more precision?

Do you REALLY have thermometers that are capable of 0.0001 degree steps? Does it really matter?

Anyway, the answer is that the second argument to the print() method, when the first argument is a float, is the number of digits to print after the decimal point.

myFile.println(myFloat, 2); // Reasonable number of digits for a temperature
myFile.println(myFloat, 22); // Not even in the same universe as reasonable...

What may be confusing the OP, and btw I'm no guru at this stuff, is that a String spelt deliberately with capital-S is not the same as a string, small-s.

Both are explained in the Reference

string vs String.PNG

mcenno:
Why is it that in some situations I can write tens of lines, sometimes a few hundred? Does the way I use the strings consume increasing amounts of memory?

I don't know but look to Jimbo's note. As I said, you may be having other memory problems if you are using a Uno.

I pieced together the reading of the sensors from examples,

That may be the real problem. There's a lot of junk out there. I fail utterly to see why anybody would ever use strings when there is clearly no need. It might be a left-over from a time long ago when Arduinos couldn't do floats. I realised you were not using the Hacktronics. Check here Arduino 1-Wire Tutorial
I recognise that these things can depend on just where you are on the road to Damascus but nothing made sense until I got onto Hacktronics and I can't recommend them highly enough.

That's what I'm doing at the moment (it's sitting at home, collecting data and I will know tonight if it worked).

Likely as not it will be fine!

But as I said that's only writing the temperatures, which are floats, to the CD card with 4 significant digits. What to do if I wanted more precision?

Perhaps a reality check might be in order here - like first finding the sensor, and then coming up with a sensible reason for using it.
I understand a float is a float, a number up to six digits, and they are all written the same way.

I'm sure the only possible reason for fiddling about with strings is that you want to feed data to a programme that won't accept floats. Such things do exist and the following does just that. I raise this because

  1. It has similarities with what you are doing and some bits might be useful
  2. It does not use stdlib.h
  3. It still sends floats to SD and LCD
/*
This programme reads temperatures from three DS18B20 sensors at one 
second intervals.

The temperatures are displayed on Nokia 5110 LCD  

Two of the temperatures, plus the difference between them, are transmitted 
over bluetooth in a format suitable for BlueTooth Terminal/Graphics (CETIN)
in order to display three graphs in real time.

Every tenth reading is recorded on SD card in real numbers to two decimal
places. Each line is timestamped.

A new file is created at midnight using the date as the filename.

Credit to Hacktronics, Mellis & Igoe, Garage Box, Bildr.org, etc.
Kudos to lar3ry in Saskatchewan, Stanley in KL, Coding Badly in Tx.

*/

#include <OneWire.h>
#include <DallasTemperature.h>
#include <SPI.h>       
#include <PCD8544.h>             // Nokia 5110
#include "Wire.h"                // MUST HAVE lib for LCD disp, SD card
#include <SD.h>
#include "RTClib.h"              //Date As Filename
#include <string.h>              //Date As Filename 

#define DS1307_ADDRESS 0x68

RTC_DS1307 RTC;
char filename[] = "00000000.CSV";
File myFile;

static PCD8544 lcd;

// Custom symbols
static const byte DEGREES_CHAR = 1;
static const byte degrees_glyph[] = { 0x00, 0x07, 0x05, 0x07, 0x00 };
static const byte SLASH_CHAR = 2;
static const byte slash_glyph[] = {0x00,0x20,0x10,0x08};

// Yellow group Lismore
byte InThermo[8] =  {
  0x28, 0x39, 0xFD, 0x50, 0x04, 0x00, 0x00, 0X69};
byte OutThermo[8] = {
  0x28, 0x09, 0xA9, 0xC0, 0x03, 0x00, 0x00, 0x95};
byte DrainThermo[8] = {
  0x28, 0x62, 0xA5, 0x2D, 0x04, 0x00, 0x00, 0x21}; 
  
#define ONE_WIRE_BUS 3
OneWire oneWire(ONE_WIRE_BUS);
DallasTemperature sensors(&oneWire);
 
int Inwhole,Outwhole,Drainwhole,diffwhole; // these are for assembly into str$
int Infrac,Outfrac,Drainfrac,difffrac;

int  second, minute, hour, weekDay, monthDay, month, year;
int k=0;

const int chipSelect = 4;

float InTemp, OutTemp, DrainTemp, diff; 

// Define the strings for our datastream IDs
char sensorId0[] = "InThermo";
char sensorId1[] = "OutThermo";
char sensorId2[] = "DrainThermo";

char strIn[8];
char strOut[8];
char strDrain[8];
char strdiff[8];

String stringOne, stringTwo, stringThree, stringFour;
//_____________________________________________________________

void setup() {
   lcd.begin(84, 48);
     // Register the custom symbols...
  lcd.createChar(DEGREES_CHAR, degrees_glyph);
  lcd.createChar(SLASH_CHAR, slash_glyph);
  
  Wire.begin();
  Serial.begin(9600);
  //RTC.begin();

  delay(300);//Wait for newly restarted system to stabilize
  
  lcd.setCursor (0,0);
  lcd.println("Init SD CARD");
  // make sure that the default chip select pin is set to
  // output, even if you don't use it:
  //pinMode(10, OUTPUT);// Uno
  pinMode(53, OUTPUT);//MEGA
  
  // see if the card is present and can be initialized:
  if (!SD.begin(chipSelect)) 
  {
    lcd.println("Card failed");
    // don't do anything more:
    return;
  }
  lcd.println("CARD OK");
  delay(2000);
  
       getFileName();
      lcd.println(filename);
        delay(2000);
  
    lcd.clear();
  
  sensors.setResolution(InThermo, 12);
  sensors.setResolution(OutThermo, 12);
  sensors.setResolution(DrainThermo, 12);
      
  //Sequence for bluetooth
  // "E256,-321,982\n" 
  //so insert three variable between four strings, two are the same twice, 
  //to make a fourth string 
  
  stringOne = String("E");
  stringTwo = String(",");
  stringThree = String("\n");

    running();
  }

//___________________________________________________________________________

void loop() {

   GetClock();
  if (hour == 0 && minute == 0 && second <2)
  {
    getFileName();
  }
  //get the values from the DS8B20's 
  sensors.requestTemperatures();

  InTemp = (sensorValue(InThermo));
  OutTemp = (sensorValue(OutThermo));  
  DrainTemp = (sensorValue(DrainThermo)); 

  diff = OutTemp - InTemp;
  
dtostrf(InTemp,4, 2, strIn);
dtostrf(OutTemp,4, 2, strOut);
dtostrf(diff,4, 2, strdiff);

stringFour = stringOne + strIn + stringTwo + strOut;
stringFour = stringFour + stringTwo + strdiff + stringThree;

  Serial.println(stringFour);

  lcd.setCursor(49,0);
  lcd.print(InTemp);
  lcd.setCursor(49,1);
  lcd.print (OutTemp);
  lcd.setCursor(49,2);
  lcd.print(DrainTemp);
  lcd.setCursor(49,3);
  lcd.print(diff);  
  lcd.setCursor(20,5);
  
     if( second==0)
  {
   lcd.print("         ");
   lcd.setCursor(20,5);
   } 
  
  lcd.print(hour);
  lcd.print(":");
  lcd.print(minute);
  lcd.print(":");
  lcd.print(second);

  k=k+1;  

  if (k>9 )
  {  
  myFile = SD.open(filename, FILE_WRITE);//<<<<<<<<<<<<< OPEN
  myFile.print(hour);
  myFile.print(":");
  myFile.print(minute);
  myFile.print(":");
  myFile.print(second);
  myFile.print(",");

  myFile.print(InTemp);
  myFile.print(",");
  myFile.print(OutTemp);
  myFile.print(",");
  myFile.print(DrainTemp);
  myFile.print(",");
  myFile.print(diff);
  myFile.println();
       myFile.close();//>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>CLOSE
       
      k=0;
  }
  delay(850);
}  // loop ends here

//sensorValue function
float sensorValue (byte deviceAddress[])
{
  float tempC = sensors.getTempC (deviceAddress);
  return tempC;
}

byte bcdToDec(byte val)  {
  // Convert binary coded decimal to normal decimal bers
  return ( (val/16*10) + (val%16) );
}

void running(){
  lcd.setCursor(0,0);
  lcd.print("In");
  lcd.setCursor(31,0);
  lcd.print("\001C ");
  lcd.setCursor(0,1);
  lcd.print("Out");
  lcd.setCursor(31,1);
  lcd.print("\001C ");
  lcd.setCursor(0,2);
  lcd.print("Drain");
  lcd.setCursor(31,2);
  lcd.print("\001C ");
  lcd.setCursor(0,3);
  lcd.print("diff");
  lcd.setCursor(31,3);
  lcd.print("\001C ");  
  }

void getFileName(){

DateTime now = RTC.now();

filename[0] = (now.year()/1000)%10 + '0'; //To get 1st digit from year()
filename[1] = (now.year()/100)%10 + '0'; //To get 2nd digit from year()
filename[2] = (now.year()/10)%10 + '0'; //To get 3rd digit from year()
filename[3] = now.year()%10 + '0'; //To get 4th digit from year()
filename[4] = now.month()/10 + '0'; //To get 1st digit from month()
filename[5] = now.month()%10 + '0'; //To get 2nd digit from month()
filename[6] = now.day()/10 + '0'; //To get 1st digit from day()
filename[7] = now.day()%10 + '0'; //To get 2nd digit from day()
}

void GetClock(){
  // Reset the register pointer
  Wire.beginTransmission(DS1307_ADDRESS);
  byte zero = 0x00;
  Wire.write(zero);
  Wire.endTransmission();
  Wire.requestFrom(DS1307_ADDRESS, 7);

  second = bcdToDec(Wire.read());
  minute = bcdToDec(Wire.read());
  hour = bcdToDec(Wire.read() & 0b111111); //24 hour time
  weekDay = bcdToDec(Wire.read()); //0-6 -> sunday - Saturday
  monthDay = bcdToDec(Wire.read());
  month = bcdToDec(Wire.read());
  year = bcdToDec(Wire.read());
}

Perhaps a reality check might be in order here

String stringOne, stringTwo, stringThree, stringFour;

Yep. A reality check IS in order.

PaulS:
Yep. A reality check IS in order.

Let's check now......... That's an output at one second intervals for fifteen days plus a bit.
So I guess the reality is 1,300,000 lines of data fed to the phone.

Hello,

removing the string conversion indeed fixed the problem and the board is now sitting there, writing data to the SD card.

Many thanks for your replies and hints. I'm still learning how to program the Arduino and maybe having a little background in Python triggered me to convert the floats to strings (or Strings, or whatever...). My bad!

Cheers,

Enno

mcenno:
maybe having a little background in Python triggered me to convert the floats to strings (or Strings, or whatever...).

Aha! I guess that's as good an explanation as any.

This has been a fascinating read. As an Arduino newbie, I have just put my first real project together (yet to b e tested) by following the example Datalogger.ino in the SD library.

That example uses Strings (not strings) to hold the data to be written, and until reading here, I would have thought that was a requirement of the library. I simply copied and extended the example. The same example includes a comment to the effect that only one file can be open at a time, though the library documentation says otherwise.

It would help if some effort was put into getting the example code in good shape. I realise that is a lot of work, but, as this thread shows, it would save a lot of heartache for both the new programmer and the seasoned ones who have to deal with the problems caused by following the examples.

Thanks to all for the insight. I'm going back to my code to do away with the Strings.

Pogo

Pogo:
by following the example Datalogger.ino in the SD library.

Hah! So that's where all this string crap is coming from....... I have never understood this, and never gotten round to finding out why, but I guessed it just might be some legacy thing.

I don't know where my SD stuff came from but I do know I was never faced with this string problem so many people have. I have note that my SD stuff is down to Mellis and Igoe. I can only conclude that, after Igoe wrote that example, Mellis came along, talked some sense in to him, and the collaborated to produce something that works..

From what I can see, the only time you ever actually need to use strings is when you are feeding data to a programme that demands to be fed that way. The only programme I am aware of that does this is Bluetooth Graphics Terminal for Android.

In the unlikely event that you need to pursue this further, Lar3ry in Saskatchewan is about the only sensible source of illumination.

mcenno:
Do your answers have anything to do with the problem I described?

So now you see that they do. There has been increasing criticism lately with people claiming their question is not being addressed and is just receiving nit picking criticism of things that have nothing to do with the fault.

It is a very good idea when fault finding to fix everything you see, or is pointed out to you as being wrong, whether you think it needs fixing or not. Very often it is indirectly the fault and if not will be the fault when you have got over the current one.

Grumpy_Mike:

mcenno:
Do your answers have anything to do with the problem I described?

So now you see that they do. There has been increasing criticism lately with people claiming their question is not being addressed and is just receiving nit picking criticism of things that have nothing to do with the fault.

It is a very good idea when fault finding to fix everything you see, or is pointed out to you as being wrong, whether you think it needs fixing or not. Very often it is indirectly the fault and if not will be the fault when you have got over the current one.

Getting a bit off topic, but I see here a mismatch between the skills and abilities of the OP and those of the respondents.

To take this reply as an example, it is certainly a good thing to fix every fault, the project will do what it is intended in a software sense, but if the goal is not simply to build a blinking widget, but to understand the software and its interaction with the hardware, then the poster has a point.

It is difficult to strike a balance, I'm sure. You don't want to disrespect posters by talking down to them, equally, you don't want to leave them floundering with technically correct advice that doesn't help them understand the reason for the fault.

I am, as I said, an Arduino newbie, and I have had a life-long aversion to c, but as a retired electrical engineer, I have a pretty good understanding of the issues involved in a broad sense, just not the technical detail. There will be posters with far less skill, and others with far more than me asking for help here. Their complaints about the responses they get should be a clue to where they are in the spectrum, and what level of guidance they require.

Arduino content: Having been twice through my code, first throwing out the Strings, then using a string (via sprintf) to format the data for the SD card (building a CSV file), it is readable again (the probable intent of using Strings), and compiles to a smaller size. Now to actually hook up the hardware and see how it runs.

Thanks again for the insight.

Pogo.