Weird issues using serial input with SD library

al3xsh · March 19, 2012, 9:45am

I'm currently trying to read data from the serial monitor and write it to a file on the SD card using the rough structure below:

// setup everything

// main program

// -- read string from serial monitor byte by byte until we get cr or nl
// -- once our string is finished, echo to serial
// -- open sd card file and append the string to the end (and close the file)

Everything works fine if I initialise the file from within the setup loop:

File dataFile 

void setup() { 

	<snip ...>

	dataFile = SD.open("log0.txt", FILE_WRITE);

}

and then write to it in the main loop:

void loop() {

	<snip ...>

	dataFile.println("string");
	dataFile.flush();

}

However if I try initialising the file within the main loop (as in the examples) I end up with weird ascii characters at the end of my character array.

void loop() {

	<snip ...>

	File dataFile = SD.open("log0.txt", FILE_WRITE);

	// if the file is available then write to it
	if (dataFile) {
		dataFile.println(str);
		dataFile.close();
	}  
	// if the file isn't open then pop up an error
	else {
		Serial.println("error opening datalog.txt");
	}

}

Could anybody explain this behaviour to me? At the moment I am working around it by not closing the file and instead calling flush() after I have tried to write the string to the file to ensure all the data is written. However, I know this isn't best practice and I am sure this will come back to bite me ...

I have provided the complete programs below for reference.

Thanks,

Alex

Working:

#include <SD.h>

// chip select is pin 4 on the ethernet shield
const int chipSelect = 4;

// the maximum data buffer is 1024 bytes
const int dataSize = 256;

// initialise data buffer
byte buff[dataSize];
int ix;

byte cr = 13; // carriage return
byte nl = 10; // new line

File dataFile;

void setup() {

// setup serial port
Serial.begin(9600);
Serial.println("serial comms working ...");

// initialise ...
ix = 0;
pinMode(10, OUTPUT);

// see if the card is present and can be initialized
if (!SD.begin(chipSelect)) {
return;
}
Serial.println("card initialized ...");

// for some reason opening the sd card file in the main loop goes weird.
// characters in the serial monitor are strange
dataFile = SD.open("log0.txt", FILE_WRITE);

}

void loop() {

// check that the serial port has data waiting to be read
while (Serial.available() > 0) {

// read a byte and, if it's not the line ending character,
// add it to the buffer
byte inByte = Serial.read();
if (inByte != cr && inByte != nl) {
buff[ix] = inByte;
ix++;
}
else {

// string conversion
char str[ix-1];
for(int i = 0; i < ix; i++) {
str_ = (char)buff*;_
_ }_
_ // Serial output*_
_ Serial.print("echo >>> ");
Serial.println(str);
* // if the file is available then write to it*
* if (dataFile) {*
* dataFile.println(str);*
* dataFile.flush();*
* } *
* // if the file isn't open then pop up an error*
* else {*
Serial.println("error opening datalog.txt");
* }*
* // reset the index*
* ix = 0;*
* }*
* }*
}
[/quote]
Producing weirdness:
> #include <SD.h>
>
*> // chip select is pin 4 on the ethernet shield *
> const int chipSelect = 4;
>
> // the maximum data buffer is 1024 bytes
> const int dataSize = 256;
>
> // initialise data buffer
> byte buff[dataSize];
> int ix;
>
> byte cr = 13; // carriage return
> byte nl = 10; // new line
>
> void setup() {
>
> // setup serial port
> Serial.begin(9600);
> Serial.println("serial comms working ...");
>
> // initialise ...
> ix = 0;
> pinMode(10, OUTPUT);
*> *
> // see if the card is present and can be initialized
*> if (!SD.begin(chipSelect)) { *
> return;
> }
> Serial.println("card initialized ...");
>
> }
>
> void loop() {
*> *
> // check that the serial port has data waiting to be read
> while (Serial.available() > 0) {
>
> // read a byte and, if it's not the line ending character,
> // add it to the buffer
> byte inByte = Serial.read();
> if (inByte != cr && inByte != nl) {
> buff[ix] = inByte;
> ix++;
> }
> else {
*> *
> // string conversion
> char str[ix-1];
*> for(int i = 0; i < ix; i++) { *
> str = (char)buff*;_
> _ }_
> _ // Serial output*
> Serial.print("echo >>> ");
> Serial.println(str);
> * // open the file. note that only one file can be open*
> * // at a time so you have to close this one before*
> * // opening another.*
> * File dataFile = SD.open("log0.txt", FILE_WRITE);
> _ // if the file is available then write to it*
> * if (dataFile) {*
> * dataFile.println(str);*
> * dataFile.close();*
> * } *
> * // if the file isn't open then pop up an error*
> * else {*
> _ Serial.println("error opening datalog.txt");_
> * }*
> * // reset the index*
> * ix = 0;*
> * }*
> * }*
> }
> [/quote]

nickgammon · March 19, 2012, 9:52am

// string conversion
      char str[ix-1];
      for(int i = 0; i < ix; i++) {        
        str = (char)buff;
      }

Just run by me what this is doing?

al3xsh · March 19, 2012, 10:06am

It should read:

      for(int i = 0; i < ix; i++) {        
        str[i] = (char)buff[i];
      }

I'm building up a character array from the raw byte array that I've read in from the serial monitor.

// initialise a character array of the correct size
// for every byte in the byte array that we've read in ...
// ... cast to a char and assign to our character array

Thanks,

Alex

nickgammon · March 19, 2012, 10:09am

Why bother?

system · March 19, 2012, 10:16am

The println() function that writes to the SD card expects a string. You really need to learn the difference between a string and an array of chars. A string is an array of chars THAT IS NULL TERMINATED. You do not have a string. Therefore, you can not expect the println() function to behave as though you did pass it a string.

It IS doing exactly what you told it to do, even though what you told it to do was wrong.

al3xsh · March 19, 2012, 10:17am

Ahh metaphysics ...

Because there is no dynamic array class I have initialised my array to be bigger than I need. So, therefore, I only want to print the first (ix - 1) elements to the file (ix-1 being the number of bytes I have read from the serial port).

al3xsh · March 19, 2012, 10:19am

@paulS

I have also tried constructing a string from my character array:

String properString = String(str);

before using println. I still get the same error (I think).

system · March 19, 2012, 10:24am

I have also tried constructing a string from my character array:

String properString = String(str);

There is a world of difference between a string and a String. The String constructor that you are using expects a string as input. You are not giving it a string. You are giving it an array of characters and no length.

So, the constructor is doing exactly what println() is doing. It starts at the start of the array, and marches along until it finds a NULL. That NULL is well beyond the end of your array, so, of course you get "garbage" in your String/file.

I can't believe the number of people that try to run marathons before they have even learned to tie their shoes.

NULL terminating an array of chars is such a fundamental part of string processing that it amazes me how many people do not think that it is necessary.

al3xsh · March 19, 2012, 11:14am

@PaulS

Thanks for your scathing assessment of my knowledge and code. You sure know how to make someone feel welcome in the community ...

Before posting on the forum I spent a while working through the String constructor tutorial on the Arduino website, but there is no mention there of null terminating strings. However, following your advice, I have added '\0' to the end of my character array to null terminate the string and it now works.

      // string conversion
      char str[ix];
      for(int i = 0; i < ix; i++) {        
        str[i] = (char)buff[i];
      }
      str[ix] = '\0';

Thanks,

Alex

system · March 19, 2012, 12:42pm

Before posting on the forum I spent a while working through the String constructor tutorial on the Arduino website, but there is no mention there of null terminating strings.

I don't know what tutorial you are referring to, but I suspect that it expects you to be familiar with strings. They are a fundamental part of C. Before doing anything with char arrays, you need to understand that they must be NULL terminated.

In every C programming book I've looked at, the need to NULL terminate the char array is mentioned on the same page as the char array itself. The need to NULL terminate the array is emphasized heavily, and CLEARLY illustrated in every single example.

Not knowing this is, in my opinion, inexcusable.

al3xsh · March 19, 2012, 6:40pm

@pauls and here was me thinking arduino was for beginners and artist types...

Much of my programming experience is with java and c# so I've not spent much time dealing with low level bytes and chars.

I still am seeing some quite strange behaviour with processing bytes and chars into strings and writing them to the sd card. I'll go read up on strings / char[]) in c...

Alex

system · March 19, 2012, 6:53pm

and here was me thinking arduino was for beginners and artist types...

That's a myth propagated by the Arduino team, but that is not supported by the evidence.

There are some things that you can do with little experience. Serial data processing is not one of them. A thorough understanding of at least the first three or four chapters of a good C book is required.

In spite of my sometimes overly harsh comments, I am perfectly happy trying to help newbies (hey, I was one, once, too). All I expect is that you have a rudimentary understanding of C OR the ability to say "how do I do that?".

nickgammon · March 19, 2012, 7:47pm

al3xsh:
@PaulS

Thanks for your scathing assessment of my knowledge and code. You sure know how to make someone feel welcome in the community ...

Your question was quite well framed compared to some we see here every day.

http://www.catb.org/~esr/faqs/smart-questions.html

But to be fair to PaulS, who I think responds to all but the most idiotic questions, he must get a bit tired of explaining the same thing every day. And yes, I know that it is often to different people. In some cases (not this one) the thread is started by someone who clearly has not made any attempt at all to research their problem. Stuff like "Can you write code for me from scratch to read an RFID reader, output to a printer, and get the valid card numbers from an SD card? And I need it done by the end of the week".

Part of the problem is that the Arduino documentation doesn't make clear (it mentions it in passing) that you are dealing with C++ and C here. The main page for Arduino states:

The microcontroller on the board is programmed using the Arduino programming language (based on Wiring) and the Arduino development environment (based on Processing).

You have to dig a bit deeper to realize "ah it's really C++".

And if you knew that, you might go grab some C++ tutorials.

al3xsh · March 20, 2012, 11:53am

I have now had a good look at handling char[]s in C/C++ - thanks for the pointers. Hopefully I now understand them a bit better.

I think my code now works. Much of the extremely strange behaviour with the SD card seems to be due to using too big a character buffer. If I use 256 or 512 for STRLEN, the code below works fine. However, if I use 1024 everything starts misbehaving (I seem to get the setup function constantly being called). I assume this is something to do with program memory being overwritten (?)

Thanks for your help.

Alex

#include <SD.h>

// string length is 256 bytes
#define STRLEN 256

// chip select is pin 4 on the ethernet shield 
const int chipSelect = 4;

// initialise the string (really a char array!)
char buffer[ STRLEN ];
int  bufferIndex = 0;

// set up the arduino
void setup() {

  // set up serial
  Serial.begin(9600);
  Serial.println("serial comms working ... ");
  
  // pin 10 must be output for the SD card!
  pinMode(10, OUTPUT);
  
  // see if the card is present and can be initialized
  if (!SD.begin(chipSelect)) {    
    return;  // do nothing ...
  }
  Serial.println("card initialized ...");
  
}

// main program loop
void loop() {
  
  // whilst there is serial data waiting ...
  if (Serial.available()) {
    
    // read character from serial monitor
    char ch = Serial.read();
    
    // if the character is a carriage return or new line
    if (ch == '\r' || ch == '\n') {
      
      // terminate the string with a 0 (null terminated)
      buffer[ bufferIndex ] = 0; 
      
      // Serial echo output
      Serial.print("echo >>> ");
      Serial.println(buffer);

      // open the file - note that only one file can be open 
      // at a time so you have to close this one before 
      // opening another
      Serial.println("opening file ...");
      File dataFile = SD.open("log1.txt", FILE_WRITE);

      // if the file is available then write to it
      if (dataFile) {
        Serial.println("writing to file ...");
        dataFile.println(buffer);
        dataFile.close();
      }  
      // if the file isn't open then pop up an error
      else {
        Serial.println("error opening file");
      }
     
      // reset the index ready for another string          
      Serial.println("resetting index ...");
      bufferIndex = 0;               
            
    }
    
    // if not a carriage return or new line then add the 
    // character to the buffer 
    else {
      
      buffer[ bufferIndex++ ] = ch;
      
    }
  }
}

system · March 20, 2012, 12:34pm

However, if I use 1024 everything starts misbehaving

You have 2K of memory on the Arduino (unless you have a Mega). Dedicating 1/2 of it to an array that way will cause problems.

The SD library writes data to the SD card in 512 byte increments. That requires a buffer to hold the data. That buffer is 1/4 of the memory on the Arduino (unless you have a Mega). There's 3/4 of your memory gone already.

    // if not a carriage return or new line then add the 
    // character to the buffer 
    else {
      
      buffer[ bufferIndex++ ] = ch;
      
    }

It is generally a good idea to make sure that there is room in the buffer before you add the character.

al3xsh · March 20, 2012, 12:46pm

Thank you - that explains a lot!

Would the following code be OK for checking that there is space in the buffer?

    // if not a carriage return or new line then add the 
    // character to the buffer 
    else {
      
        // if there is room in the buffer for this character (plus the null terminator)
        // then copy the character into the buffer
        if (bufferIndex < (STRLEN - 1)) {
            buffer[ bufferIndex++ ] = ch;
        }
      
    }

Thanks again,

Alex

system · March 20, 2012, 12:58pm

Would the following code be OK for checking that there is space in the buffer?

Almost.

Suppose STRLEN was 10. Suppose bufferIndex is currently 8. That means that the array positions 0 to 8 are currently being used. 8 is less than 9, so, the character will be placed in position 9. Where are you now going to put the NULL? The array is full.

You need "- 2" in that statement, not "- 1", because you need to have room for two characters - the one to add and the terminating NULL.

al3xsh · March 20, 2012, 2:31pm

Thanks Paul!

Alex

Topic		Replies	Views
reading text from an sd card Programming	21	14138	May 5, 2021
strange characters reading sd file Storage	3	1550	May 6, 2021
Incomplete data write to SD carrd Storage	15	2754	May 6, 2021
Reading Sd file to serial port Programming	11	3364	May 5, 2021
Serial Streaming question Programming	68	15462	May 5, 2021

Weird issues using serial input with SD library

Related topics