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

Update to SdFat 2.0.0 #7779

Merged
merged 12 commits into from
Dec 23, 2020
Merged

Update to SdFat 2.0.0 #7779

merged 12 commits into from
Dec 23, 2020

Conversation

earlephilhower
Copy link
Collaborator

Fixes #7772

@forkineye can you give this a try and report back? I'm getting ~1.5MB/s w/my setup (limited SPI speed due to wiring/card) which is somewhat above what I saw before.

@earlephilhower earlephilhower changed the title WIP - Update to SdFat 2.0.0 Update to SdFat 2.0.0 Dec 19, 2020
@forkineye
Copy link
Contributor

@earlephilhower I'll pull this down today and test it, thank you

Avoid branch insanity by merging the fix20 branch into
ESP8266SdFat/master and referencing that in the core.
@forkineye
Copy link
Contributor

forkineye commented Dec 19, 2020

The bench example failed to compile with SD_FAT_TYPE set to the default of 0, claiming that "file" of type "File". Setting to SD_FAT_TYPE to 3 was successful and showed substantial increases to compared to the current master branch. I am however experiencing issues when attempt to write or delete files from my application. I need to troubleshoot further to make sure it's not my code, but I've included the top of the stack dump below. It does work however in 2.7.4 and the current master, it's just slow.

Bench tests

master branch - 130KB/s write; 650KB/s read
sdfat20 branch - 1700KB/s write; 3170KB/s read

bench.ino error

#if SD_FAT_TYPE == 0
SdFat sd;
File file;
#elif SD_FAT_TYPE == 1
SdFat32 sd;
File32 file;
#elif SD_FAT_TYPE == 2
SdExFat sd;
ExFile file;
#elif SD_FAT_TYPE == 3
SdFs sd;
FsFile file;
#else  // SD_FAT_TYPE
#error Invalid SD_FAT_TYPE
#endif  // SD_FAT_TYPE
Bench:177:13: error: 'class fs::File' has no member named 'open'
  177 |   if (!file.open("bench.dat", O_RDWR | O_CREAT | O_TRUNC)) {
      |             ^~~~
Bench:202:17: error: 'class fs::File' has no member named 'preAllocate'
  202 |       if (!file.preAllocate(FILE_SIZE)) {
      |                 ^~~~~~~~~~~
Bench:220:28: error: 'class fs::File' has no member named 'curPosition'; did you mean 'position'?
  220 |         skipLatency = file.curPosition() < 512;
      |                            ^~~~~~~~~~~
      |                            position
Bench:230:10: error: 'class fs::File' has no member named 'sync'
  230 |     file.sync();
      |          ^~~~
Bench:232:14: error: 'class fs::File' has no member named 'fileSize'
  232 |     s = file.fileSize();
      |              ^~~~~~~~
Bench:243:10: error: 'class fs::File' has no member named 'rewind'
  243 |     file.rewind();
      |          ^~~~~~
Bench:273:14: error: 'class fs::File' has no member named 'fileSize'
  273 |     s = file.fileSize();
      |              ^~~~~~~~

Stack dump from my code

Deleting File: '/bench.dat'

User exception (panic/abort/assert)
Panic core_esp8266_main.cpp:136 __yield

0x4022e8ee: raise_exception at core_esp8266_postmortem.cpp line ?
0x401005d6: millis at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\cores\esp8266/core_esp8266_wiring.cpp line 188
0x4022e95b: __panic_func at ?? line ?
0x4022e151: yield at ?? line ?
0x40227e40: sdfat::SysCall::curTimeMS() at c:\users\sporadic\documents\arduino\hardware\esp8266com\esp8266\libraries\esp8266sdfat\src\common/syscall.h line 82
:  (inlined by) sdfat::SdSpiCard::isTimedOut(unsigned short, unsigned short) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266SdFat\src\SdCard/SdSpiCard.cpp line 461
0x40227f81: sdfat::SdSpiCard::waitNotBusy(unsigned short) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266SdFat\src\SdCard/SdSpiCard.cpp line 693
0x40227fcc: sdfat::SdSpiCard::spiSend(unsigned char) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266SdFat\src\SdCard/SdSpiCard.h line 327
:  (inlined by) sdfat::SdSpiCard::cardCommand(unsigned char, unsigned int) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266SdFat\src\SdCard/SdSpiCard.cpp line 369
0x4022826d: sdfat::SdSpiCard::readStart(unsigned int) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266SdFat\src\SdCard/SdSpiCard.cpp line 581
0x4022852b: sdfat::SdSpiCard::readSectors(unsigned int, unsigned char*, unsigned int) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266SdFat\src\SdCard/SdSpiCard.cpp line 611
0x402279cc: sdfat::FatCache::read(unsigned int, unsigned char) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266SdFat\src\FatLib/FatPartition.cpp line 43
0x40227a4a: sdfat::FatPartition::fatGet(unsigned int, unsigned int*) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266SdFat\src\FatLib/FatPartition.cpp line 252
:  (inlined by) sdfat::FatPartition::fatGet(unsigned int, unsigned int*) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266SdFat\src\FatLib/FatPartition.cpp line 228
0x40227bc7: sdfat::FatPartition::freeChain(unsigned int) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266SdFat\src\FatLib/FatPartition.cpp line 376
0x402262f8: sdfat::FatFile::open(sdfat::FatFile*, char const*, unsigned char) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266SdFat\src\FatLib/FatFile.cpp line 473
0x40226cb1: sdfat::FatFile::remove() at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266SdFat\src\FatLib/FatFileLFN.cpp line 583 (discriminator 1)
0x4022ce4d: String::changeBuffer(unsigned int) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\cores\esp8266/WString.cpp line 167
0x4022f62e: uart_write at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\cores\esp8266/uart.cpp line 509
0x40224834: sdfs::SDFSImpl::remove(char const*) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\libraries\SDFS\src/SDFS.h line 130 (discriminator 4)
0x40231e3c: fs::FS::remove(char const*) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\cores\esp8266/FS.cpp line 404
0x4022ba06: fs::FS::remove(String const&) at C:\Users\sporadic\Documents\Arduino\hardware\esp8266com\esp8266\cores\esp8266/FS.cpp line 408
0x402169e8: c_FPPDiscovery::DeleteFseqFile(String&) at c:\tmp\build\sketch\src\service/FPPDiscovery.cpp line 1177

@earlephilhower
Copy link
Collaborator Author

Thanks for the update!

The bench example failed to compile with SD_FAT_TYPE set to the default of 0, claiming that "file" of type "File". Setting to SD_FAT_TYPE to 3 was successful and showed substantial increases to compared to the current master branch.

This is expected because only the private definition of File used in the SDFat example that works/is needed for the normal Arduino File is that one. There are some ugly name overlap issues still in the other methods they use. So Type 0, 1, and 3 aren't used in the 8266 core.

I am however experiencing issues when attempt to write or delete files from my application. I need to troubleshoot further to make sure it's not my code, but I've included the top of the stack dump below. It does work however in 2.7.4 and the current master, it's just slow.

If you can get an MCVE to reproduce it that would be great! Looking at the trace it looks like a yield() was called from inside an ISR callback. If that's the case, it's illegal on the ESP8266. My guess is that the new SPI object the author wrote has a yield() in it or is trying to delay() inside an ISR...either or which is a no-no.

What's weird is that I'm not seeing it on my (limited) testing w/the FSBrowser and LittleFS_SpeedTest. Both of which do seem to be much snappier than before for me, as well.

Stack dump from my code

Deleting File: '/bench.dat'

User exception (panic/abort/assert)
Panic core_esp8266_main.cpp:136 __yield

extern "C" void __yield() {
if (can_yield()) {
esp_schedule();
esp_yield_within_cont();
}
else {
panic();
}
}

@forkineye
Copy link
Contributor

As a test, can you try applying this 1-line change to the SDFat library replacing yield() with delay(0) which, IIRC, is IRQ safe?

earle@server:~/Arduino/hardware/esp8266com/esp8266/libraries/ESP8266SdFat/src/common$ git diff
diff --git a/src/common/SysCall.h b/src/common/SysCall.h
index 34b9ca6..c565e58 100644
--- a/src/common/SysCall.h
+++ b/src/common/SysCall.h
@@ -92,7 +92,7 @@ inline void SysCall::yield() {
 #elif defined(ARDUINO)
 inline void SysCall::yield() {
   // Use the external Arduino yield() function.
-  ::yield();
+  ::delay(0);
 }
 #else  // defined(PLATFORM_ID)
 inline void SysCall::yield() {}

That seems to have fixed it, thanks!

The ESP8266SdFat examples now build, even if they're not recommended and
are incompatible with the Arduino EWSP8266 File implementation.
@d-a-v d-a-v added the alpha included in alpha release label Dec 22, 2020
@earlephilhower earlephilhower merged commit c487ca5 into esp8266:master Dec 23, 2020
@earlephilhower earlephilhower deleted the sdfat20 branch December 23, 2020 19:39
davisonja added a commit to davisonja/Arduino that referenced this pull request Dec 28, 2020
…lash

* upstream/master: (72 commits)
  Typo error in ESP8266WiFiGeneric.h (esp8266#7797)
  lwip2: use pvPortXalloc/vPortFree and "-free -fipa-pta" (esp8266#7793)
  Use smarter cache key, cache Arduino IDE (esp8266#7791)
  Update to SdFat 2.0.2, speed SD access (esp8266#7779)
  BREAKING - Upgrade to upstream newlib 4.0.0 release (esp8266#7708)
  mock: +hexdump() from debug.cpp (esp8266#7789)
  more lwIP physical interfaces (esp8266#6680)
  Rationalize File timestamp callback (esp8266#7785)
  Update to LittleFS v2.3 (esp8266#7787)
  WiFiServerSecure: Cache SSL sessions (esp8266#7774)
  platform.txt: instruct GCC to perform more aggressive optimization (esp8266#7770)
  LEAmDNS fixes (esp8266#7786)
  Move uzlib to master branch (esp8266#7782)
  Update to latest uzlib upstream (esp8266#7776)
  EspSoftwareSerial bug fix release 6.10.1: preciseDelay() could delay() for extremely long time, if period duration was exceeded on entry. (esp8266#7771)
  Fixed OOM double count in umm_realloc. (esp8266#7768)
  Added missing check for failure on umm_push_heap calls in Esp.cpp (esp8266#7767)
  Fix: cannot build after esp8266#7060 on Win64 (esp8266#7754)
  Add the missing 'rename' method wrapper in SD library. (esp8266#7766)
  i2s: adds i2s_rxtxdrive_begin(enableRx, enableTx, driveRxClocks, driveTxClocks) (esp8266#7748)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alpha included in alpha release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ESP8266SdFat Performance vs Upstream SdFat
3 participants