Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SD card FILE_WRITE issue #214

Closed
kenb2054 opened this issue Jun 16, 2021 · 15 comments · Fixed by #216
Closed

SD card FILE_WRITE issue #214

kenb2054 opened this issue Jun 16, 2021 · 15 comments · Fixed by #216

Comments

@kenb2054
Copy link

Hi,
I am trying to create a new file for writing on an SD card from my Pico using:

myFile = SD.open("example.txt", FILE_WRITE);

I can successfully initialize the SD Card reader and can open a file for reading and read the data OK but cannot create a new file with the FILE_WRITE command.

I am using code based on the example file known as Files.ino within the Arduino IDE with core ver 1.8.4. I have slightly modified the file to configure the pins for the SPI bus. The fact that I can initialize and read files means the SPI bus is working ok.

Below is the example code I am using and the result is shown further down:

/*
  SD card basic file example
*/
#include <SPI.h>
#include <SD.h>

File myFile;

void setup() {

  delay(5000);    //This delay is just to give time for user to open Serial Monitor window
                   //after uploading the sketch
               
  //Configure pins for SPI comms to SD card when used with Raspberry Pi Pico.
  SPI.setRX(0);    // MISO
  SPI.setCS(1);
  SPI.setSCK(2);
  SPI.setTX(3);    // MOSI

  // Open serial communications and wait for port to open:
  Serial.begin(115200);

  Serial.print("Initializing SD card...");

  if (!SD.begin(1)) {
    Serial.println("initialization failed!");
    return;
  }
  Serial.println("initialization done.");

  if (SD.exists("example.txt")) {
    Serial.println("example.txt exists.");
  } else {
    Serial.println("example.txt doesn't exist.");
  }

  // open a new file and immediately close it:
  Serial.println("Creating example.txt...");
  myFile = SD.open("example.txt", FILE_WRITE);        //  I expected this to create a new file but it didn't in core ver 1.8.4
  myFile.close();

  // Check to see if the file exists:
  if (SD.exists("example.txt")) {
    Serial.println("example.txt exists.");
  } else {
    Serial.println("example.txt doesn't exist.");
  }

  // delete the file:
  Serial.println("Removing example.txt...");
  SD.remove("example.txt");

  if (SD.exists("example.txt")) {
    Serial.println("example.txt exists.");
  } else {
    Serial.println("example.txt doesn't exist.");
  }
}

void loop() {
  // nothing happens after setup finishes.
}

The result I am getting is
FILE_WRITE Result

@earlephilhower
Copy link
Owner

I believe this is just the way the updated SdFAT library behaves. I need to wire it up but I have a vague recollection that it "optimizes" unwritten files out to minimize flash wear. It'll be a few days before I have the time.

I believe you should be fine actually writing and using files, though. Is that fine for you?

@jonathan84clark
Copy link

jonathan84clark commented Jun 18, 2021

I am seeing this issue as well. The library can't create a new file.

@khoih-prog
Copy link
Contributor

khoih-prog commented Jun 18, 2021

Issue is confirmed. Writing new file is not OK. Writing to / Reading from existing file is OK

I'm testing with this code

#include <SPI.h>
#include <SD.h>

File root;

void setup() 
{
  // Open serial communications and wait for port to open:
  Serial.begin(115200);
  while (!Serial);

  delay(1000);

  Serial.print("Initializing SD card with SS = "); Serial.println(PIN_SPI0_SS);
  Serial.print("SCK = "); Serial.println(PIN_SPI0_SCK);
  Serial.print("MOSI = "); Serial.println(PIN_SPI0_MOSI);
  Serial.print("MISO = "); Serial.println(PIN_SPI0_MISO);

  if (!SD.begin(PIN_SPI0_SS))
  {
    Serial.println("initialization failed!");
    return;
  }
  
  Serial.println("initialization done.");

  root = SD.open("/");

  printDirectory(root, 0);

  Serial.println("done!");

  // OK here to write existing file
  File myWFile = SD.open("datalog.txt", "w");

  if (myWFile)
  {
    char writeData[]  = "Testing RP2040 SD";
  
    myWFile.write((uint8_t *) &writeData, sizeof(writeData));

    myWFile.close();
  }
  else
  {
    Serial.println("Error open for writing datalog.txt...");
  }

  // Error here to write new file
  myWFile = SD.open("newdatalog.txt", "w");

  if (myWFile)
  {
    char writeData[]  = "Testing RP2040 SD";
   
    myWFile.write((uint8_t *) &writeData, sizeof(writeData));

    myWFile.close();
  }
  else
  {
    Serial.println("Error open for writing newdatalog.txt...");
  }

  // OK here to read file
  File myRFile = SD.open("datalog.txt", "r"); 
  
  if (myRFile)
  {
    char readData[64]  = "\0";

    //myWFile.seek(0, (SeekMode) SEEK_END);
    
    myRFile.read((uint8_t *) &readData, sizeof(readData) - 1);

    myRFile.close();

    Serial.print("readData = "); Serial.println(readData);
  }
  else
  {
    Serial.println("Error open for reading datalog.txt...");
  }
}

void loop() 
{
  // nothing happens after setup finishes.
}

void printDirectory(File dir, int numTabs) 
{
  while (true) 
  {
    File entry =  dir.openNextFile();

    if (! entry) 
    {
      // no more files
      break;
    }
    
    for (uint8_t i = 0; i < numTabs; i++) 
    {
      Serial.print('\t');
    }
    
    Serial.print(entry.name());
    
    if (entry.isDirectory()) 
    {
      Serial.println("/");
      printDirectory(entry, numTabs + 1);
    } 
    else 
    {
      // files have sizes, directories do not
      Serial.print("\t\t");
      Serial.print(entry.size(), DEC);
      time_t cr = entry.getCreationTime();
      time_t lw = entry.getLastWrite();
      struct tm * tmstruct = localtime(&cr);
      Serial.printf("\tCREATION: %d-%02d-%02d %02d:%02d:%02d", (tmstruct->tm_year) + 1900, (tmstruct->tm_mon) + 1, tmstruct->tm_mday, tmstruct->tm_hour, tmstruct->tm_min, tmstruct->tm_sec);
      tmstruct = localtime(&lw);
      Serial.printf("\tLAST WRITE: %d-%02d-%02d %02d:%02d:%02d\n", (tmstruct->tm_year) + 1900, (tmstruct->tm_mon) + 1, tmstruct->tm_mday, tmstruct->tm_hour, tmstruct->tm_min, tmstruct->tm_sec);
    }
    
    entry.close();
  }
}

the debug output

Initializing SD card with SS = 17
SCK = 18
MOSI = 19
MISO = 16
SDFS::begin: _mounted = true
initialization done.
datalog.txt		108	CREATION: 2021-06-18 03:28:32	LAST WRITE: 2098-01-01 00:00:04
mydatalog.txt		0	CREATION: 2021-06-18 03:28:38	LAST WRITE: 2021-06-18 03:28:32
done!
Error open for writing newdatalog.txt...
readData = Testing RP2040 SD

@jonathan84clark
Copy link

Try appending to a file, that's broken as well. I'm happy that there is some support for the RP2040 but without the ability to append to a file...its going to be really tough to do anything all that useful.

@khoih-prog
Copy link
Contributor

I tested and saw appending is OK. Just seek using SEEK_END before writing.

  // OK here to write / append to existing file
  File myWFile = SD.open("datalog.txt", "w");

  if (myWFile)
  {
    char writeData[]  = "Testing RP2040 SD";
  
    myWFile.seek(0, (SeekMode) SEEK_END);

    myWFile.write((uint8_t *) &writeData, sizeof(writeData));

    myWFile.close();
  }
  else
  {
    Serial.println("Error open for writing datalog.txt...");
  }

@jonathan84clark
Copy link

I tested and saw appending is OK. Just seek using SEEK_END before writing.

  // OK here to write / append to existing file
  File myWFile = SD.open("datalog.txt", "w");

  if (myWFile)
  {
    char writeData[]  = "Testing RP2040 SD";
  
    myWFile.seek(0, (SeekMode) SEEK_END);

    myWFile.write((uint8_t *) &writeData, sizeof(writeData));

    myWFile.close();
  }
  else
  {
    Serial.println("Error open for writing datalog.txt...");
  }

Thanks! I'll give it a shot.

@earlephilhower
Copy link
Owner

Very odd, but thanks for the report. The SD.h is taken from the ESP8266 where, AFAIK, we have been running just fine for over a year w/o any changes. I need to blue-wire up a testbed this weekend to look at it.

@earlephilhower
Copy link
Owner

Thanks, guys. I am able to reproduce this and will look into it. The issue seems to be in SDFS or SdFat, not SD, so that at least narrows things down a tiny bit.

@khoih-prog
Copy link
Contributor

khoih-prog commented Jun 19, 2021

Great.

I'm just finished porting Arduino SD library to use for both this core and Arduino-mbed RP2040 core.
In the final step to clean up. Testing OK so far with writing / reading for both cores.

Just a back up solution in case you don't have time to spend on. If you'd like, I'll create PR later if everything is OK.

This arduino-pico core

Initializing SD card with SS = 17
SCK = 18
MOSI = 19
MISO = 16
initialization done.
Writing to newtest0.txt ==> Testing writing to newtest0.txt
done.
Reading from newtest0.txt
===============
Testing writing to newtest0.txt
Testing writing to newtest0.txt
Testing writing to newtest0.txt
Testing writing to newtest0.txt
===============

Arduino-mbed core

Initializing SD card with SS = 5
SCK = 2
MOSI = 3
MISO = 4
initialization done.
Writing to newtest0.txt ==> Testing writing to newtest0.txt
done.
Reading from newtest0.txt
===============
Testing writing to newtest0.txt
Testing writing to newtest0.txt
Testing writing to newtest0.txt
Testing writing to newtest0.txt
Testing writing to newtest0.txt
===============

earlephilhower added a commit that referenced this issue Jun 19, 2021
The O_CREAT/etc. flags on the Pico are a full 32-bits in size, but the
core was generating a flag for SdFat using an 8-bit type, so all the
O_CREAT, O_TRUNC, O_APPEND, etc. flags got cut off.

Fix the flag size.

Fixes #214
@earlephilhower
Copy link
Owner

Thanks @khoih-prog , but it was a pretty silly bug on my part which resulted in a 2-line fix in my own code.

Basically, the POSIX flags need a full 32bits to represent them (i.e. O_CREAT=0x400), but I was using a uint8_8 type (since on the ESP8266 where this came from, we defined them that way).

When CI finishes I'll push a bugfix release. @kenb2054 and @khoih-prog if you could verify yourselves afterwards, it would be much appreciated. I ran the code here and it does work fine now:

12:07:51.359 -> Initializing SD card...
12:07:51.392 -> initialization done.
12:07:51.392 -> example.txt doesn't exist.
12:07:51.392 -> Creating example.txt...
12:07:51.392 -> example.txt exists.
12:07:51.392 -> Removing example.txt...
12:07:51.392 -> example.txt doesn't exist.

earlephilhower added a commit that referenced this issue Jun 19, 2021
The O_CREAT/etc. flags on the Pico are a full 32-bits in size, but the
core was generating a flag for SdFat using an 8-bit type, so all the
O_CREAT, O_TRUNC, O_APPEND, etc. flags got cut off.

Fix the flag size.

Fixes #214
@earlephilhower
Copy link
Owner

Reopening for your feedback on 1.8.6

@khoih-prog
Copy link
Contributor

Amazingly fast. I test and report to you ASAP.

@khoih-prog
Copy link
Contributor

Confirmed the bug is gone using core v1.8.6

@earlephilhower
Copy link
Owner

Thx for confirming!

@kenb2054
Copy link
Author

Thanks for the fix. It's through things like this that newbies like me learn what's under the hood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants