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

SDIO storage Read/write, M21, and print from media menu selectable access #25683

Open
wants to merge 26 commits into
base: bugfix-2.1.x
Choose a base branch
from

Conversation

ansonl
Copy link
Contributor

@ansonl ansonl commented Apr 15, 2023

Description

Read and write ability for SDIO Cards. Add SDIO media support to M21. Add SDIO access in user Print From Media menu.

Requirements

This requires the use of a board with an onboard SD slot accessible by SDIO (ex: BTT Octopus/Octopus Pro). If access to other storage media is desired at the same time, #MULTI_VOLUME should be enabled

Benefits

Expose SDIO storage capability to hosts and users.
Many board bootloaders use the 'firmware.bin' method update firmware on startup. This firmware upgrade method often only works with the built in onboard SD slot which uses SDIO. When boards are installed in some printers it is difficult to access the onboard SD slot and repeated physical movement of the board in tight spaces leads to damage.

With SDIO access , the firmware.bin and gcode files can now be read and written to onboard SDIO storage through a host application with M21 O, M28 filename, M29. Remote firmware upgrade will be possible on newer boards.

SDIO can also be selected in Print From Media now as an option when #MULTI_VOLUME is enabled.

Configurations

BTT Octopus Pro Configuration_14apr23.zip

Related Issues

Implements and improves on issue #25197

#12782

@ansonl ansonl changed the title Read/write, M21, and menu selectable access for SDIO storage SDIO storage Read/write, M21, and print from media menu selectable access Apr 15, 2023
@thinkyhead
Copy link
Member

  • Are there some boards with an onboard SD card – or more than one – wired to use both SPI and SDIO?
  • If a board only has media_driver_sdiocard shouldn't M21 P0 / M21 S select it?
  • If M21 is sent without any parameters, shouldn't it mount the same media that was previously mounted?

@@ -543,10 +543,11 @@
#endif

#if DISABLED(USB_FLASH_DRIVE_SUPPORT) || BOTH(MULTI_VOLUME, VOLUME_SD_ONBOARD)
#if ENABLED(SDSUPPORT)
#define NEED_SD2CARD_SPI 1
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SDSUPPORT option is used as the flag for any and all media types, not just SD Cards. If SDSUPPORT is not enabled, none of the file access code (e.g., cardreader.cpp) will be compiled in. So the code needs to be reworked a lot more if the intent is to make SDSUPPORT only refer to SPI-based SD cards.

Since it is possible to have both onboard and external media drives, we would also need to rework the code to remove the assumption that there is only one SD card, either ONBOARD, LCD, or CUSTOM_CABLE.

Copy link
Contributor Author

@ansonl ansonl Apr 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have SD_SPI_STORAGE, SD_SDIO_STORAGE, and USBFLASH_STORAGE options underneath that requires SDSUPPORT to be defined. I see that there are currently 3 media_driver_X types implemented:

  1. media_driver_usbFlash (USB)
  2. media_driver_sdcard (SPI)
  3. media_driver_sdiocard (SDIO)

Even though the location of the SD slots may be onboard or on LCD to the user, the access method for the SD slots can vary between all three methods no matter where the slots physically are. There is a maximum of one SD slot connected for each media_driver_X type, I think

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current assumptions are probably a little constrained. As I've been working on separating out the LCD pins in another PR I've become better acquainted with the way that SD Card pins are allocated, either with or without an LCD controller. Here we have a number of different possibilities.

  • Onboard SD Card drive with dedicated pins, sometimes but not always the native Hardware SPI.
  • Onboard SD Card drive with SDIO, so the card can be released by Marlin and seen by the host PC over USB.
    • I'm not sure whether SPI-based SD Cards can be shared in this way.
  • No onboard SD Card drive, but the LCD has one, usually sharing SPI pins with the (SPI-based) LCD.
  • No onboard SD Card drive, but an SD Card can be added. With native Hardware SPI or Software SPI.
  • Onboard USB Flash Drive is rare, but I have a board or two with one. Presumably Hardware SPI.
    • I'm not sure whether any board has SDIO Flash Drive, or if SDIO is a thing for Flash Drives.
    • I'm not sure whether SPI or SDIO Flash Drives can be released and seen by the host PC over USB.

Right now pins files tend to follow the configuration of the original machine they came with, so if there is an LCD Controller with an SD Card drive, the pins file assumes that the presence of an LCD indicates the use of the SD Card drive on the LCD. Most pins files allow that to be overridden.

The "Custom Cable" option for SD Card drives is handled by very few pins files, and it could be better. If you define your own SPI pins (with SD_ prefix) and use CUSTOM_CABLE then pins files should allow it, but the logic is not there.

There is a lot of overlap between the allocation of pins for the LCD and SD, so it might be good to tackle these two problems at the same time or in a similar manner. See #25650 for that little project, which is coming along slowly but steadily. There I'm trying to weed out assumptions about whether Software or Hardware SPI is used, but also trying to see if that can be based on the chosen pins. i.e., If the pins are not the standard SPI0, SPI1, etc., then we can assume Software-based SPI for that drive.

Marlin/Configuration.h Outdated Show resolved Hide resolved
@ansonl
Copy link
Contributor Author

ansonl commented Apr 16, 2023

  • Are there some boards with an onboard SD card – or more than one – wired to use both SPI and SDIO?

I don't know of any boards that have a single SD slot that can be dual accessed over SPI and SDIO. Most newer boards I've seen have an onboard slot that is accessible by either SDIO or SPI and then the manufacturer or 3rd party has a LCD assembly that includes a second SD slot on the LCD assembly that is mounted somewhere more accessible to the user (like next to the LCD and encoder).

  • If a board only has media_driver_sdiocard shouldn't M21 P0 / M21 S select it?

Currently, P0 and S are hard coded to card.media_driver_sdcard.

  • If M21 is sent without any parameters, shouldn't it mount the same media that was previously mounted?

Yes, that's what I've noticed, I'm not sure what that achieves for the normal user since the menu would just mount the same media from before?

@ansonl
Copy link
Contributor Author

ansonl commented Apr 16, 2023

Expanding M21 to be able to select between SPI and SDIO is also helpful so that the default SDCARD_CONNECTION can be set to LCD for normal use with the accessible external SD slot and M21 O can dynamically change media to the onboard SDIO slot before transferring a firmware file over marlin binary protocol with the Octoprint Firmwareupdater plugin

I just tested M21 functionality to select SDIO media on the fly with #define SDCARD_CONNECTION LCD and Firmwareupdater plugin. If I run firmwareupdater without switching to SDIO with M21, the firmware file gets transferred to the external SD card normally used for print files and no update happens. Running M21 O to switch to SDIO before firmware transfer and reset update succeeds.

When I set #define SDCARD_CONNECTION ONBOARD to default to SDIO slot, my I2C UltiController display no longer works -- somehow due to the LCD SPI SD slot being on the same board as the I2C display...

@thinkyhead
Copy link
Member

I'm not sure what that achieves for the normal user since the menu would just mount the same media from before

It ensures that an M21 after an M22 (for whatever reason) mounts the same media as before. Someone might unmount and remove a card, then insert a new card and call M21, such as for a situation where the SD Detect Pin is not available, (as with an SD extender).

@thinkyhead
Copy link
Member

I suggest that we defer to the board's definition of SDIO_SUPPORT and assume that the onboard SD Card is SDIO in that instance. If it's not SDIO it should still be accessible as the "Onboard SD Card" but using SPI. We should not put that option in front of the user, unless we find that some boards have SD cards that do both SDIO and SPI, and the SDIO is prone to breaking.

If there is a second SD Card drive installed, either on the LCD or elsewhere, that one will presumably be connected via SPI, and we can just call it the "External SD Card."

If there is a USB Flash Drive installed, it should be accessible simply as "USB Drive."

This way we are not assuming the user knows or cares whether the onboard SD card is SDIO or SPI. We should be able to assess that automatically based on the board / pins definition.

When it comes to selecting one of the SD cards or the Flash drive, I suggest we allow the user to specify whatever order they prefer for Drive 0 (default), 1, 2, etc. The onboard SD Card will usually be the default, for setups that have the slot exposed on the side or front of the machine, while the external (LCD) SD Card will be the default on machines that are set up with that slot being more prominent, even though there may be an accessible SD Drive on the board. Of course, going forward we may see USB Flash Drive becoming more common as the default, in which case the LCD SD Card might be the secondary, and the onboard the tertiary or altogether inaccessible, depending on the machine.

@thinkyhead
Copy link
Member

thinkyhead commented Apr 20, 2023

Have a look at #25715 where I'm exhibiting a new concept for configuring drives that aims to simplify the user-level configuration of multiple volumes. It's not meant to compile successfully yet, but just to show how this PR might be continued. Most basically, the user should be able to simply specify what drives they have, and in what order. So…

//#define SDSUPPORT
#if ENABLED(SDSUPPORT)
  //#define DRIVE1_LOCATION ONBOARD
  //#define DRIVE2_LOCATION LCD
  //#define DRIVE3_LOCATION USBFD
#endif
  • The definition for Drive 1 would replace the need to define a DEFAULT_SHARED_VOLUME.
  • The existence of more than one DRIVE#_LOCATION setting replaces the need to define MULTI_VOLUME.
  • Board pins define ONBOARD_SDIO (replacing SDIO_SUPPORT) if the onboard SD is known to be SDIO.
  • SDCARD_CONNECTION is replaced by DRIVE#_LOCATION.
  • The macro SD_DRIVE_IS(1, ONBOARD) (for example) is used to get the drive location / type.
  • Board definitions can use ANY_DRIVE_IS(...) to decide which pins are required.
  • The user doesn't need to set USB_FLASH_DRIVE_SUPPORT, since we can use ANY_DRIVE_IS(USBFD).
  • Ultimately, the SDSUPPORT setting itself can be replaced by defined(DRIVE1_LOCATION).
  • …and so on…

@Bob-the-Kuhn
Copy link
Contributor

I agree that the SDIO/SPI choice be hidden from the user. I see no value add

I agree that the pins_YOUR_BOARD.h files definition of SDIO_SUPPORT should dictate how the onboard SD card slot is handled.

FYI on the hardware side. A board with an SDIO interface to the SD card can access the SD card either via the SDIO interface or using a SPI interface. The SPI option would probably be a software SPI. I'm not aware of a STM32 processor that has both SPI and SDIO mapping to the same pins.

@ansonl
Copy link
Contributor Author

ansonl commented Apr 20, 2023

The concept for #25715 looks good. Please let me know when it is ready to test, my new btt octopus pro board has all 3 storage methods so it would be interesting to see how it works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants