Correction proposition method


About the method documentation:

I believe a part is missing where the return value is concerned.

Current :

The next byte (or character), or -1 if none is available.

Should be:


When used as ""
The next byte (or character), or -1 if none is available. 

When used as ", len)"
The amount of bytes read, or -1 if an error occured.

The proposed correction appears to be correct, and three additional corrections may also be useful.

1st: The documentation states that inherits its functionality from the Stream class which is inaccurate. The documentation should say that is inherited from Stream, and that, len) is not inherited from and is not inherited from Stream.readBytes(buffer, length).

From a brief look through File.cpp, SD.h, SD.cpp, and SdFile.cpp it appears that, length) functionality is defined in SDFile.cpp, where the source code uses functions defined elsewhere in the class to follow the FAT block chain for the file and to manage the physically reading data blocks from the SD card and caching them.

2nd - The documentation should describe buffer as any data type, and length as an unsigned 16 bit integer not greater than 32767 (0x7FFF).

From the library source code, the buffer is (void *) and recast to an unsigned byte pointer for data copying. The length parameter is coded as an unsigned 16 bit integer but the signed 16 bit return value can't be guaranteed when the length can't be represented as a positive 16 bit signed integer value and the conversion may cause an exception on some processors.

The code for file position is unsigned 32 bit math and the number of bytes read is an unsigned 16 bit value, but the return value is a signed 16 bit value. It's possible to code the conversion to guarantee that the unsigned to signed conversion of the return value can be cast to an unsigned integer and raise the limit on the length parameter to 65534 (0xFFFE), but its probably not a productive exercise. It seems more reasonable to expect that the SD card library will be replaced to support file sizes > 4GB than that a future AVR architecture Arduino to have more than 32K SRAM and SD card reads larger than 32K but less than 64K would be a desirable change.

3rd: The documentation states -1 is returned when no bytes are available, but that description isn't accurate, data may have been copied to the buffer when an error occurs. It would be more accurate to say that "-1 is returned when an error other than end-of-file is encountered before reading the number of bytes requested".

The source code notes that -1 is returned for all errors, including file not open, I/O error, and corrupt file system. That isn't the same as no data available. From reading the source code, when the FAT block chain is shorter than the file length in the directory entry, the function will copy data from the current file position to the end of the FAT block chain to the buffer before returning -1. The function also returns -1 when data has been read from either a buffer or the SD card before an I/O error prevents reading additional data.