next topic »

[on:]

I am trying to create database by using JSON and SD library.
But i stuck in a problem "Controller hanging" after removing lot of stuff to make Memory Efficent Solution still i happens again.

Specifications
Flash 128k
SRAM 16k

Hardware
Offical Arduino Ethernet Shield
Zigduino board

To debug problem four tests were perform

Test 1 Only Parsing funtion Call
Status Ok
Test 2 Only Writng funtion Call
Status Ok
Test 3 Only Parsing funtion Call
Status Ok
Test 4 Both funtion Call
Status Hang

This the code and outputs of all tests attached .Status ok in zip file due limitation of 4 files

Code:

#include <aJSON.h>
#include <SD.h>

File myFile;

char* readFile(char* file)
{
  Serial.println(FreeRam(),DEC);
  char buf[30];
  char strFile[600];
  int16_t  readed;

  myFile = SD.open(file,FILE_READ);
  if(myFile)
  {
    readed = myFile.read(buf,30);
    buf[readed] = '\0';
    String stringOne =  String(buf);
    while( readed > 0) {
      readed = myFile.read(buf,30);
      buf[readed] = '\0';
      stringOne += String(buf);
    }
    myFile.close();
    stringOne.toCharArray(strFile,stringOne.length()+1);
    //Serial.println(strFile);
    return strFile;
  }
  else
    Serial.println("Fail to open file");
}
void parseFile(char* object)
{
  Serial.println(FreeRam(),DEC);
  char* pch =readFile("test.txt");
  Serial.println(pch);
  aJsonObject* root = aJson.parse(pch);
  
  if (root != NULL) {
    Serial.println(F("Parsed successfully 1 "));
    aJsonObject* query = aJson.getObjectItem(root, object);
    
    char *json_String=aJson.print(query);
    
    Serial.println(json_String);
    Serial.println(F("**"));
    free(json_String);
    free(pch);
  }
  else
    Serial.println(F("Parsed unsuccessfully 1"));
  aJson.deleteItem(root);
}

void writeDb(char* object)
{
  char* buff = readFile("test.txt");
  //Serial.println(buff);
  Serial.println(FreeRam(),DEC);
  Serial.println(F("HANG POINT"));
  aJsonObject* root = aJson.parse(buff);

  if (root != NULL) {
    Serial.println(F("writdb successfully 1 "));

    aJsonObject *tmp = aJson.createObject();
    aJson.addStringToObject(tmp, "value1","1101");
    aJson.addStringToObject(tmp, "value2","1102");
    aJson.addStringToObject(tmp, "value3","1103");
    aJson.replaceItemInObject(root, object, tmp);

    SD.remove("test.txt");
    delay(100);
    myFile = SD.open("test.txt", FILE_WRITE);

    // if the file opened okay, write to it:
    if (myFile) {
      char* json_String = aJson.print(root);
      
      myFile.print(json_String);
      myFile.close();
      Serial.println(F("FileClosed"));
      Serial.println(F("WR**"));

      //aJson.deleteItem(tmp);
      free(json_String);
      free(buff);
    }
    else
      Serial.println(F("File Not Truncate"));
  }
  else
    Serial.println(F("wrtidb unsuccessfully 1 "));
  aJson.deleteItem(root);
}
void setup() {
  Serial.begin(9600);

  pinMode(SS, OUTPUT);                       // set the SS pin as an output (necessary!)
  digitalWrite(SS, HIGH);                    // but turn off the W5100 chip!
  if (!SD.begin(4)) {
    Serial.println(F("initialization failed!"));
    return;
  }
  Serial.println(FreeRam(),DEC);
  parseFile("config0");
  Serial.println(F("Parse done"));
  writeDb("config0");
  Serial.println(F("Write done"));
  Serial.println(FreeRam(),DEC);
}

void loop() {
  // Nothing to do
  ;
}

I really need suggestion because after playing 10 hours with this now i am in Blackout Stage.

[Reply #1 on:]

Code:

    String stringOne =  String(buf);

Please note that, at present, the String library has bugs as discussed here and here.

In particular, the dynamic memory allocation used by the String class may fail and cause random crashes.

I recommend reworking your code to manage without String. Use C-style strings instead (strcpy, strcat, strcmp, etc.).

[Reply #2 on:]

Thnx Nick Gammon

I recommend reworking your code to manage without String. Use C-style strings instead (strcpy, strcat, strcmp, etc.).

Ok.  One more question
Is this is a way about which you are suggesting? 

Code:

  myFile = SD.open("config2.txt", FILE_READ);

  Serial.println(myFile.size());
  char* buf = (char*) malloc(myFile.size());
  readed = myFile.read(buf,myFile.size());
  Serial.println(readed);
  myFile.close();

[Reply #3 on:]

How big is this file?

[Reply #4 on:]

How big is this file?

At this time less than 300 bytes.

[Reply #5 on:]

Don't do this

Code:

    if (myFile) {

it is at least a bug waiting to happen. You should always use an explicit test such as if (myFile!=NULL)

Mark

[Reply #6 on:]

Code:

char* readFile(char* file)
{
  char strFile[600];
  myFile = SD.open(file,FILE_READ);
  if(myFile)
  {
    stringOne.toCharArray(strFile,stringOne.length()+1);
    return strFile;
  }
  else
    Serial.println("Fail to open file");
}

One path returns something. One does not. On any reasonable system, you'd be warned about that.

As Nick points out, using the String class is a bad idea. It isn't even close to being necessary in your case, since the read function is using a char array and the output is a larger char array. Copying the data from one to another using a for loop instead of dynamic memory allocation is going to waste far fewer resources.

Snipped down like this routine is, though, makes your real problem far easier to spot. You have a local variable, strFile, that goes out of scope when the function ends. Yet, you return a pointer to that memory. When the function ends, you've got a pointer to memory that no longer belongs to a function, and is, therefore free to be reused in any amount anywhere. Not a useful thing to have.

Define the array in the caller, and pass a pointer to the array to this function to fill.

[Reply #7 on:]

After taking break and enjoying weekend now i am out from Blackout Stage.
 

One path returns something. One does not. On any reasonable system, you'd be warned about that.

Ok next time i take care of that.

As Nick points out, using the String class is a bad idea. It isn't even close to being necessary in your case, since the read function is using a char array and the output is a larger char array.

Yes, you are right. (Nick, sorry for PM. I really don't know why there is PM option if this is not useful)

Copying the data from one to another using a for loop instead of dynamic memory allocation is going to waste far fewer resources.

Yes, that's why i tried both ways.

Snipped down like this routine is, though, makes your real problem far easier to spot. You have a local variable, strFile, that goes out of scope when the function ends. Yet, you return a pointer to that memory. When the function ends, you've got a pointer to memory that no longer belongs to a function, and is, therefore free to be reused in any amount anywhere. Not a useful thing to have.

Yes, After taking break and enjoy the weekend to recover mysellf from Blackout Stage i start debugging again and isuue on these lines find as earlier you mention that.

Code:

free(buff);

Code:

free(pch);

Issue solved and i modify it according to your and holmes4 suggestion.

But this problem open lots of issue front of me i start playing with this platform a month ago (but i am not a bad engineer ). Is there really issue of system crash due String, free(), and malloc. In a post http://arduino.cc/forum/index.php/topic,115552 mention by executing this sketch system wil crash.

Code:

#include <MemoryFree.h>
void setup() {
  Serial.begin(9600);
}
void loop() {
  int n = 0;
  while(n <=100) {
    int u = 20*n;
    int v = 30*n;
    String str = String(n) + ", " + String(u) + ", " + String(v) + [color=red]", " + String(freeMemory());[/color]
    Serial.println(str);
    delay(500);
    n++;
  }
  Serial.println("Done");  
}

result comes in favour of post but if it is modify like that system execute smoothly

Code:

#include <SD.h>
void setup() {
  Serial.begin(9600);
}
void loop() {
  int n = 0;
  while(n <=100) {
    int u = 20*n;
    int v = 30*n;
    //int x =
    String str = String(n) + ", " + String(u) + ", " + String(v);
    str        = str + String(n) + ", " + String(u) + ", ";        //coment this line and next indicates test 1
    str        = str + String(n) + ", " + String(u);                //coment only this line indicates test 2.
    str        = str + ", " + String(FreeRam(),DEC);            // execute all lines indicate test3
    Serial.println(str);
    //Serial.println(FreeRam(),DEC);
    delay(100);
    n++;
  }
  Serial.println("Done");  
}

tests output attached in txt format. Actually i want to say that there is some patterns on which system will crash why?
Is this is due to bug OR our mistake logically.
NOTE :Spelling and grammer mistake ignore thnx it's your suggestion which makes my playground play-able

[Reply #8 on:]

Code:

    String str = String(n) + ", " + String(u) + ", " + String(v);
    str        = str + String(n) + ", " + String(u) + ", ";        //coment this line and next indicates test 1
    str        = str + String(n) + ", " + String(u);                //coment only this line indicates test 2.
    str        = str + ", " + String(FreeRam(),DEC);            // execute all lines indicate test3
    Serial.println(str);

Do not use the String class. Is there something about that simple statement that escapes you?

[Reply #9 on:]

Is there something about that simple statement that escapes you?

Yes ,there is some patterns on which system will crash why? Is this is due to bug OR our mistake logically.

Actually i execute the test mention here http://code.google.com/p/arduino/issues/detail?id=857 it's results also fine.

[Reply #10 on:]

Is this is due to bug OR our mistake logically.

Get rid of EVERY SINGLE instance of the String class in your code. Then, if it still has a problem, we'll talk.

[Reply #11 on:]

ok thnx for your suggestion.