greiman / SdFat

Arduino FAT16/FAT32 exFAT Library
MIT License
1.08k stars 505 forks source link

Example build error on Raspberry Pico (earlephilhover core for Arduino IDE) #397

Closed XFer012 closed 2 years ago

XFer012 commented 2 years ago

Hello, I am using the latest SDFat 2.2.0 with Arduino IDE 1.8.19.

Board is Raspberry Pico (RP2040); I am using the "Arduino-Pico" core board package from earlephilhover (https://github.com/earlephilhower/arduino-pico), latest version 2.4.1

When trying to build SDFat examples (here I'm trying "bench" but I get the same error with every example I try), I get this build error:


F:\Arduino\libraries\SdFat\src/SdFat.h:452:2: warning: #warning File not defined because __has_include(FS.h) [-Wcpp]
  452 | #warning File not defined because __has_include(FS.h)
      |  ^~~~~~~
bench:69:1: error: 'File' does not name a type; did you mean 'SdFile'?
   69 | File file;
      | ^~~~
      | SdFile
F:\Arduino\libraries\SdFat\examples\bench\bench.ino: In function 'void loop()':
bench:170:8: error: 'file' was not declared in this scope
  170 |   if (!file.open("bench.dat", O_RDWR | O_CREAT | O_TRUNC)) {
      |        ^~~~
bench:193:5: error: 'file' was not declared in this scope
  193 |     file.truncate(0);
      |     ^~~~
bench:236:5: error: 'file' was not declared in this scope
  236 |     file.rewind();
      |     ^~~~
bench:272:3: error: 'file' was not declared in this scope
  272 |   file.close();
      |   ^~~~
greiman commented 2 years ago

The type "File" can not be used since it is defined in the board package.

If the board package did not define "File", this typedef would have been used. typedef FsFile File;

You can select SD_FAT_TYPE with a value of three to use SdFs and FsFile.

Make this change at the start of bench:

// SD_FAT_TYPE = 0 for SdFat/File as defined in SdFatConfig.h,
// 1 for FAT16/FAT32, 2 for exFAT, 3 for FAT16/FAT32 and exFAT.
#define SD_FAT_TYPE 3  //<<----------------  Was zero.
XFer012 commented 2 years ago

Thank you Bill, I tried defining SD_FAT_TYPE 3 but now I have a different build error:

In file included from L:\Progetti\Arduino\PICO_SDbench\PICO_SDbench.ino:4:
L:\Progetti\Arduino\libraries\SdFat\src/SdFat.h:452:2: warning: #warning File not defined because __has_include(FS.h) [-Wcpp]
  452 | #warning File not defined because __has_include(FS.h)
      |  ^~~~~~~
In file included from L:\Progetti\Arduino\libraries\SdFat\src\ExFatLib\upcase.cpp:25:
L:\Progetti\Arduino\libraries\SdFat\src\ExFatLib\upcase.h:31:25: error: 'ExChar16_t' does not name a type; did you mean 'char16_t'?
   31 |                   const ExChar16_t* name, size_t offset, size_t n);
      |                         ^~~~~~~~~~
      |                         char16_t
L:\Progetti\Arduino\libraries\SdFat\src\ExFatLib\upcase.h:33:30: error: 'ExChar16_t' does not name a type; did you mean 'char16_t'?
   33 | uint16_t exFatHashName(const ExChar16_t* name, size_t n, uint16_t hash);
      |                              ^~~~~~~~~~
      |                              char16_t
L:\Progetti\Arduino\libraries\SdFat\src\ExFatLib\upcase.cpp:188:30: error: 'ExChar16_t' does not name a type; did you mean 'char16_t'?
  188 | uint16_t exFatHashName(const ExChar16_t* name, size_t n, uint16_t hash) {
      |                              ^~~~~~~~~~
      |                              char16_t
L:\Progetti\Arduino\libraries\SdFat\src\ExFatLib\upcase.cpp:235:25: error: 'ExChar16_t' does not name a type; did you mean 'char16_t'?
  235 |                   const ExChar16_t* name, size_t offset, size_t n) {
      |                         ^~~~~~~~~~
      |                         char16_t
greiman commented 2 years ago

What version of SdFat are you using? The current version of SdFat does not have the type ExChar16_t.

See upcase.h here and upcase.cpp here.

XFer012 commented 2 years ago

Last version 2.2.0, and the "bench" example I'm trying to compile is the one bundled with it.

XFer012 commented 2 years ago

What version of SdFat are you using? The current version of SdFat does not have the type ExChar16_t.

See upcase.h here and upcase.cpp here.

This does not match what I have locally as 2.2.0 (downloaded via Library Manager)

This is my upcase.cpp:

/**
 * Copyright (c) 2011-2019 Bill Greiman
 * This file is part of the SdFat library for SD memory cards.
 *
 * MIT License
 *
 * Permission is hereby granted, free of charge, to any person obtaining a
 * copy of this software and associated documentation files (the "Software"),
 * to deal in the Software without restriction, including without limitation
 * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 * and/or sell copies of the Software, and to permit persons to whom the
 * Software is furnished to do so, subject to the following conditions:
 *
 * The above copyright notice and this permission notice shall be included
 * in all copies or substantial portions of the Software.
 *
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
 * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
 * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
 * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 * DEALINGS IN THE SOFTWARE.
 */
#include "upcase.h"

#ifdef __AVR__
#include <avr/pgmspace.h>
#define TABLE_MEM PROGMEM
#define readTable8(sym)  pgm_read_byte(&sym)
#define readTable16(sym) pgm_read_word(&sym)
#else  // __AVR__
#define TABLE_MEM
#define readTable8(sym)  (sym)
#define readTable16(sym) (sym)
#endif  // __AVR__

struct map16 {
  uint16_t base;
  int8_t off;
  uint8_t count;
};
typedef struct map16 map16_t;

struct pair16 {
  uint16_t key;
  uint16_t val;
};
typedef struct pair16 pair16_t;
//-----------------------------------------------------------------------------
static const map16_t mapTable[] TABLE_MEM = {
  {0X0061, -32,  26},
  {0X00E0, -32,  23},
  {0X00F8, -32,  7 },
  {0X0100,   1,  48},
  {0X0132,   1,   6},
  {0X0139,   1,  16},
  {0X014A,   1,  46},
  {0X0179,   1,   6},
  {0X0182,   1,   4},
  {0X01A0,   1,   6},
  {0X01B3,   1,   4},
  {0X01CD,   1,  16},
  {0X01DE,   1,  18},
  {0X01F8,   1,  40},
  {0X0222,   1,  18},
  {0X0246,   1,  10},
  {0X03AD, -37,   3},
  {0X03B1, -32,  17},
  {0X03C3, -32,   9},
  {0X03D8,   1,  24},
  {0X0430, -32,  32},
  {0X0450, -80,  16},
  {0X0460,   1,  34},
  {0X048A,   1,  54},
  {0X04C1,   1,  14},
  {0X04D0,   1,  68},
  {0X0561, -48,  38},
  {0X1E00,   1, 150},
  {0X1EA0,   1,  90},
  {0X1F00,   8,   8},
  {0X1F10,   8,   6},
  {0X1F20,   8,   8},
  {0X1F30,   8,   8},
  {0X1F40,   8,   6},
  {0X1F60,   8,   8},
  {0X1F70,  74,   2},
  {0X1F72,  86,   4},
  {0X1F76, 100,   2},
  {0X1F7A, 112,   2},
  {0X1F7C, 126,   2},
  {0X1F80,   8,   8},
  {0X1F90,   8,   8},
  {0X1FA0,   8,   8},
  {0X1FB0,   8,   2},
  {0X1FD0,   8,   2},
  {0X1FE0,   8,   2},
  {0X2170, -16,  16},
  {0X24D0, -26,  26},
  {0X2C30, -48,  47},
  {0X2C67,   1,   6},
  {0X2C80,   1, 100},
  {0X2D00,   0,  38},
  {0XFF41, -32,  26},
};
const size_t MAP_DIM = sizeof(mapTable)/sizeof(map16_t);
//-----------------------------------------------------------------------------
static const pair16_t lookupTable[] TABLE_MEM = {
  {0X00FF, 0X0178},
  {0X0180, 0X0243},
  {0X0188, 0X0187},
  {0X018C, 0X018B},
  {0X0192, 0X0191},
  {0X0195, 0X01F6},
  {0X0199, 0X0198},
  {0X019A, 0X023D},
  {0X019E, 0X0220},
  {0X01A8, 0X01A7},
  {0X01AD, 0X01AC},
  {0X01B0, 0X01AF},
  {0X01B9, 0X01B8},
  {0X01BD, 0X01BC},
  {0X01BF, 0X01F7},
  {0X01C6, 0X01C4},
  {0X01C9, 0X01C7},
  {0X01CC, 0X01CA},
  {0X01DD, 0X018E},
  {0X01F3, 0X01F1},
  {0X01F5, 0X01F4},
  {0X023A, 0X2C65},
  {0X023C, 0X023B},
  {0X023E, 0X2C66},
  {0X0242, 0X0241},
  {0X0253, 0X0181},
  {0X0254, 0X0186},
  {0X0256, 0X0189},
  {0X0257, 0X018A},
  {0X0259, 0X018F},
  {0X025B, 0X0190},
  {0X0260, 0X0193},
  {0X0263, 0X0194},
  {0X0268, 0X0197},
  {0X0269, 0X0196},
  {0X026B, 0X2C62},
  {0X026F, 0X019C},
  {0X0272, 0X019D},
  {0X0275, 0X019F},
  {0X027D, 0X2C64},
  {0X0280, 0X01A6},
  {0X0283, 0X01A9},
  {0X0288, 0X01AE},
  {0X0289, 0X0244},
  {0X028A, 0X01B1},
  {0X028B, 0X01B2},
  {0X028C, 0X0245},
  {0X0292, 0X01B7},
  {0X037B, 0X03FD},
  {0X037C, 0X03FE},
  {0X037D, 0X03FF},
  {0X03AC, 0X0386},
  {0X03C2, 0X03A3},
  {0X03CC, 0X038C},
  {0X03CD, 0X038E},
  {0X03CE, 0X038F},
  {0X03F2, 0X03F9},
  {0X03F8, 0X03F7},
  {0X03FB, 0X03FA},
  {0X04CF, 0X04C0},
  {0X1D7D, 0X2C63},
  {0X1F51, 0X1F59},
  {0X1F53, 0X1F5B},
  {0X1F55, 0X1F5D},
  {0X1F57, 0X1F5F},
  {0X1F78, 0X1FF8},
  {0X1F79, 0X1FF9},
  {0X1FB3, 0X1FBC},
  {0X1FCC, 0X1FC3},
  {0X1FE5, 0X1FEC},
  {0X1FFC, 0X1FF3},
  {0X214E, 0X2132},
  {0X2184, 0X2183},
  {0X2C61, 0X2C60},
  {0X2C76, 0X2C75},
};
const size_t LOOKUP_DIM = sizeof(lookupTable)/sizeof(pair16_t);

//-----------------------------------------------------------------------------
uint16_t exFatHashName(const ExChar16_t* name, size_t n, uint16_t hash) {
  for (size_t i = 0; i < n; i++) {
    uint16_t c = toUpcase(name[i]);
    hash = ((hash << 15) | (hash >> 1)) + (c & 0XFF);
    hash = ((hash << 15) | (hash >> 1)) + (c >> 8);
  }
  return hash;
}
//-----------------------------------------------------------------------------
static size_t searchPair16(const pair16_t* table, size_t size, uint16_t key) {
  size_t left = 0;
  size_t right = size;
  size_t mid;
  while (right - left > 1) {
    mid = left + (right - left)/2;
    if (readTable16(table[mid].key) <= key) {
      left = mid;
    } else {
      right = mid;
    }
  }
  return left;
}
//-----------------------------------------------------------------------------
uint16_t toUpcase(uint16_t chr) {
  uint16_t i, first;
  // Optimize for simple ASCII.
  if (chr < 127) {
    return chr - ('a' <= chr && chr <= 'z' ? 'a' - 'A' : 0);
  }
  i = searchPair16(reinterpret_cast<const pair16_t*>(mapTable), MAP_DIM, chr);
  first = readTable16(mapTable[i].base);
  if (first <= chr && (chr - first)  < readTable8(mapTable[i].count)) {
    int8_t off = readTable8(mapTable[i].off);
    if (off == 1) {
      return chr - ((chr - first) & 1);
    }
    return chr + (off ? off : -0x1C60);
  }
  i = searchPair16(lookupTable, LOOKUP_DIM, chr);
  if (readTable16(lookupTable[i].key) == chr) {
    return readTable16(lookupTable[i].val);
  }
  return chr;
}
//-----------------------------------------------------------------------------
bool exFatCmpName(const DirName_t* unicode,
                  const ExChar16_t* name, size_t offset, size_t n) {
  uint16_t u;
  for (size_t i = 0; i < n; i++) {
    u = getLe16(unicode->unicode + 2*i);
    if (toUpcase(name[i + offset]) != toUpcase(u)) {
      return false;
    }
  }
  return true;
}
//-----------------------------------------------------------------------------
static char toUpper(char c) {
  return c - ('a' <= c && c <= 'z' ? 'a' - 'A' : 0);
}
//-----------------------------------------------------------------------------
uint16_t exFatHashName(const char* name, size_t n, uint16_t hash) {
  for (size_t i = 0; i < n; i++) {
    uint8_t c = name[i];
    if ('a' <= c && c <= 'z') {
      c -= 'a' - 'A';
    }
    hash = ((hash << 15) | (hash >> 1)) + c;
    hash = ((hash << 15) | (hash >> 1));
  }
  return hash;
}
//-----------------------------------------------------------------------------
bool exFatCmpName(const DirName_t* unicode,
                  const char* name, size_t offset, size_t n) {
  uint16_t u;
  for (size_t i = 0; i < n; i++) {
    u = getLe16(unicode->unicode + 2*i);
    if (u >= 0x7F  || toUpper(name[i + offset]) != toUpper(u)) {
      return false;
    }
  }
  return true;
}
//-----------------------------------------------------------------------------
uint32_t upcaseChecksum(uint16_t uc, uint32_t sum) {
  sum = (sum << 31) + (sum >> 1) + (uc & 0XFF);
  sum = (sum << 31) + (sum >> 1) + (uc >> 8);
  return sum;
}
XFer012 commented 2 years ago

And this is my "library.properties":

name=SdFat
version=2.2.0
license=MIT
author=Bill Greiman <fat16lib@sbcglobal.net>
maintainer=Bill Greiman <fat16lib@sbcglobal.net>
sentence=Provides access to SD memory cards.
paragraph=The SdFat library supports FAT16, FAT32, and exFAT file systems on Standard SD, SDHC, and SDXC cards.
category=Data Storage
url=https://github.com/greiman/SdFat
repository=https://github.com/greiman/SdFat.git
architectures=*

And it is the version actually used by Arduino:

Multiple libraries were found for "SdFat.h"
 Used: L:\Progetti\Arduino\libraries\SdFat
 Not used: C:\Users\Fernando\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.1\libraries\ESP8266SdFat

Is it possible that the v2.2.0 installed by Arduino Library Manager does not contain the right version of upcase.cpp and upcase.h?

greiman commented 2 years ago

Earle Philhower often uses a wrapper for SdFat to implement the SD.h library. I see that he has an include for SdFat here.

So it could be the version he is using.

I tried the Library Manager on my test machine and it installed the correct files. I have no idea what is happening on your machine.

XFer012 commented 2 years ago

Is it OK to overwrite my libraries/SdFat with the zipped one from Github? Any adverse effect I should be warned against?

greiman commented 2 years ago

If a different version is used by the board package there could still be problems. Philhower depends on internals of SdFat I never intended users to access.

I would move the current version from the library folder and try an unzipped version from github.

XFer012 commented 2 years ago

Thank you! Will try

XFer012 commented 2 years ago

Overwriting libraries/SdFat with the zip from Github did the trick.

Builds OK and runs OK.

Performances are quite disappointing, and I wonder why. A 16 GB UHS-I / A1 Toshiba Exceria card which gives 22 MB/s on Teensy 3.6 (I know, 4-bit SDIO, but still), reaches a maximum of 920 KB/s with the Raspberry Pico, despite overclocking to 250 MHz, building with -Ofast and defining USE_SPI_ARRAY_TRANSFER 1 in SdFatConfig.h (it was 480 KB/s with everything stock).

Anyway, many thanks for the tips!

greiman commented 2 years ago

The problem is SdFat used the default SPI byte transfers in a loop. The Pico SPI library has the standard transfer(void *buf, size_t count); but this is just a slow loop calling single byte transfer.

The Pico library also has a call that is not in standard Arduino libraries, transfer(void *txbuf, void *rxbuf, size_t count). It may be faster since it calls a lower level Pico C function.

The release version SdFat does not support this call but I have used it experimentally with the Adafruit M4 feather and I get a large increase in speed since the call uses DMA in the feather .

I plan to release a version of SdFat with the option to call this function if it exists in the SPI library for the board.

You could do a test of this call something like this:

  uint8_t buf[512];
  SPI.begin();
  // try various speeds if 50000000 is a problem
  SPI.beginTransaction(SPISettings(50000000, MSBFIRST, SPI_MODE0));
  uint32_t m = micros();
  // if rxbuf is null the C function spi_write_blocking(_spi, txbuff, count); is called
  SPI.transfer(buf, nullptr, 512);
  m = micros() - m;
  Serial.print("KB/s: ");
  Serial.println(512000.0/m);
XFer012 commented 2 years ago

Very interesting!

Under the same conditions (clock = 250 MHz, -Ofast), this way I get

KB/s: 2426.54

2.5x the previous speed!

I know this is SPI only without writing on the SD card of course, but very promising nonetheless!

XFer012 commented 2 years ago

(Why a fast SD speed is important for me: I'm working on a low-power cameracorder with OV2640. I need at least 1.6 MB/s sustained to capture 800x600 MJPEG at 18 FPS with reasonable compression)

greiman commented 2 years ago

I was hoping for more than 2426 KB/s, Teensy SPI at 50MHz does about 5000 KB/sec. Still getting more than a factor of two is worth adding an option to use transfer(txBuf, rxBuf, count).

I will post a patch here for you to try. I may take a day to do it.

greiman commented 2 years ago

Looks like my test driver might work with Pico. I attached a zip with SdSpiLibDriver.h. You need to replace the file located at:

libraries/SdFat/src/SpiDriver/SdSpiLibDriver.h

Then edit libraries/SdFat/src/SdFatConfig.h and change USE_SPI_ARRAY_TRANSFER to two.

/**
 * If USE_SPI_ARRAY_TRANSFER is non-zero and the standard SPI library is
 * use, the array transfer function, transfer(buf, size), will be used.
 * This option will allocate up to a 512 byte temporary buffer for send.
 * This may be faster for some boards.  Do not use this with AVR boards.
 */
#ifndef USE_SPI_ARRAY_TRANSFER
#define USE_SPI_ARRAY_TRANSFER 2  // <<----------------------- was zero
#endif  // USE_SPI_ARRAY_TRANSFER

I have implemented three new choices for USE_SPI_ARRAY_TRANSFER. First try two, this will use a nullptr for both array send and receive. If two doesn't work try three this uses a dummy array for rxBuf in send. Three is needed for the Feather M4. Finally four uses dummy buffers for both receive and send. Four is needed for STM32.

Here is the improvement I get on Feather M4:

With single byte transfer:

write speed and latency speed,max,min,avg KB/Sec,usec,usec,usec 1347.71,3454,378,379 1348.07,395,378,379

read speed and latency speed,max,min,avg KB/Sec,usec,usec,usec 1323.10,388,385,386 1322.75,388,385,386

With USE_SPI_ARRAY_TRANSFER set to three:

write speed and latency speed,max,min,avg KB/Sec,usec,usec,usec 2791.74,199,182,182 2791.74,198,182,182

read speed and latency speed,max,min,avg KB/Sec,usec,usec,usec 2812.15,183,181,181 2812.15,183,181,181

Click on this to download the patch.

SdSpiLibDriver.zip

XFer012 commented 2 years ago

Tried your patch (many thanks!!), but I have the following build error (no matter the value for USE_SPI_ARRAY_TRANSFER):

In file included from L:\Progetti\Arduino\PICO_SDbench_SDFat\PICO_SDbench_SDFat.ino:11:

L:\Progetti\Arduino\libraries\SdFat\src/SdFat.h:452:2: warning: #warning File not defined because __has_include(FS.h) [-Wcpp]

  452 | #warning File not defined because __has_include(FS.h)

      |  ^~~~~~~

In file included from l:\progetti\arduino\libraries\sdfat\src\spidriver\SdSpiArduinoDriver.h:92,

                 from l:\progetti\arduino\libraries\sdfat\src\spidriver\sdspidriver.h:146,

                 from L:\Progetti\Arduino\libraries\SdFat\src/SdCard/SdSpiCard.h:35,

                 from L:\Progetti\Arduino\libraries\SdFat\src/SdCard/SdCard.h:28,

                 from L:\Progetti\Arduino\libraries\SdFat\src/SdFat.h:32,

                 from L:\Progetti\Arduino\PICO_SDbench_SDFat\PICO_SDbench_SDFat.ino:11:

l:\progetti\arduino\libraries\sdfat\src\spidriver\SdSpiLibDriver.h: In member function 'uint8_t SdSpiArduinoDriver::receive(uint8_t*, size_t)':

l:\progetti\arduino\libraries\sdfat\src\spidriver\SdSpiLibDriver.h:70:38: error: no matching function for call to 'arduino::HardwareSPI::transfer(std::nullptr_t, uint8_t*&, size_t&)'

   70 |   m_spi->transfer(nullptr, buf, count);

      |                                      ^

In file included from C:\Users\Fernando\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.1\libraries\SPI\src/SPI.h:24,

                 from l:\progetti\arduino\libraries\sdfat\src\spidriver\sdspidriver.h:85,

                 from L:\Progetti\Arduino\libraries\SdFat\src/SdCard/SdSpiCard.h:35,

                 from L:\Progetti\Arduino\libraries\SdFat\src/SdCard/SdCard.h:28,

                 from L:\Progetti\Arduino\libraries\SdFat\src/SdFat.h:32,

                 from L:\Progetti\Arduino\PICO_SDbench_SDFat\PICO_SDbench_SDFat.ino:11:

C:\Users\Fernando\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.1\cores\rp2040/api/HardwareSPI.h:109:21: note: candidate: 'virtual uint8_t arduino::HardwareSPI::transfer(uint8_t)'

  109 |     virtual uint8_t transfer(uint8_t data) = 0;

      |                     ^~~~~~~~

C:\Users\Fernando\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.1\cores\rp2040/api/HardwareSPI.h:109:21: note:   candidate expects 1 argument, 3 provided

C:\Users\Fernando\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.1\cores\rp2040/api/HardwareSPI.h:111:18: note: candidate: 'virtual void arduino::HardwareSPI::transfer(void*, size_t)'

  111 |     virtual void transfer(void *buf, size_t count) = 0;

      |                  ^~~~~~~~

C:\Users\Fernando\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.1\cores\rp2040/api/HardwareSPI.h:111:18: note:   candidate expects 2 arguments, 3 provided

In file included from l:\progetti\arduino\libraries\sdfat\src\spidriver\SdSpiArduinoDriver.h:92,

                 from l:\progetti\arduino\libraries\sdfat\src\spidriver\sdspidriver.h:146,

                 from L:\Progetti\Arduino\libraries\SdFat\src/SdCard/SdSpiCard.h:35,

                 from L:\Progetti\Arduino\libraries\SdFat\src/SdCard/SdCard.h:28,

                 from L:\Progetti\Arduino\libraries\SdFat\src/SdFat.h:32,

                 from L:\Progetti\Arduino\PICO_SDbench_SDFat\PICO_SDbench_SDFat.ino:11:

l:\progetti\arduino\libraries\sdfat\src\spidriver\SdSpiLibDriver.h: In member function 'void SdSpiArduinoDriver::send(const uint8_t*, size_t)':

l:\progetti\arduino\libraries\sdfat\src\spidriver\SdSpiLibDriver.h:110:34: error: no matching function for call to 'arduino::HardwareSPI::transfer(const uint8_t*&, uint8_t [512], size_t&)'

  110 |     m_spi->transfer(buf, rxTmp, n);

      |                                  ^

In file included from C:\Users\Fernando\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.1\libraries\SPI\src/SPI.h:24,

                 from l:\progetti\arduino\libraries\sdfat\src\spidriver\sdspidriver.h:85,

                 from L:\Progetti\Arduino\libraries\SdFat\src/SdCard/SdSpiCard.h:35,

                 from L:\Progetti\Arduino\libraries\SdFat\src/SdCard/SdCard.h:28,

                 from L:\Progetti\Arduino\libraries\SdFat\src/SdFat.h:32,

                 from L:\Progetti\Arduino\PICO_SDbench_SDFat\PICO_SDbench_SDFat.ino:11:

C:\Users\Fernando\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.1\cores\rp2040/api/HardwareSPI.h:109:21: note: candidate: 'virtual uint8_t arduino::HardwareSPI::transfer(uint8_t)'

  109 |     virtual uint8_t transfer(uint8_t data) = 0;

      |                     ^~~~~~~~

C:\Users\Fernando\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.1\cores\rp2040/api/HardwareSPI.h:109:21: note:   candidate expects 1 argument, 3 provided

C:\Users\Fernando\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.1\cores\rp2040/api/HardwareSPI.h:111:18: note: candidate: 'virtual void arduino::HardwareSPI::transfer(void*, size_t)'

  111 |     virtual void transfer(void *buf, size_t count) = 0;

      |                  ^~~~~~~~

C:\Users\Fernando\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.1\cores\rp2040/api/HardwareSPI.h:111:18: note:   candidate expects 2 arguments, 3 provided

Multiple libraries were found for "SdFat.h"

 Used: L:\Progetti\Arduino\libraries\SdFat

 Not used: C:\Users\Fernando\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.1\libraries\ESP8266SdFat

exit status 1

Error compiling for board Raspberry Pi Pico.
greiman commented 2 years ago

I see where the problem is. The standard class name for SPI devices is SPIClass. Unfortunately The typedef for SPIClass in the board support package is:

// Alias SPIClass to HardwareSPI since it's already the defacto standard for SPI classe name
typedef HardwareSPI SPIClass;

the symbol SPI is not of type SPIClass, but of class SPIClassRP2040:

SPIClassRP2040 SPI(__SPI0_DEVICE, PIN_SPI0_MISO, PIN_SPI0_SS, PIN_SPI0_SCK, PIN_SPI0_MOSI);

I use pointers of type SPIClass* so transfer(txBuf, rxBuf, count) is not accessible. Wish I knew the reason the typedef is not: typedef SPIClassRP2040 SPIClass;

The result is this single function is not accessible through a pointer:

    // Sends one buffer and receives into another, much faster! can set rx or txbuf to NULL
    void transfer(void *txbuf, void *rxbuf, size_t count); 

I suspect this is a bug but I have given up pursuing bugs in SPI libraries.

XFer012 commented 2 years ago

Understood. Many thanks for your support Bill, I will try to ask Earle Philhower if he can have a look at this issue.

It would be really great to have the Pico writing at around 2 MB/s. The RP2040 has some impressive features for high-speed data logging (dual core, fast clock, PIO, enough RAM for ring buffers) but at 500-900 KB/s, writing on SD becomes a bottleneck unfortunately.

greiman commented 2 years ago

Edit: looks like @earlephilhower already has a version with the change. Wow I often fight for months to get changes/fixes with SPI libraries.

Could you do an experiment to see how fast the three argument SPI will be.

Set USE_SPI_ARRAY_TRANSFER to two and change these two lines in SdSpiLibDriver.h

Line 70 from: m_spi->transfer(nullptr, buf, count); To: SPI.transfer(nullptr, buf, count);

Line 105 from: m_spi->transfer(buf, nullptr, count); to: SPI.transfer(buf, nullptr, count);

I use a pointer since many boards including Pico have more than one SPI port. This allows dedicating a second port to the SD.

This should allow access to the three argument transfer.

XFer012 commented 2 years ago

Works a treat!! Thank you very much, Bill and @earlephilhower

With the test "SD2" Arduino-Pico branch from Earle Philhower and your patched SdFat, we get a 2.5x actual SD card speed!

This is, as usual, with Pico overclocked at 250 MHz and "-Ofast" compiler optimization:

FreeStack: 257896
Type is FAT32
Card size: 16.01 GB (GB = 1E9 bytes)

Manufacturer ID: 0X2
OEM ID: TM
Product: UC0C5
Revision: 5.0
Serial number: 0XD74489B2
Manufacturing date: 3/2017

FILE_SIZE = 5000000
BUF_SIZE = 512 bytes
Starting write test, please wait.

write speed and latency
speed,max,min,avg
KB/Sec,usec,usec,usec
2326.66,15641,213,219
2345.22,15578,213,217

Starting read test, please wait.

read speed and latency
speed,max,min,avg
KB/Sec,usec,usec,usec
2418.96,225,210,211
2420.14,244,210,211

Done