From c05d81e1b6da61d5a0a9e5593948201bf09e0b00 Mon Sep 17 00:00:00 2001 From: me-no-dev Date: Mon, 1 Mar 2021 18:53:54 +0200 Subject: [PATCH] Fix several SD card issues - 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 --- libraries/FS/src/FS.cpp | 32 ++++++------- libraries/SD/src/SD.cpp | 4 +- libraries/SD/src/SD.h | 2 +- libraries/SD/src/sd_diskio.cpp | 84 ++++++++++++++++++++++++++------- libraries/SD/src/sd_diskio.h | 2 +- libraries/SD_MMC/src/SD_MMC.cpp | 4 +- libraries/SD_MMC/src/SD_MMC.h | 2 +- 7 files changed, 90 insertions(+), 40 deletions(-) diff --git a/libraries/FS/src/FS.cpp b/libraries/FS/src/FS.cpp index ab4f5d5289a..3bfcd54fcc0 100644 --- a/libraries/FS/src/FS.cpp +++ b/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,7 +155,7 @@ const char* File::name() const //to implement boolean File::isDirectory(void) { - if (!_p) { + if (!*this) { return false; } return _p->isDirectory(); @@ -163,7 +163,7 @@ boolean File::isDirectory(void) File File::openNextFile(const char* mode) { - if (!_p) { + if (!*this) { return File(); } return _p->openNextFile(mode); @@ -171,7 +171,7 @@ File File::openNextFile(const char* mode) void File::rewindDirectory(void) { - if (!_p) { + if (!*this) { return; } _p->rewindDirectory(); diff --git a/libraries/SD/src/SD.cpp b/libraries/SD/src/SD.cpp index bbe6dde2577..db46671e2fd 100644 --- a/libraries/SD/src/SD.cpp +++ b/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; diff --git a/libraries/SD/src/SD.h b/libraries/SD/src/SD.h index b54e73bc04a..cfd95624410 100644 --- a/libraries/SD/src/SD.h +++ b/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(); diff --git a/libraries/SD/src/sd_diskio.cpp b/libraries/SD/src/sd_diskio.cpp index ea94aa37b51..81244ee7881 100644 --- a/libraries/SD/src/sd_diskio.cpp +++ b/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); diff --git a/libraries/SD/src/sd_diskio.h b/libraries/SD/src/sd_diskio.h index 9d386663aea..77866ab4fee 100644 --- a/libraries/SD/src/sd_diskio.h +++ b/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); diff --git a/libraries/SD_MMC/src/SD_MMC.cpp b/libraries/SD_MMC/src/SD_MMC.cpp index 5b96c16a8b8..a4dca92d437 100644 --- a/libraries/SD_MMC/src/SD_MMC.cpp +++ b/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 }; diff --git a/libraries/SD_MMC/src/SD_MMC.h b/libraries/SD_MMC/src/SD_MMC.h index 04d309a89a3..8f0f62fd0f5 100644 --- a/libraries/SD_MMC/src/SD_MMC.h +++ b/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();