Thank you! I had actually started a similar implementation myself, done this week, which is working but not nearly as comprehensive as what is done here. So I'll forget about mine :)

From a brief look, it looks good. One thing to discuss is where to put the SDCard class: now that two ports support the same thing it might be good to place it in a common module. machine would be a candidate, but it might get crowded adding this and then other related things, like MMCard. So perhaps a new module like storage, which would have scope for other functionality like internal flash storage access, and partitioning block devices (eg to mount multiple partitions on an SD card).

The PR includes documentation of the new class as part of the ESP32-specific library documentation.

It'd be great to make this generic documentation for all boards/ports that have an SDCard class.

Given that most ESP32 devices are not short of ROM space I would recommend having support switched on by default

I agree, it's a common feature to have an SD card slot, and the standard esp32 firmware is supposed to be able to run on all esp32 based boards.

I was wondering into which module I should put this. When I first wrote it I put it in its own sdcard module and then I moved it to esp32 because the PyBoard version is in pyb. Since this is a capability that is common across many platforms, but which is directly related to hardware, machine does seem like the right place, since I'd argue that an SD Card slot is sort of like a Pin, a Timer or an SPI interface. Of course on the PyBoard many of the classes in machine also exist in pyb so it could exist in both, but that does add to the clutter.

Either way, this is a capability that is going to exist on lots of different platforms so a single common place to put it would be good. If we go that route we should make sure that the parameters are common across all implementations as far as possible. I think that the parameters that I have here are probably the superset of what we will find elsewhere but I'm open to suggestions.

Looking through the other ports it seems that the CC3200 port currently contains an SD Card driver that wraps up TI's driver included with the FatFS code. As if to highlight the need for consistency, this class is in the machine module, named SD rather than SDCard and has a rather different signature to the constructor (it just takes an ID and a tuple of pins as a positional argument).

It seems that there are three related questions here:

• Do we want to have strong consistency between the constructors' signatures or do we want signatures that concisely represent just what the hardware needs?
• Do we want to put the class in a device-specific module or in one of the common modules?
• If we want it in a common module, should that be machine or some new storage-related module?

On the signature front, we are talking about the same interface every time but different platforms and implementations have different degrees of flexibility. My inclination would be to require all implementations to:

• Name the class SDCard.
• "Do the right thing" (i.e. connect to the obvious slot with default parameters) when the constructor is called without arguments.
• Allow a 1st positional or keyword argument to select which slot is to be used, and offer this even if there is only one slot. In keeping with the naming of other peripherals such as ADCs and timers this should probably be called id rather than slot. Selecting the non-default slot should set sensible values for the non-default pins.
• Allow (but not require) a keyword argument named width, to set the interface width, and raise a value error if the requested width is not supported.
• If the underlying interface is an SPI port and pins are selectable then the arguments for setting which pins to use should be keyword arguments named to be consistent with machine.SPI.
• Everything else can be platform specific.

If we get to that level of consistency in the driver initialisation process then I would be very happy to see this class land in a standard, platform-neutral location. Without consistency I'd be inclined to keep it in a platform-specific module. If we do end up putting it in a neutral location then my inclination would be to put it in machine and not worry about the clutter, but I'm happy to go with the flow on that question.

Looking through the other ports it seems that the CC3200 port currently contains an SD Card driver that wraps up TI's driver included with the FatFS code. As if to highlight the need for consistency, this class is in the machine module, named SD rather than SDCard and has a rather different signature to the constructor

It may have been called SD because that was what the stm32 port originally called it. And it takes a tuple of pins because that's how the cc3200 port handles all peripherals (eg UART, I2C), but things moved on since then and now pins are named explicitly (eg scl and sda). So I wouldn't take this port as something to be consistent with, it can eventually change to be consistent with other ports.

Do we want to have strong consistency between the constructors' signatures or do we want signatures that concisely represent just what the hardware needs?

Going by other things like UART and Pin: their constructors are quite consistent across ports, at least for a common set of basic and most used functionality. Some ports define additional keyword args to configure extra things. It makes sense to follow this for an SDCard interface, ie try to have consistency.

To state the obvious, at the very least the SDCard object should implement the block protocol so it can be mounted, and then the following code can work on any port:

sd = SDCard(...) # perhaps port-specific initialisation
uos.mount(sd, '/sd')

The cc3200 already works this way.

Do we want to put the class in a device-specific module or in one of the common modules?

In the end I don't think it really matters. Construction/initialisation/configuration is always going to be board/port specific. But if multiple ports will have this class it does make sense to put it somewhere common.

If we want it in a common module, should that be machine or some new storage-related module?

It doesn't seem right to me to put it in machine. The machine module has very general IO/bus objects which can be used for many things, and doesn't have specific drivers for things that build on an IO/bus objects (eg it has I2C but not any accelerometer drivers). An SD card is a concrete device that communicates over an SDIO/MMC bus, so it seems to make more sense to have a general SDIO/MMC bus class in machine and then an SD card driver in a separate module. The SD card driver can then use either an SPI bus or an SDIO/MMC bus as its communication interface.

But I wouldn't actually advocate having an SDIO/MMC bus in machine because it's a very specialised thing and I really don't know what the methods would look like to expose its functionality. Also, it's probably only ever going to be used to interface with an SD card (or MMC) so doesn't need to be exposed explicitly to the user.

Consider external SPI flash, which is logically very similar to an SD card: a driver for this wouldn't go in the machine module (I argue), instead it would be something like storage.SPIFlash() and take in a machine.SPI or machine.QSPI object to communicate over. Also in this case it's arguable whether a QSPI class/interface should be explicitly exposed (similar to SDIO/MMC) because it's almost always going to be used to talk with SPI flash.

In summary: I don't think machine is the right place for SDCard because it a driver for a specific component, not a generic IO bus.

My inclination would be to require all implementations to:

I agree with most of these points. Except that, if the underlying interface is a SPI port, it should accept a spi keyword argument that you can pass in a machine.SPI object to use.

I agree with most of these points. Except that, if the underlying interface is a SPI port, it should accept a spi keyword argument that you can pass in a machine.SPI object to use.

That would certainly make the construction cleaner in those cases.

Do you ses supporting spi=... instead of supporting any of the SPI config on the SDCard constructor? In the case of the ESP32 using the SD-SPI implementation in the SDK we would need to extract the pin assignment information from the SPI object, deinit it and then pass the pin config back to the SDK (which would mean making the internals of the SPI implementation visible to the SDCard implementation).

I am happy to move this to a new storage model. I will update the API based on these comments and move the documentation for the SDCard class into a new docs section for the new module.

Do you ses supporting spi=... instead of supporting any of the SPI config on the SDCard constructor? In the case of the ESP32 using the SD-SPI implementation in the SDK we would need to extract the pin assignment information from the SPI object, deinit it and then pass the pin config back to the SDK

I see, I didn't realise it's this complicated. It seems that the SPI interface used for the SD card is actually SD-SPI, so not really machine.SPI() (although it's closely related). In that case perhaps it's simpler to just keep it like you have it, with explicit sck/mosi/miso pin arguments to the SDCard() constructor. [Also, doing it with a SPI object, it may do something electrically unsound if the SPI is first initialised as a SPI bus then deinitialised. Probably better to play it safe here and just consider the SD-SPI as a different mode of the SD/MMC bus, and pass all pins into the constructor.]

I am happy to move this to a new storage model.

Well, maybe we can come up with a better module name. But at least it has scope for SDCard(), MMCard(), and maybe Flash() for internal flash.

Irrespective of the complexity of implementation (which is my problem, no the users'), I think that the down side to passing in an SPI object is there is a lot more that might have been (mis-)configured on the SPI than just the set of pins. With the software SDCard implementation you need to set this all up yourself but on most platforms that host library code for implementing the SD Card protocol the library will do all of the rest of the configuration and the user only needs to specify the pins.

As for the new module, if it were called storage it might make sense to move stubs and wrappers for the user-facing VFS code into there (particularly things like formatting a new image).

Irrespective of the complexity of implementation (which is my problem, no the users'),

Yes, true, the implementation should not directly dictate the API. But in this case the SDCard is not really using the SPI bus itself, it's just using the particular pin configuration of a SPI bus. As such I would agree with your original proposal to just let the user specify the pins directly to SDCard.

if it were called storage it might make sense to move stubs and wrappers for the user-facing VFS code into there

Yes, it seems attractive to put mount/umount/VfsFat/etc in such a storage module as well. But that mixes layers: hardware drivers with the filesystem itself (a pure software thing). So I'd be inclined to put the VFS items in something like a dedicated vfs module (if anything).

To make progress here, how about we just put it in the machine module, as machine.SDCard?

OK. I've moved the class back to the machine module and also added documentation.

Thanks, it looks pretty good. I will squash all the commits when merging.

I guess the changes to docs/library/esp32.rst should be dropped, since all the relevant info is now in docs/library/machine.SDCard.rst?

I guess the changes to docs/library/esp32.rst should be dropped, since all the relevant info is now in docs/library/machine.SDCard.rst?

Yes. My mistake. Now that I've written then generic documentation that file should have no changes. I'll fix that.

Ok, I squashed this into 2 separate commits (implementation and docs) and merged it in 8e3af7d and 6077d17

Thanks very much for your efforts!

Re MMC: stm32 does support both SD card and MMC and they are actually different classes, so the docs would eventually need to be updated to reflect that fact (or combine them into a single SDCard class...). Since esp32 doesn't (yet) support MMC it's not clear how the API for MMC on esp32 would look at this stage.

Thanks, and glad to be of service.

Regarding MMC support, as far as I can see from the documentation the API for handling this on the ESP32 is expected to be the same as for SD cards. They two interfaces share the same logical protocol and only differ at the hardware level. The code here may need either new options or new slot numbers to enable MMC support just to tell the hardware which type of hardware is using the (shared) pins but otherwise it should look just the same. As a result I would lean towards a single class rather than a separate MMC class.

They two interfaces share the same logical protocol and only differ at the hardware level.

The do share the same physical pins, so the difference lies between the physical IO and the high-level interface. On stm32 SD and MMC are two completely different HAL layers, although these layers do have a lot in common. I'm not sure how to distinguish between an SD card and MMC connected to the SD/MMC bus, so in stm32 there is currently pyb.SDCard and pyb.MMCard. You must instantiate the one corresponding to what is connected.

The code here may need either new options or new slot numbers to enable MMC support just to tell the hardware which type of hardware is using the (shared) pins but otherwise it should look just the same. As a result I would lean towards a single class rather than a separate MMC class.

That does make sense (to have a single class) since the bus is the same it's just the mode that is different (SD vs MMC).

In that case I would really like to rename machine.SDCard to machine.SDMMC to reflect the fact that it may/can support both. If it's possible to automatically distinguish between a connected SD or MMC then that's great, you always simply call machine.SDMMC() to access it. If automatic detection is not possible then we can add an option like mode=MMC or mmc=True.

Thank you very much for this, I was looking for something like this before. I'm hoping I can use this to serve a small spa webpage using micropython and javascript
where micropython just acts as a basic rest API, and all the frontend is done via a single bundled javascript file bundled via webpack and served out.
The last time I tried this on an ESP32 the main limiting factor was the read speed from the SD card

esp32/machine_sdcard.c: Invalid pin numbers are allowed. #8112