Fix several SD card issues (espressif#4876)

- File might not eval to false if opened with write/append and SD is gone
- Allow card to be formatted if FAT partition was not found
- Mark card as gone in certain situations
- Fix several logic errors in low level SD API

Author: me-no-dev

libraries/FS/src/FS.cpp

@@ -25,7 +25,7 @@ using namespace fs;
 
 size_t File::write(uint8_t c)
 {
-    if (!_p) {
+    if (!*this) {
         return 0;
     }
 
@@ -34,7 +34,7 @@ size_t File::write(uint8_t c)
 
 time_t File::getLastWrite()
 {
-    if (!_p) {
+    if (!*this) {
         return 0;
     }
 
@@ -43,7 +43,7 @@ time_t File::getLastWrite()
 
 size_t File::write(const uint8_t *buf, size_t size)
 {
-    if (!_p) {
+    if (!*this) {
         return 0;
     }
 
@@ -52,7 +52,7 @@ size_t File::write(const uint8_t *buf, size_t size)
 
 int File::available()
 {
-    if (!_p) {
+    if (!*this) {
         return false;
     }
 
@@ -61,7 +61,7 @@ int File::available()
 
 int File::read()
 {
-    if (!_p) {
+    if (!*this) {
         return -1;
     }
 
@@ -75,7 +75,7 @@ int File::read()
 
 size_t File::read(uint8_t* buf, size_t size)
 {
-    if (!_p) {
+    if (!*this) {
         return -1;
     }
 
@@ -84,7 +84,7 @@ size_t File::read(uint8_t* buf, size_t size)
 
 int File::peek()
 {
-    if (!_p) {
+    if (!*this) {
         return -1;
     }
 
@@ -96,7 +96,7 @@ int File::peek()
 
 void File::flush()
 {
-    if (!_p) {
+    if (!*this) {
         return;
     }
 
@@ -105,7 +105,7 @@ void File::flush()
 
 bool File::seek(uint32_t pos, SeekMode mode)
 {
-    if (!_p) {
+    if (!*this) {
         return false;
     }
 
@@ -114,7 +114,7 @@ bool File::seek(uint32_t pos, SeekMode mode)
 
 size_t File::position() const
 {
-    if (!_p) {
+    if (!*this) {
         return 0;
     }
 
@@ -123,7 +123,7 @@ size_t File::position() const
 
 size_t File::size() const
 {
-    if (!_p) {
+    if (!*this) {
         return 0;
     }
 
@@ -140,12 +140,12 @@ void File::close()
 
 File::operator bool() const
 {
-    return !!_p;
+    return _p != nullptr && *_p != false;
 }
 
 const char* File::name() const
 {
-    if (!_p) {
+    if (!*this) {
         return nullptr;
     }
 
@@ -155,23 +155,23 @@ const char* File::name() const
 //to implement
 boolean File::isDirectory(void)
 {
-    if (!_p) {
+    if (!*this) {
         return false;
     }
     return _p->isDirectory();
 }
 
 File File::openNextFile(const char* mode)
 {
-    if (!_p) {
+    if (!*this) {
         return File();
     }
     return _p->openNextFile(mode);
 }
 
 void File::rewindDirectory(void)
 {
-    if (!_p) {
+    if (!*this) {
         return;
     }
     _p->rewindDirectory();

libraries/SD/src/SD.cpp

@@ -22,7 +22,7 @@ using namespace fs;
 
 SDFS::SDFS(FSImplPtr impl): FS(impl), _pdrv(0xFF) {}
 
-bool SDFS::begin(uint8_t ssPin, SPIClass &spi, uint32_t frequency, const char * mountpoint, uint8_t max_files)
+bool SDFS::begin(uint8_t ssPin, SPIClass &spi, uint32_t frequency, const char * mountpoint, uint8_t max_files, bool format_if_empty)
 {
     if(_pdrv != 0xFF) {
         return true;
@@ -35,7 +35,7 @@ bool SDFS::begin(uint8_t ssPin, SPIClass &spi, uint32_t frequency, const char *
         return false;
     }
 
-    if(!sdcard_mount(_pdrv, mountpoint, max_files)){
+    if(!sdcard_mount(_pdrv, mountpoint, max_files, format_if_empty)){
         sdcard_unmount(_pdrv);
         sdcard_uninit(_pdrv);
         _pdrv = 0xFF;

libraries/SD/src/SD.h

@@ -29,7 +29,7 @@ class SDFS : public FS
 
 public:
     SDFS(FSImplPtr impl);
-    bool begin(uint8_t ssPin=SS, SPIClass &spi=SPI, uint32_t frequency=4000000, const char * mountpoint="/sd", uint8_t max_files=5);
+    bool begin(uint8_t ssPin=SS, SPIClass &spi=SPI, uint32_t frequency=4000000, const char * mountpoint="/sd", uint8_t max_files=5, bool format_if_empty=false);
     void end();
     sdcard_type_t cardType();
     uint64_t cardSize();

libraries/SD/src/sd_diskio.cpp

@@ -60,6 +60,31 @@ typedef struct {
 
 static ardu_sdcard_t* s_cards[FF_VOLUMES] = { NULL };
 
+#if ARDUHAL_LOG_LEVEL >= ARDUHAL_LOG_LEVEL_ERROR
+const char * fferr2str[] = {
+    "(0) Succeeded",
+    "(1) A hard error occurred in the low level disk I/O layer",
+    "(2) Assertion failed",
+    "(3) The physical drive cannot work",
+    "(4) Could not find the file",
+    "(5) Could not find the path",
+    "(6) The path name format is invalid",
+    "(7) Access denied due to prohibited access or directory full",
+    "(8) Access denied due to prohibited access",
+    "(9) The file/directory object is invalid",
+    "(10) The physical drive is write protected",
+    "(11) The logical drive number is invalid",
+    "(12) The volume has no work area",
+    "(13) There is no valid FAT volume",
+    "(14) The f_mkfs() aborted due to any problem",
+    "(15) Could not get a grant to access the volume within defined period",
+    "(16) The operation is rejected according to the file sharing policy",
+    "(17) LFN working buffer could not be allocated",
+    "(18) Number of open files > FF_FS_LOCK",
+    "(19) Given parameter is invalid"
+};
+#endif
+
 /*
  * SD SPI
  * */
@@ -73,6 +98,9 @@ bool sdWait(uint8_t pdrv, int timeout)
         resp = s_cards[pdrv]->spi->transfer(0xFF);
     } while (resp == 0x00 && (millis() - start) < (unsigned int)timeout);
 
+    if (!resp) {
+        log_w("Wait Failed");
+    }
     return (resp > 0x00);
 }
 
@@ -91,7 +119,10 @@ bool sdSelectCard(uint8_t pdrv)
 {
     ardu_sdcard_t * card = s_cards[pdrv];
     digitalWrite(card->ssPin, LOW);
-    sdWait(pdrv, 300);
+    bool s = sdWait(pdrv, 300);
+    if (!s) {
+        log_e("Select Failed");
+    }
     return true;
 }
 
@@ -105,10 +136,11 @@ char sdCommand(uint8_t pdrv, char cmd, unsigned int arg, unsigned int* resp)
             token = sdCommand(pdrv, APP_CMD, 0, NULL);
             sdDeselectCard(pdrv);
             if (token > 1) {
-                return token;
+                break;
             }
             if(!sdSelectCard(pdrv)) {
-                return 0xFF;
+                token = 0xFF;
+                break;
             }
         }
 
@@ -159,7 +191,10 @@ char sdCommand(uint8_t pdrv, char cmd, unsigned int arg, unsigned int* resp)
 
         break;
     }
-
+    if (token == 0xFF) {
+        log_e("Card Failed! cmd: 0x%02x", cmd);
+        card->status = STA_NOINIT;
+    }
     return token;
 }
 
@@ -215,7 +250,7 @@ bool sdReadSector(uint8_t pdrv, char* buffer, unsigned long long sector)
 {
     for (int f = 0; f < 3; f++) {
         if(!sdSelectCard(pdrv)) {
-            break;
+            return false;
         }
         if (!sdCommand(pdrv, READ_BLOCK_SINGLE, (s_cards[pdrv]->type == CARD_SDHC) ? sector : sector << 9, NULL)) {
             bool success = sdReadBytes(pdrv, buffer, 512);
@@ -235,7 +270,7 @@ bool sdReadSectors(uint8_t pdrv, char* buffer, unsigned long long sector, int co
 {
     for (int f = 0; f < 3;) {
         if(!sdSelectCard(pdrv)) {
-            break;
+            return false;
         }
 
         if (!sdCommand(pdrv, READ_BLOCK_MULTIPLE, (s_cards[pdrv]->type == CARD_SDHC) ? sector : sector << 9, NULL)) {
@@ -271,7 +306,7 @@ bool sdWriteSector(uint8_t pdrv, const char* buffer, unsigned long long sector)
 {
     for (int f = 0; f < 3; f++) {
         if(!sdSelectCard(pdrv)) {
-            break;
+            return false;
         }
         if (!sdCommand(pdrv, WRITE_BLOCK_SINGLE, (s_cards[pdrv]->type == CARD_SDHC) ? sector : sector << 9, NULL)) {
             char token = sdWriteBytes(pdrv, buffer, 0xFE);
@@ -307,12 +342,12 @@ bool sdWriteSectors(uint8_t pdrv, const char* buffer, unsigned long long sector,
     for (int f = 0; f < 3;) {
         if (card->type != CARD_MMC) {
             if (sdTransaction(pdrv, SET_WR_BLK_ERASE_COUNT, currentCount, NULL)) {
-                break;
+                return false;
             }
         }
 
         if(!sdSelectCard(pdrv)) {
-            break;
+            return false;
         }
 
         if (!sdCommand(pdrv, WRITE_BLOCK_MULTIPLE, (card->type == CARD_SDHC) ? currentSector : currentSector << 9, NULL)) {
@@ -344,9 +379,8 @@ bool sdWriteSectors(uint8_t pdrv, const char* buffer, unsigned long long sector,
                     break;
                 }
 
-                sdDeselectCard(pdrv);
-
                 if (token == 0x0A) {
+                    sdDeselectCard(pdrv);
                     unsigned int writtenBlocks = 0;
                     if (card->type != CARD_MMC && sdSelectCard(pdrv)) {
                         if (!sdCommand(pdrv, SEND_NUM_WR_BLOCKS, 0, NULL)) {
@@ -365,7 +399,7 @@ bool sdWriteSectors(uint8_t pdrv, const char* buffer, unsigned long long sector,
                     currentCount = count - writtenBlocks;
                     continue;
                 } else {
-                    return false;
+                    break;
                 }
             }
         } else {
@@ -380,7 +414,7 @@ unsigned long sdGetSectorsCount(uint8_t pdrv)
 {
     for (int f = 0; f < 3; f++) {
         if(!sdSelectCard(pdrv)) {
-            break;
+            return false;
         }
 
         if (!sdCommand(pdrv, SEND_CSD, 0, NULL)) {
@@ -714,7 +748,7 @@ uint8_t sdcard_unmount(uint8_t pdrv)
     return 0;
 }
 
-bool sdcard_mount(uint8_t pdrv, const char* path, uint8_t max_files)
+bool sdcard_mount(uint8_t pdrv, const char* path, uint8_t max_files, bool format_if_empty)
 {
     ardu_sdcard_t * card = s_cards[pdrv];
     if(pdrv >= FF_VOLUMES || card == NULL){
@@ -739,9 +773,25 @@ bool sdcard_mount(uint8_t pdrv, const char* path, uint8_t max_files)
 
     FRESULT res = f_mount(fs, drv, 1);
     if (res != FR_OK) {
-        log_e("f_mount failed 0x(%x)", res);
-        esp_vfs_fat_unregister_path(path);
-        return false;
+        log_e("f_mount failed: %s", fferr2str[res]);
+        if(res == 13 && format_if_empty){
+            BYTE work[FF_MAX_SS];
+            res = f_mkfs(drv, FM_ANY, 0, work, sizeof(work));
+            if (res != FR_OK) {
+                log_e("f_mkfs failed: %s", fferr2str[res]);
+                esp_vfs_fat_unregister_path(path);
+                return false;
+            }
+            res = f_mount(fs, drv, 1);
+            if (res != FR_OK) {
+                log_e("f_mount failed: %s", fferr2str[res]);
+                esp_vfs_fat_unregister_path(path);
+                return false;
+            }
+        } else {
+            esp_vfs_fat_unregister_path(path);
+            return false;
+        }
     }
     AcquireSPI lock(card);
     card->sectors = sdGetSectorsCount(pdrv);

libraries/SD/src/sd_diskio.h

@@ -22,7 +22,7 @@
 uint8_t sdcard_init(uint8_t cs, SPIClass * spi, int hz);
 uint8_t sdcard_uninit(uint8_t pdrv);
 
-bool sdcard_mount(uint8_t pdrv, const char* path, uint8_t max_files);
+bool sdcard_mount(uint8_t pdrv, const char* path, uint8_t max_files, bool format_if_empty);
 uint8_t sdcard_unmount(uint8_t pdrv);
 
 sdcard_type_t sdcard_type(uint8_t pdrv);

libraries/SD_MMC/src/SD_MMC.cpp

@@ -35,7 +35,7 @@ SDMMCFS::SDMMCFS(FSImplPtr impl)
     : FS(impl), _card(NULL)
 {}
 
-bool SDMMCFS::begin(const char * mountpoint, bool mode1bit)
+bool SDMMCFS::begin(const char * mountpoint, bool mode1bit, bool format_if_mount_failed)
 {
     if(_card) {
         return true;
@@ -68,7 +68,7 @@ bool SDMMCFS::begin(const char * mountpoint, bool mode1bit)
     }
 
     esp_vfs_fat_sdmmc_mount_config_t mount_config = {
-        .format_if_mount_failed = false,
+        .format_if_mount_failed = format_if_mount_failed,
         .max_files = 5,
         .allocation_unit_size = 0
     };

libraries/SD_MMC/src/SD_MMC.h

@@ -29,7 +29,7 @@ class SDMMCFS : public FS
 
 public:
     SDMMCFS(FSImplPtr impl);
-    bool begin(const char * mountpoint="/sdcard", bool mode1bit=false);
+    bool begin(const char * mountpoint="/sdcard", bool mode1bit=false, bool format_if_mount_failed=false);
     void end();
     sdcard_type_t cardType();
     uint64_t cardSize();