Problem when reading from the SD card

Hello everybody,
I am fresh in the Arduino world and have some issues with SD card data manipulation. I am running simple webserver using Arduino MEGA and W5100 ethernet shield. I picked code from the web and made some modifications.
I log the data to the SD card and display it via webserver (with "client.print()" function).

I want to read the data from the SD card in larger chunks using the "int FatFile::read(void* buf, size_t nbyte)" from the "SdFat" library.
This is my sketch (only webserver part):

#define DEBUG_ENABLE 1 // enable debugging over serial

////////////////////DEBUG MACRO////////////////////////////
#if DEBUG_ENABLE
#define debug(x)     Serial.print(x)
#define debugln(x)   Serial.println(x)
#else
#define debug(x)
#define debugln(x)
#endif 
/////////////////////////WEBSERVER/////////////////////////
#include <Ethernet.h>
#include <SPI.h>
#include <SdFat.h>

byte mac[] = {0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED};
IPAddress ip(192, 168, 100, 22);
EthernetServer server(80); //port 80

SdFat SD; //class for initialization of card, root, volume
boolean sdCardOK = true;

void setup() {
  #if DEBUG_ENABLE
  Serial.begin(9600); // DEBUG Open serial communications and wait for port to open
  #endif
	
  pinMode(10, OUTPUT);
  digitalWrite(10, HIGH); //disable W5100 ethernet before SD begin - MEGA!!!
	
  if (!SD.begin(4, SPI_HALF_SPEED)) { // check if the card is present and can be initialized:
    debugln(F("SD card failed, or not present")); //DEBUG
		sdCardOK = false;
  }
	
  Ethernet.begin(mac, ip);// start the Ethernet connection and the server:
  server.begin();
}

void loop() {
	webLoggerServer();
}

void webLoggerServer() {
  EthernetClient client = server.available(); // listen for incoming clients
  if (client) {
    const byte bufferSize = 50;
    char clientline[bufferSize];
    int index = 0; //buffer reset position
    while (client.connected()) {
      if (client.available()) {
        char c = client.read();
        if (c != '\n' && c != '\r') { // If it isn't a new line, add the character to the buffer
          clientline[index] = c;
          index++;
          if (index >= bufferSize) { // are we too big for the buffer? start tossing out data
            index = bufferSize -1;
          }
          continue; // continue to read more data!
        }
        debugln(index);
        clientline[index] = 0; // got a \n or \r new line, which means the string is done
        debugln(clientline); //DEBUG Print it out for debugging
        if (strstr(clientline, "GET / ") != 0) {
          client.println(F("HTTP/1.1 200 OK\r\nContent-Type: text/html; charset=US-ASCII\r\nConnection: close\r\n")); //http response header
        }
        else if (strstr(clientline, "GET /") != 0 && sdCardOK) {
          char *filename;
          filename = clientline + 5; // look after the "GET /" (5 chars)
          // a little trick, look for the " HTTP/1.1" string and 
          // turn the first character of the substring into a 0 to clear it out.
          (strstr(clientline, " HTTP"))[0] = 0;
          debugln(filename); //DEBUG print the file we want
          SdFile requestedFile;  
          if (requestedFile.open(filename, O_READ)) {
            debugln(F("File opened!")); //DEBUG
            client.println(F("HTTP/1.1 200 OK\r\nContent-Type: text/plain; charset=US-ASCII\r\nConnection: close\r\n"));
            //int16_t c; 
            //while ((c = requestedFile.read()) > 0) //read and print byte by byte - too slow!
              //client.print((char)c);
            char readBuffer[50];
            while (requestedFile.available()) {
              requestedFile.read(readBuffer,sizeof(readBuffer)); // SOMETHING IS WRONG HERE?
              //requestedFile.fgets(readBuffer,sizeof(readBuffer)); //THIS WORKS
              //debugln(readBuffer);
              client.print(readBuffer);
            }
            requestedFile.close();
					}
          else {
            print404Html(client);
					}
        }
        else { // everything else is a 404
          print404Html(client);
        }
        break;
      }
    }
    delay(1); // give the web browser time to receive the data
    client.stop(); // close the connection:
  }
}

void print404Html(EthernetClient client) {
  client.println(F("HTTP/1.1 404 Not Found\r\nContent-Type: text/html; charset=US-ASCII\r\n")); //response header
  client.println();
	client.println(F("<!DOCTYPE HTML><h2>File Not Found!</h2>"));
}

I expect to get data (in web browser) in a form like this:

1433542612;27.37;29.00;
1433542913;27.25;29.00;
1433543214;27.25;29.00;
1433543515;27.31;29.25;
1433543816;27.31;29.50;
1433544117;27.00;29.25;
...

but instead I get this:

1433542612;27.37;29.00;
1433542913;27.25;29.00;
GET /2015_JUN.TXT1433543214;27.25;29.00;
1433543515;27.31;29.25;
GET /2015_JUN.TXT1433543816;27.31;29.50;
1433544117;27.00;29.25;
...

"GET /2015_JUN.TXT" is inserted after each segment (50 bytes buffer), which is part of the http header (file name).
Why is this happening?

By the way, everything is fine if "fgets" function is used (but reading line by line is slower than using a larger buffer).

I don't know the answer, but what you read at the debugln()? The same thing or a different thing? Is a problem with what you read or with what you write (to the client)?

Result is the same (wrong) when printing to serial -> "debugln(readBuffer)".

while (requestedFile.available()) {
              requestedFile.read(readBuffer,sizeof(readBuffer)); // SOMETHING IS WRONG HERE?
              //requestedFile.fgets(readBuffer,sizeof(readBuffer)); //THIS WORKS
              //debugln(readBuffer);
              client.print(readBuffer);
            }

You need to think about what this will do. You check if there is data ( any data ) in the file, and then read 50 characters. What happens when there is only one character in the file ? I'm not sure what the answer to that question is, but I think that is your problem.

That would also explain why you don't have a problem when you use fgets( )

if (strstr(clientline, "GET / ") != 0) {
          client.println(F("HTTP/1.1 200 OK\r\nContent-Type: text/html; charset=US-ASCII\r\nConnection: close\r\n")); //http response header
        }
else if (strstr(clientline, "GET /") != 0 && sdCardOK) {

The logic behind this is elusive. If the first if( ) statement is true, then the second if( ) statement might be true, but since the first if( ) statement is true, you will never be considering the second one.

The other problem you might have, is that when you fill your 50-char array with text, there is nothing at the end of it. No zero terminating byte. So when you try to print( ) this, the print function will ramble on through memory from the start of the 50-char array, and keep going at the end until it happens to come to a byte in RAM which contains a zero.

There are various ways around that, I am sure.

My approach would be to read one line of the file at a time, not 50 characters which would be 2 and half lines and end randomly somewhere in the middle of a line. There are other ways.

client.print(readBuffer);

It may be an issue, that there is never a "println" anywhere in this sequence of events.

michinyon, thanks for your response.

You are right, condition check in IF has to be put in order.

Probably I'll use fgets in the end.

I played a little more with the read function.

"readBuffer" array is not zero terminated, like you said, so I tried this:

while (requestedFile.available()) {
  requestedFile.read(readBuffer,sizeof(readBuffer)-1);
  readBuffer[50] = '\0';
  client.print(readBuffer)

Output is still not correct:

1433542612;27.37;29.00;
1433542913;27.25;29.00;
 
1433543214;27.25;29.00;
1433543515;27.31;29.25;
1433543816;27.31;29.50;
1433544117;27.00;29.25 ;
1433544418;26.69;29.25;
1433544719;26.56;29.2 5;
1433545020;26.50;29.00;
1433545321;26.44;29. 00;

Empty space is shown as "ENQ" in notepad++.

I found the problem, it should be like this:

const byte bufSize = 50;
char readBuffer[bufSize];

while (requestedFile.available()) {
  int n = requestedFile.read(readBuffer,bufSize - 1);
  readBuffer[n] = '\0';
  client.print(readBuffer);
}

And if the function requestedFile.read() returns error (I think that in that case will return -1)?

readBuffer[50] = '\0';

well, if you do this, you are overwriting whatever is in the byte, which is after the end of your array.

Reply #8 looks good to me. Actually, it doesn't. Check the result isn't -1 , first. And, if the result is -1 or 0, don't send the data to the client, there isn't any.

What about this:

const byte bufSize = 50;
char readBuffer[bufSize];

while (requestedFile.available()) {
  int n = requestedFile.read(readBuffer,bufSize - 1);
  if (n > 0) {
    readBuffer[n] = '\0';
    client.print(readBuffer);
  }
}

You may need to test if n=0 too, don't?