strtok_r and arrays debug problem reading SD Card

Hi All

This is my first post, I got my first Adruino a month ago.

I want to read a config file from a SD Card and used the lines as variables after it was split using strtok_r.

Now the strange thing is it seams to work but when I use the actual file it does not want to work.

Here is my results with the test file and it works the way I want it to:

Test file =:
A1 = B1
A2 = B2
A3 = B3
A4 = B4
A5 = B5
A6 = B6
A7 = B7
A8 = B8
A9 = B9
A10 = B10

Results =:
Opened PRINT00.TXT

A1 = B1
A2 = B2
A3 = B3
A4 = B4
A5 = B5
A6 = B6
A7 = B7
A8 = B8
A9 = B9
A10 = B10

Done loading file
printing string

A1
B1
A2
B2
A3
B3
A4
B4
A5
B5
A6
B6
A7
B7
A8
B8
A9
B9
A10
B10

sorting out parms
i = 0 A1 B1
i = 1 A2 B2
i = 2 A3 B3
i = 3 A4 B4
i = 4 A5 B5
i = 5 A6 B6
i = 6 A7 B7
i = 7 A8 B8
i = 8 A9 B9
i = 9 A10 B10

Done

If I change the file to:

Real file = :
Start Temp = 11
End Temp = 22
Cycle Length = 33
Delta Fan = 44
Delta Heater = 55
Temp margin = 66
Window top dist = 77
Window bottom dist = 88
Manual delay = 99
Start Date = 2011/02/05

Results =:
type any character to start

Opened PRINT00.TXT

Start Temp = 11
End Temp = 22
Cycle Length = 33
Delta Fan = 44
Delta Heater = 55
Temp margin = 66
Window top dist = 77
Window bottom dist = 88
Manual delay = 99
Start Date = 2011/02/05

Done loading file
printing string

Start Temp
11
End Temp
22
Cycle Length
33
Delta Fan
44
Delta Heater
55

66Í
type any character to start

As you can see it did not finish the 10 lines, I think it has to do with the array.
Please help and let me know why this happens.

Thank you

Regards

Luan

Code:
/*

  • This sketch reads and prints the file
  • PRINT00.TXT created by SdFatPrint.pde or
  • WRITE00.TXT created by SdFatWrite.pde
    */
    #include <SdFat.h>
    #include <SdFatUtil.h>
    #include <string.h>

Sd2Card card;
SdVolume volume;
SdFile root;
SdFile file;

// store error strings in flash to save RAM
#define error(s) error_P(PSTR(s))

void error_P(const char* str) {
PgmPrint("error: ");
SerialPrintln_P(str);
if (card.errorCode()) {
PgmPrint("SD error: ");
Serial.print(card.errorCode(), HEX);
Serial.print(’,’);
Serial.println(card.errorData(), HEX);
}
while(1);
}

char SD_lines[15][30]; // 15 lines and each line may have 30 characters, Read from SD Card
String SD_param[15][2]; // 15 lines and and two variables

void setup(void)
{

for( int i=0; i < 15; i++ ) // init SD_lines
{
for( int i2=0; i2 < 30; i2++ ) // init SD_lines
{
SD_lines*[i2] = NULL; *

  • }*
  • }*
  • Serial.begin(9600);*
  • Serial.println();*
  • Serial.println(“type any character to start”);*
  • while (!Serial.available());*
  • Serial.println();*
  • // initialize the SD card at SPI_HALF_SPEED to avoid bus errors with*
  • // breadboards. use SPI_FULL_SPEED for better performance.*
  • if (!card.init(SPI_FULL_SPEED)) error(“card.init failed”);*
  • // initialize a FAT volume*
  • if (!volume.init(&card)) error(“volume.init failed”);*
  • // open the root directory*
  • if (!root.openRoot(&volume)) error(“openRoot failed”);*
  • // open a file*
  • if (file.open(&root, “PRINT00.TXT”, O_READ)) {*
  • Serial.println(“Opened PRINT00.TXT”);*
  • }*
  • else if (file.open(&root, “WRITE00.TXT”, O_READ)) {*
  • Serial.println(“Opened WRITE00.TXT”); *
  • }*
  • else{*
  • error(“file.open failed”);*
  • }*
  • Serial.println();*
  • // copy file to*
  • int16_t n;*
  • uint8_t buf[100];// nothing special about 7, just a lucky number.*
  • int il = 0; // Array line counter*
  • int ic = 0; // Caracter counter*
  • while ( (n = file.read(buf, sizeof(buf)) ) > 0)*
  • {*
  • for (uint8_t i = 0; i < n; i++)*
  • {*
    _ if(buf == ‘\n’) // if there is a new line put next line in new aray and reset counter_
    * {*
    * il++;*
    * ic = 0;*
    * }*
    SD_lines[il][ic] = SD_lines[il][ic] + buf*;
    _ ic ++;
    Serial.print(buf);
    }
    }
    Serial.println();
    Serial.println("\nDone loading file");
    Serial.println("\nprinting string");
    Serial.println();
    //int lw = 0; // lines while counter*

    * for( int lw = 0; lw < 10; lw++ )
    {
    //Serial.print(“lw = “);
    //Serial.println(lw);
    char *p = SD_lines[lw];
    char str;
    for( int i=0; i < 2; i++ ) // Split characters twice for each line, to get name and value

    * {
    //Serial.print(“i = “);
    //Serial.println(i);_

    (str = strtok_r(p, “=”, &p)); // delimiter is the =
    _ String str1 = str;
    str1 = str1.trim(); // Clean up string*

    * Serial.println(str1);
    SD_param[lw] = str1; // Problem might be here
    }
    }
    Serial.println();
    Serial.println(“sorting out parms”);
    for( int i=0; i < 10; i++ ) // Put all parms in to varibels*

    * {
    Serial.print(“i = “);
    Serial.print(i);
    Serial.print(”\t”);_

    Serial.print(SD_param[0]);
    _ Serial.print(”\t”);_
    Serial.print(SD_param[1]);
    _ Serial.println();
    }
    Serial.println();
    Serial.println(”\nDone”);
    }
    void loop(void)
    {
    }*_

Hi All

What is the correct way to post code on the form and how do I do it?

Thank you

Luan :slight_smile:

for posting code use the code tags [code ] ... / code(the icon with the #).

so, for your problem. Looks like you are eating too much memory. microControllers like the arduino don't offer a lot of sRam. So use it with care: strings eat sRam, arrays eat sRam. The file you read has something like 600 bytes of data, the static array uses 450 byte. The library itself and the print strings will eat the rest. at least it seems.

hope that helps

Thank you very much Juergen, than was the problem.

I made the file a bit smaller and I am cleaning the old arrays up by making them NULL.

      SD_lines[il][ic] = SD_lines[il][ic] + buf[i];
      ic ++;
      Serial.print(buf[i]);
      buf[i] = NULL;  // Free up mem

and I am cleaning the old arrays up by making them NULL.

That won't do anything to save memeory

ok, I think you should explain what the project is about.
I got the feeling that you need to reorganize your code and also get a deeper understanding on some C specific subjects:

There are some parts of code that are quite buggy:

char SD_lines[15][30];  // 15 lines and each line may have 30 characters, Read from SD Card
void setup(void)
{
  for( int i=0; i < 15; i++ )  // init SD_lines
  {
    for( int i2=0; i2 < 30; i2++ )  // init SD_lines
    {
// this is not ok, should be SD_lines[i][i2]=0;
// you actually write in memory when i2 becomes 15, setting every 30th cell to zero
      SD_lines[i2] = NULL;      
    }
  }

Anyway, as far as I understand what you are up to, reading, an writing parameters i think you are better off using structs on a microcontroller.

struct
{
   int StartTemp;
   int EndTemp;
// etc.
} yourdata;
// [...]
read(&yourdata, sizeof(yourdata)).

// [...]
write(&yourdata, sizeof(yourdata)).

Hi Juergen

I am building a system to control the temperature in a building. The system must be user customizable, that is why I want to read the conf file from the SD Card and then put the settings into variables to use in my code.

My next step is to put each line after the “=” in to the correct variable.

At my previous job I used Python and it was very easy to do string manipulation with Python, So now I am learning C for Arduino.

Thank you for your assistance so far.

Regards

Luan

Luan:

I want to read a config file from a SD Card and used the lines as variables after it was split using strtok_r…

  1. You are not using strtok correctly.

The loop that parses the lines should be something like the following:

    char *p = SD_lines[lw];
    char *psave;
    char *str;
    // First time through the loop, the first argument to strtok_r is p.
    // Second time through the loop, the first argument to strtok_r is NULL
    // You should NOT change the value of the third argument!
    //
    for( int i=0; i < 2; i++,p = NULL ) // Split characters twice for each line, to get name and value
    {

      //Serial.print("i = ");
      //Serial.println(i);
      (str = strtok_r(p, "=", &psave)); // delimiter is the =

      String str1 = str;
      str1 = str1.trim();    // Clean up string
      Serial.println(str1);

      SD_param[lw][i] = str1;
    }
  1. If this is for a '328 processor, the sketch uses too much RAM. See Footnote.
    Try changing the number of lines to a smaller value:
    const int numlines=12;
    char SD_lines[numlines][30];  // numlines lines and each line may have 30 characters, Read from SD Card
    String SD_param[numlines][2];  // numlines lines and and two variables

void setup(void) 
{

  for( int i=0; i < numlines; i++ )  // init SD_lines
  {
    for( int i2=0; i2 < 30; i2++ )  // init SD_lines
    {
      SD_lines[i][i2] = NULL;      
    }
  }
  Serial.begin(9600);
  Serial.println();
  Serial.println("type any character to start");
.
.
.

There may be other “issues” (stylistic and other) with the program, but maybe this will get you started.

Here are a couple of “issues” that you should address:

  1. After reading lines from the file your code should use the value of the variable il instead of 10 in subsequent loops that process the lines. What if the file had something other than 10 lines?

Something like the following:

  Serial.println("\nDone loading file");
  Serial.println("\nprinting string");
  Serial.println();
  for( int lw = 0; lw < il; lw++ ) 
  {
.
.
.

and

  Serial.println("sorting out parms");

  for( int i=0; i < il; i++ )  // Put all parms in to variables
  {
    Serial.print("i = ");
.
.
.
  1. In general, you should always test the return value of strtok_r to make sure it is not NULL. I mean for simple little programs like this you can inspect the output as you are developing the program, but in general you must have a strategy that takes into account the fact that lines that your program reads from a file might not be what you expect.

  2. Also, in general, when you are learning C you absolutely must get into the habit of making sure that you don’t read more lines than your array can hold and that the individual lines are not too long. Bad habits formed early in one’s development are notoriously hard to break.

Stuff like that.

Regards,

Dave

Footnote:
I think that you may be able to reduce memory usage (and other uncertainties) by using char arrays instead of the String library functions.
[/begin editorial]
The String library uses dynamic memory allocation, which is extremely problematic for embedded systems with limited RAM. Past versions had outright bugs, which have been, for the most part, bandaided over, but the fundamental problem of malloc remains. I mean, for small one-off programs like this, maybe it’s OK, but for “real” embedded programs, which should run forever, fragmentation of the program heap is unsolved (and, in my opinion, unsolvable without major rewrites of String stuff and/or malloc, and maybe even then).
[/end editorial]

Note that my editorial comments are an opinion. My opinion. Freely given and worth what you want it to be worth.

Furthermore…

“The opinions expressed here are not mine,
but those of the dang voices in my head.”
—davekw7x

scjurgen: ... There are some parts of code that are quite buggy:

Those bugs are not in the original post's source code, that's just what they look like since he didn't enclose the code in code tags.

If you want to see the original code, hit the 'Quote' button and copy paste the code to your Arduino edit window (or other editor of choice). The original code will compile without complaint, but there are a couple of fundamental problems, as have been noted.

Regards,

Dave

Hi Dave

Here is my updated code, I added it to a function to make it neater and that way the arrays will be cleared for the rest of the program. I now only read the amount of lines as per your suggestion.

How do I clean up the white spaces with out using the string class?

If I change “(str = strtok_r(p, “=”, &p));” to “(str = strtok_r(p, “=”, &psave)); ” it gives me the first token twice.

Now I need to put the results into the correct values.

Thank you for all your help

Regards

Luan

New file:
STMP = 11
ETMP = 22
MTMP = 66
DF = 44
DH = 55
WTD = 77
WBD = 88
MD = 99
SD = 2011/02/05
TC = 33


STMP = Start Temp
ETMP = End Temp
etc

result:
type any character to start

Opened conf.ini

STMP = 11
ETMP = 22
MTMP = 66
DF = 44
DH = 55
WTD = 77
WBD = 88
MD = 99
SD = 2011/02/05
TC = 33

Done loading file
printing string

STMP
11
ETMP
22
MTMP
66
DF
44
DH
55
WTD
77
WBD
88
MD
99
SD
2011/02/05
TC
33

sorting out parms
i = 0 STMP 11
i = 1 ETMP 22
i = 2 MTMP 66
i = 3 DF 44
i = 4 DH 55
i = 5 WTD 77
i = 6 WBD 88
i = 7 MD 99
i = 8 SD 2011/02/05
i = 9 TC 33

Done

#include <SdFat.h>
#include <SdFatUtil.h>
#include <string.h>

Sd2Card card;
SdVolume volume;
SdFile root;
SdFile file;

// store error strings in flash to save RAM
#define error(s) error_P(PSTR(s))

void error_P(const char* str) 
{
  PgmPrint("error: ");
  SerialPrintln_P(str);
  if (card.errorCode()) 
  {
    PgmPrint("SD error: ");
    Serial.print(card.errorCode(), HEX);
    Serial.print(',');
    Serial.println(card.errorData(), HEX);
  }
  while(1);  // need to chage it LFC
}


void Read_conf_SD()
{
  int numlines = 10;
  char SD_lines[numlines][30];  // numlines lines and each line may have 30 characters, Read from SD Card
  String SD_param[numlines][2];  // numlines lines and and two variables

  for( int i=0; i < numlines; i++ )  // init SD_lines
  {
    for( int i2=0; i2 < 30; i2++ )  // init SD_lines
    {
      SD_lines[i][i2] = NULL;      
    }
  }

  // initialize the SD card at SPI_HALF_SPEED to avoid bus errors with
  // breadboards.  use SPI_FULL_SPEED for better performance.
  if (!card.init(SPI_FULL_SPEED)) error("card.init failed");

  // initialize a FAT volume
  if (!volume.init(&card)) error("volume.init failed");

  // open the root directory
  if (!root.openRoot(&volume)) error("openRoot failed");

  // open a file
  if (file.open(&root, "conf.ini", O_READ)) 
  {
    Serial.println("Opened conf.ini");
  }
  else
  {
    error("file.open failed");
  }
  Serial.println();

  // copy file to 
  int16_t n;
  uint8_t buf[7];// nothing special about 7, just a lucky number.

  int il = 0;  // Array line counter
  int ic = 0;  // Caracter counter
  while ( ((n = file.read(buf, sizeof(buf)) ) > 0))  //read file
  {
    for (uint8_t i = 0; i < n; i++) 
    {
      if(buf[i] == '\n')  // if there is a new line put next line in new aray and reset counter
      {
        il++;
        ic = 0;
      }
      if ( il < numlines )  // ( il < numlines ) Only Load first numlines lines of file
      {
        SD_lines[il][ic] = SD_lines[il][ic] + buf[i];
        ic ++;
        Serial.print(buf[i]);
      }
      buf[i] = NULL;  // Free up mem
    }
  }

  Serial.println();
  Serial.println("\nDone loading file");
  Serial.println("\nprinting string");
  Serial.println();

  //lw  lines while counter
  for( int lw = 0; lw < numlines; lw++ )  // only do the amount of lines needed
  {
    //Serial.print("lw = ");
    //Serial.println(lw);
    char *p = SD_lines[lw];
    //char *psave;
    char *str;
    for( int i=0; i < 2; i++ ) // Split characters twice for each line, to get name and value
    {
      (str = strtok_r(p, "=", &p)); // delimiter is the = for splitting the string
      //while ((str = strtok_r(p, ";", &psave)) != NULL) // 
      

      String str1 = str;
      str1 = str1.trim();    // Clean up string
      Serial.println(str1);
      SD_param[lw][i] = str1;
    }

  }

  Serial.println();
  Serial.println("sorting out parms");

  for( int i=0; i < numlines; i++ )  // Put all parms in to varibels
  {
    Serial.print("i = ");
    Serial.print(i);
    Serial.print("\t");
    Serial.print(SD_param[i][0]);
    Serial.print("\t");
    Serial.print(SD_param[i][1]);
    Serial.println();
  }

  /*
  for( int i=0; i < numlines; i++ )  // Put all parms in to varibels
  {  
   if( SD_param[i][0] == "Start Temp" )
   {  
   Serial.println("Found Start Temp ");
   Serial.println(SD_param[i][1]); 
   // add to varuble here 
   }
   else if( SD_param[i][0] == "End Temp" )
   {	
   Serial.println("Found end Temp ");
   Serial.println(SD_param[i][1]);
   }
   
   else if( SD_param[i][0] == "Cycle Length" )
   {	
   Serial.println("Cycle Length ");
   Serial.println(SD_param[i][1]);
   }
   else if( SD_param[i][0] == "Start Date" )
   {	
   Serial.println("Start Date ");
   Serial.println(SD_param[i][1]);
   } 
   else if( SD_param[i][0] == "Delta Fan" )
   {	
   Serial.println("Delta Fan ");
   Serial.println(SD_param[i][1]);
   } 
   else if( SD_param[i][0] == "Delta Heater" )
   {	
   Serial.println("Delta Heater ");
   Serial.println(SD_param[i][1]);
   } 
   else if( SD_param[i][0] == "Temp margin" )
   {	
   Serial.println("Temp margin ");
   Serial.println(SD_param[i][1]);
   } 
   else if( SD_param[i][0] == "Window top dist" )
   {	
   Serial.println("Window top dist ");
   Serial.println(SD_param[i][1]);
   } 
   else if( SD_param[i][0] == "Window bottom dist" )
   {	
   Serial.println("Window bottom dist ");
   Serial.println(SD_param[i][1]);
   } 
   else if( SD_param[i][0] == "Manual delay" )
   {	
   Serial.println("Manual delay ");
   Serial.println(SD_param[i][1]);
   }    	
   else
   {
   Serial.println("Value not found ");
   }
  }
   */

  Serial.println();  
}


void setup(void) 
{

  Serial.begin(9600);
  Serial.println();
  Serial.println("type any character to start");

  while (!Serial.available());
  Serial.println();

  Read_conf_SD();


  Serial.println("\nDone");

}

void loop(void) 
{
}

Luan:

If I change “(str = strtok_r(p, “=”, &p));” to “(str = strtok_r(p, “=”, &psave)); ” it gives me the first token twice.

In your code, at the loop to parse the lines I still see:

Luan:

   for( int i=0; i < 2; i++ ) // Split characters twice for each line, to get name and value

The fact that it “seems” to work with a particular set of input lines doesn’t make it right. There will be problems if you don’t do it right.
I hate to repeat myself but

davekw7x:

    char *p = SD_lines[lw];

char *psave;
   char *str;
   // First time through the loop, the first argument to strtok_r is p.
   // Second time through the loop, the first argument to strtok_r is NULL
   // You should NOT change the value of the third argument!
   //
   for( int i=0; i < 2; i++,p = NULL ) // Split characters twice for each line, to get name and value
   {
     //Serial.print("i = ");
     //Serial.println(i);
     (str = strtok_r(p, “=”, &psave));

Did you look at my comments before the for statement? Did you look at the for statement?

Luan:
clean up the white spaces

You have to locate the first non-white-space character and the last non-white-space character in the “string” and copy all of the non-white-space stuff to the beginning.

Maybe something like this:

//
// A way to trim leading and trailing whitespace from C-style "string."
//
//  davekw7x
//
void setup()
{
    char str[80];
    Serial.begin(9600);
    strcpy(str, "  A couple of leading spaces.");
    Serial.print("Original: <");Serial.print(str);Serial.println(">");
    trim(str);
    Serial.print("Trimmed : <");Serial.print(str);Serial.println(">");
    Serial.println();
    
    strcpy(str, "A couple of trailing spaces.  ");
    Serial.print("Original: <");Serial.print(str);Serial.println(">");
    trim(str);
    Serial.print("Trimmed : <");Serial.print(str);Serial.println(">");
    Serial.println();
    
    strcpy(str, "  Leading and trailing spaces.  ");
    Serial.print("Original: <");Serial.print(str);Serial.println(">");
    trim(str);
    Serial.print("Trimmed : <");Serial.print(str);Serial.println(">");
    Serial.println();
  
    strcpy(str, "No leading or trailing spaces.");
    Serial.print("Original: <");Serial.print(str);Serial.println(">");
    trim(str);
    Serial.print("Trimmed : <");Serial.print(str);Serial.println(">");
    Serial.println();
    
    Serial.println("An 'empty' string:");
    str[0] = '\0';
    Serial.print("Original: <");Serial.print(str);Serial.println(">");
    trim(str);
    Serial.print("Trimmed : <");Serial.print(str);Serial.println(">");
    Serial.println();
    
    
    
}

void loop(){}

void trim(char *charry)
{
    int i, j;
    
    /* First find trailing whitespace and eliminate it from the "string" */
    for (i = strlen(charry)-1; i >= 0; i--) {
        if (!isspace(charry[i])) {
            break;
        }
        /* This was whitespace.  Make this the end of the "string */
        charry[i] = '\0';
    }
    
    /* Next, start at the beginning and skip leading whitespace */
    for (i = 0; charry[i] != '\0'; i++) {
        if (!isspace(charry[i])) {
            break;
        }
    }

    /* Now, copy from present position to the end of the "string." */
    for (j = 0; charry[i] != '\0'; i++, j++) {
        charry[j] = charry[i];
    }
    /* Finally, put the terminating zero byte at the end of the trimmed "string." */
    charry[j] = '\0';
}

Output:


[color=blue]Original: <  A couple of leading spaces.>
Trimmed : <A couple of leading spaces.>

Original: <A couple of trailing spaces.  >
Trimmed : <A couple of trailing spaces.>

Original: <  Leading and trailing spaces.  >
Trimmed : <Leading and trailing spaces.>

Original: <No leading or trailing spaces.>
Trimmed : <No leading or trailing spaces.>

An 'empty' string:
Original: <>
Trimmed : <>[/color]

Regards,

Dave

Hi Dave

I missed the part in bold below. I was late at night for me when I read you reply.

for( int i=0; i < 2; i++,p = NULL ) // Split characters twice for each line, to get name and value

I used your function to clean up the whit spaces.

Once again Thank you for all your help.

Regards

Luan