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

Multiple luns #6555

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Multiple luns #6555

wants to merge 9 commits into from

Conversation

hathach
Copy link
Member

@hathach hathach commented Jul 4, 2022

This PR exposes up to 4 LUNs for MSC, for additional fatfs such as SDCard. Mostly based on @dhalbert https://github.com/dhalbert/circuitpython/tree/multiple-luns branch combined with following changes

  • response to SCSI_CMD_PREVENT_ALLOW_MEDIUM_REMOVAL as unsupported request in all msc examples. This is required for macos to send TEST_UNIT_READY regularly for "SDCard reader" application. Response OK to this request for non-removal device such as ram/flash disk can be beneficial since macos will skip TEST_UNIT_READY command and assume medium is always present (saved bandwidth).
  • tud_msc_set_sense(lun, SCSI_SENSE_NOT_READY, 0x3A, 0x00) is optional, since tinyusb stack will default to response with that when test unit ready callback return false.

NOTE: I originally planned to make PR to Danh's fork as of dhalbert#1 . However, the tinyusb lib version there requires update to work with macOS, therefore I merge from latest main since it already got later version of tinyusb. Now the changes compared to Danh's fork is too large. So it is better to just make PR to main directly.

Tested with PyPortal using built-in card reader with the following code.py on windows/linux/macos. Note: SD is slow, if your card has lots of files, it may take several minutes to show up on the file explorer.

import os
import digitalio
import board
import busio
import storage
import sdcardio
import time

spi = busio.SPI(board.SCK, board.MOSI, board.MISO)
sdcard = sdcardio.SDCard(spi, board.SD_CS)
vfs = storage.VfsFat(sdcard)
storage.mount(vfs, "/sd")

def print_directory(path, tabs=0):
    for file in os.listdir(path):
        stats = os.stat(path + "/" + file)
        filesize = stats[6]
        isdir = stats[0] & 0x4000

        if filesize < 1000:
            sizestr = str(filesize) + " by"
        elif filesize < 1000000:
            sizestr = "%0.1f KB" % (filesize / 1000)
        else:
            sizestr = "%0.1f MB" % (filesize / 1000000)

        prettyprintname = ""
        for _ in range(tabs):
            prettyprintname += "   "
        prettyprintname += file
        if isdir:
            prettyprintname += "/"
        print('{0:<40} Size: {1:>10}'.format(prettyprintname, sizestr))

        # recursively print directory contents
        if isdir:
            print_directory(path + "/" + file, tabs + 1)

   # print only 1 file
   break 


print("Files on filesystem:")
print("====================")
print_directory("/sd")

while True:
	time.sleep(1)

@dhalbert
Copy link
Collaborator

dhalbert commented Jul 7, 2022

@hathach I tried this on Linux, macOS 12.4 (the latest), and Windows 10. I wrote a code.py that mounted the SD card file system, and then just looped to stay in code.py.

On Linux and macOS it worked OK. On macOS the SD card drive appeared quickly; on Linux it took a few seconds longer. Occasionally it would get hung up, but a reset fixed things.

On Windows, I had more trouble. Windows took a long time to show drives D, E, F, G, as long as 15 or 30 seconds. Drive D was CIRCUITPY. Trying to edit a file on the SD card drive caused a spinning mouse cursor, and did not work. Sometimes after a reset the drives would not appear, and I had to resort to unplug/replug. In general it was not a satisfactory experience.

Did you notice similar behavior on Windows, or did it work better for you?

Since we can't tell what the host system is, we could do something more restrictive, but simpler: we could restrict this to allowing only as many LUNs as mounted filesystems. The SD card or other secondary filesystem would need to be mounted in boot.py to be assigned a LUN. That might get around the problems on Windows. @ladyada is OK with this.

@hathach
Copy link
Member Author

hathach commented Jul 7, 2022

@hathach I tried this on Linux, macOS 12.4 (the latest), and Windows 10. I wrote a code.py that mounted the SD card file system, and then just looped to stay in code.py.

On Linux and macOS it worked OK. On macOS the SD card drive appeared quickly; on Linux it took a few seconds longer. Occasionally it would get hung up, but a reset fixed things.

On Windows, I had more trouble. Windows took a long time to show drives D, E, F, G, as long as 15 or 30 seconds. Drive D was CIRCUITPY. Trying to edit a file on the SD card drive caused a spinning mouse cursor, and did not work. Sometimes after a reset the drives would not appear, and I had to resort to unplug/replug. In general it was not a satisfactory experience.

Did you notice similar behavior on Windows, or did it work better for you?

Thanks Dan for detail testing, hmm, I actually test this more on Linux (main pc) and macOS
since we have issue with macos before. I didn't test much with windows, only plug it in to check if the drive appear. Let me do more testing and update this PR if needed.

Since we can't tell what the host system is, we could do something more restrictive, but simpler: we could restrict this to allowing only as many LUNs as mounted filesystems. The SD card or other secondary filesystem would need to be mounted in boot.py to be assigned a LUN. That might get around the problems on Windows. @ladyada is OK with this.

I guess there maybe some scsi command that does not meet windows expectation. I will pull out my analyzer to test with.

Copy link

@lilmoonlil lilmoonlil left a comment

Choose a reason for hiding this comment

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

supervisor/shared/usb/usb_msc_flash.c

@hathach
Copy link
Member Author

hathach commented Jul 8, 2022

@dhalbert confirmed the slow enumerating on windows 10, is troubleshooting, have no idea I miss this in the first place. Will post more update when I have one.

@hathach
Copy link
Member Author

hathach commented Jul 11, 2022

@dhalbert I have updated the msc to fix issue with null vfs. These should fix current issue with windows. Would you mind testing it again.

also updated tinyusb that has recent fix for samd lock-up with high traffic hathach/tinyusb#1535 (also point the stm to new dwc2 driver which is rquired for L4 family) tinyusb update requires code changes to host API with testing and should be better to be done on a separated PR, feel free to squash the commits.

PS: Issues are caused by windows skipping Test Unit Ready for certain commands (probably for speed) and try to read & access LUN. Previously, we didn't check the SD existence for read capacity and/or read/write --> caused misbehavior due to incorrect response. Linux/macos doing better job by always test to check if unit is ready first before doing any access command.

@tannewt
Copy link
Member

tannewt commented Jul 11, 2022

FYI, Dan is out on vacation this week and should get back to you next week on this.

@hathach
Copy link
Member Author

hathach commented Jul 12, 2022

thanks @tannewt for the info

@dhalbert
Copy link
Collaborator

I had a user test this, and they said they were unable to create new files on CIRCUITPY after the latest commit, using Windows

I tried the artifacts from the the latest commit on PyPortal on Linux with an SD card inserted, and confirmed that. I did this test on the artifact from before and after the latest commit.

$ cat >/media/halbert/CIRCUITPY/foo.txt
abc
[ctrl-D]
$ sync
$ ls /media/halbert/CIRCUITPY

Note that I did not cd /media/halbert/CIRCUITPY, because if the drive gets remounted, the directory I'm in cam become stale.

On the previous artifact (a512f83), foo.txt gets created. On the latest commit (e780461), after the attempted write, one or both drives get remounted, and foo.txt does not show up.

@hathach
Copy link
Member Author

hathach commented Jul 21, 2022

thanks @dhalbert for testing, just want to re-confirm

  • Does the multiple lun work with pyportal, i.e both CPYTHON and SDCard show up fine with all platforms with above code.py
  • For the unable-to-create new file, do you test with running code.py (2 LUNs show-up), or doing it without the code running. I will test with both but asking just to be sure.

@dhalbert
Copy link
Collaborator

dhalbert commented Jul 21, 2022

I am testing e780461 with a PyPortal with the simpler test code below. I initially only tested on Linux, but here are results for all.

Linux Ubuntu 22.04: as above. Both drives appear, but when attempting to create a new file on either drive, it never shows up. Sometimes it appears as if the drives are remounting (disappear and appear in the file manager).

Windows 10 (latest version): Both drives appear, and show the files I expect. But if I right-click and try to create a "New Text Document", I get an error window like the below. I also tried saving a file in Notepad, and got a different error about the drive not existing.
multiple-luns-create-file-windows

macOS 12.4 on an M1 Mac Mini: The drives never show up. The PyPortal screen shows auto-reload happening over and over every few seconds. This is quite new behavior. macOS used to be the best-behaved.

Test program, which just mounts the SD card and waits. I am using an 512MB (not a typo - it's small) SD card.

import board
import sdcardio
import storage
import time


sdcard = sdcardio.SDCard(board.SPI(), board.SD_CS)
vfs = storage.VfsFat(sdcard)
storage.mount(vfs, "/sd")

while True:
    time.sleep(1)

@hathach
Copy link
Member Author

hathach commented Jul 21, 2022

@dhalbert thanks, yeah, I confirmed the issue with creating new file. I probably don't test this enough. Will definitely running some disk benchmark this time to make sure everything is OK. Will push the fix if as soon I could

@tannewt
Copy link
Member

tannewt commented Nov 15, 2022

@hathach Did you ever get back to this?

@hathach
Copy link
Member Author

hathach commented Nov 16, 2022

@tannewt unfortunately not yet, I have switched to other esp32/rp2040 works. Will back to this later when possible.

@microdev1 microdev1 marked this pull request as draft February 25, 2023 11:15
@eightycc
Copy link

Perhaps using a single LUN that presents additional devices as partitions in a fake MBR would work around host OS problems.

@dhalbert
Copy link
Collaborator

Perhaps using a single LUN that presents additional devices as partitions in a fake MBR would work around host OS problems.

Could it be dynamic in that case? Would the partitions for unmounted filesystems be visible?

@eightycc
Copy link

Could it be dynamic in that case?

It could be dynamic in the same sense as adding/changing/deleting partitions with fdisk if you imagine fdisk running inside CP. You'd need to remove the single LUN from the host OS and reinsert it in order to ensure that the host OS sees the changes. It could be messy.

Would the partitions for unmounted filesystems be visible?

No. We'd elide all but existing filesystems from the fake MBR.

This approach would work best where the additional SD card devices are inserted into the CP system before attaching USB to the host and are then left undisturbed until unmounted from the host OS.

@hridpath
Copy link

hridpath commented Mar 9, 2024

Just an FYI... Espressif ESP_IDF has the esp-iot-solution with examples. An one of those examples is the usb_msc_wireless_disk.
I have loaded this example on an ESP32S3 board and it works for windows very well.
May be take a look at what they have done in the example and implement something like that.

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

Successfully merging this pull request may close these issues.

None yet

6 participants