-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
rp2: Add MSC support. #7402
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
rp2: Add MSC support. #7402
Conversation
6c1c727
to
f68d994
Compare
@iabdalkader how does this work? The same filesystem is shared between both micropython internally and the host filesystem, right? Usually operating systems do not expect the data to be changed on the medium by anybody else than themselves, so they take quite a lot of liberty with caching and delayed writes. Doesn't this corrupt data? |
Yes. That is a common problem since the first days of the STM32 port, meaning that people who do not like that trouble simply avoid like the plague the MSC mode. |
No I don't think so, this is based on the tinyusb ramdisk msc example, they use "TinyUSB" for vid, which is 7 characters so seems there's no requirement to null terminate in this function at least. |
Well, it is 7 chars so there it will actually be null terminated. But I have checked the msc_device.c in tinyusb code and they say: |
How about this ? will always leave 1 space in the buffers: strncpy((char *)vendor_id, vid, 8-1);
strncpy((char *)product_id, pid, 16-1);
strncpy((char *)product_rev, rev, 4-1); |
That should be with sizeof(). After checking the code of tinyusb I think that it is safe to fill the buffer without null termination, though. |
Can't use sizeof (gcc 10):
I agree the comment clearly says to fill the buffers with a string up to 8, 16, 4 characters respectively, so will keep 8, 16, 4. |
Ach, damn it, I forgot about this stupid rule in C. I know that the array is acctually passed to the function as a pointer but if we are allowed to specify the argument to be an array of fixed size, not a pointer, we should have a way to extract the size we have declared (so that we don't have to repeat ourselves).. but there's no way to do this in C. Bummer. |
6eed135
to
519245d
Compare
MSC mode is a support PITA in the forum. We are forever having to explain to users why they have trashed their filesystem, I'm very much -1 for this. |
Nevertheless it's still valuable to have standard access to the filesystem, at least the option to enable it if it's needed without having to use some Micropython fork (note this is not enabled by default and it's configured per-board not for the whole rp2 port). That said maybe we can avoid some of these forums questions by disabling the internal filesystem access as long as the storage is mounted/used by the host (this may need to be done at higher levels) this way it can only be corrupted if the board is not safely removed (same as using a USB stick). |
Fortunately I have no vote here because it is a hard choice. On one hand, having this feature disabled should keep surprised people out of the forums, as only somewhat more experienced users would build their own version with this option (but not having this problem documented may mean they will still be surprised), on the other hand not having this feature enabled by default may mean it will be harder to maintain in good shape. |
This should be common behavior to all ports that support USB MSC in the same way (ex: the stm32 port MSC support is more or less the same, and has the same issues), so it needs to be supported somewhere in upper layers, but not sure where exactly and it's unlikely to be fixed in this PR. |
Rather than disabling, it would be best to provide read-only access. This would fix the problem, but the r/o restriction would need to apply only to that filesystem. The user might have an SD card or an EEPROM chip attached and would expect full access. As far as I know STM is the only port to allow MSC mode so my guess is that @dpgeorge thought better of it. It would be interesting to hear his view. |
Unrelated to the MSC discussion, was wondering why does this port enable both FAT and LFS2 at the same time ? Are both filesystems needed at the same time ? If not I can merge the frozen modules that create the filesystems in one module. |
The default file system of the RP2 port is LFS2. For people who attach a SD card to RP2 or who prefer FAT, FAT was added as well. So you may have FAT and LFS2 being used at the same time, but not at the same media. No need to change that. Does using MSC require the file system of the RP2 to be FAT? |
No not really but it's the only filesystem that will work out of the box on all OSes. |
Will MSC work with Windows, OS X or Linux, if the RP2 file system is LFS2? |
Depends on what you mean by "work". MSC will, in general, work, you can access the block device but you can't access the files unless you mount the filesystem. There is a littlefs-fuse,a userspace driver for LFS2 filesystem, that could be used to mount the filesystem on Linux. I'm not sure if anything similar is available on Windows or Mac OS X. Technically there is no reason why it wouldn't be possible but stock installation for sure isn't going to work. |
With "work" I mean indeed: use files on the file system. A little search revealed already the littlefs-fuse driver. But without file access, MSC support is of little use. That brings it back to FAT on the RP2, with all the PITA, as Peter said. I used that in the beginning with PyBoard. And even if I tried to be very careful, I wrecked the Pyboards filesystem regularly. Once I stopped using MSC, everything is stable. Whether Littlfs behaves more stable is to be determined. Using mpremote and it's reverse mount is definitely robust. |
Some of the problems could be mitigated slightly if we had two separate filesystems - the internal one and external one. Internal could be LFS and not accessible via MSC, the other one could be FAT, that would be read-only on the microcontroller (could be made writable, which would disable MSC, or it could become writable when MSC is not in use). In this case, the exported FS could still be broken but at least that wouldn't brake the internal one. As long as the MSC is properly ejected before removal, the external FS should be able to survive for some time. |
My understanding of the problem is that it's unrelated to the filesystem. It is a consequence of the USB MSC spec. The PC assumes that the MSC device is a dumb disk drive. If instead the device modifies the filesystem chaos results. I really can't see any good reason for providing this. Most ports don't have MSC mode. In the one which does (STM) it is an endless source of grief until people learn how to disable it. I would question whether any experienced users actually employ it. Disabling write access if MSC is enabled would fix the problem, but then we'll have to field queries about why perfectly valid Python code throws exceptions... |
Not really. USB MSC has nothing to do with filesystems, as far as I understand. It just exposes the block device. The problem is with the filesystems in the Operating Systems. In order to achieve good performance, they cheat - they don't flush everything to the block device as soon as possible and they use cache instead of reading everything from the device itself. Basically, OS assumes it is the only one who modifies the content of a filesystem. |
Look like we all say the same: trouble is ahead. The operating systems behavior cannot be reliably controlled. Even splitting the board's file system in two partitions is not really an advantage. It will keep the board's own file system intact, but the one accessed by the PC will be corrupted regularly and therefore not useful. A better alternative would be to implement MTP or the like, which are robust with respect to sudden disconnects and a file system agnostic. |
MSC is a lifesaver for field maintenance. The way I use it is very easy and reliable. I design my systems so the MCU is powered from USB even when the system is off. Just turn the system off, plug in a USB cable and copy files to PYBOARD. I rely on whoever is available at remote sites, usually an electrician or technician, to do this. It's not their main job and I don't expect them to install drivers. In fact many corporate computers are so locked down they couldn't even if they wanted to. If you want to make it really reliable you can sense when USB is connected and disable disk writes, and disable outputs when system power is down. |
Flash fix moved to |
c7a2de5
to
7ae1017
Compare
Hello @dpgeorge I pushed another commit: 7ae1017 which gives exclusive access to the filesystem to the host (if the board is connected to a host) or access to the device if it's not (if for example powered from battery). If the board is connected to a host, the storage must be ejected to access the filesystem from the device. This should make MSC as safe as possible to use, perhaps it will help with making a decision about this PR. |
I'm still undecided about this. There are good arguments on both sides, for and against having MSC support. It does provide a very simple way to get started and develop code, but for anything beyond that it has too many limitations. And having USB MSC available puts a big dependence on the FAT filesystem (which is not great for embedded) and constraints on how block devices integrate with the rest of the system. I would much rather see |
YES! This needs to be promoted to new users (I'm not sure quite how). |
Here are my current thoughts on the situation. What it boils down to is not whether MSC is useful (it is) or whether ports with native USB should support it (IMO they should). But rather whether MSC is enabled by default and and encouraged for use by new users, and I don't think it should be. Serial IO (stdio, input and output) is a far more fundamental feature than MSC. Serial IO really is the minimum common feature of all ports. Serial can be over UART, USB, BLE, telnet (WiFi, ETH), I2C, CAN, etc. So it seems like a good choice to make serial the main way to control and interact with a board for development purposes, to build the workflow around serial. MSC as a workflow does have the advantage that it is zero install, that it just works on a PC without installing anything. And that is a big selling point, and the original selling point for developing with MicroPython and the pyboard. But these days serial has a much lower barrier to entry, because on Windows 10 the USB CDC driver is there by default, and Chrome has WebSerial/WebUSB (Linux and Mac have USB CDC as well). We've also seen that serial-only access has been just fine for esp8266 and esp32 (prior to S2, S3 variants). The micro:bit also doesn't have MSC, only serial (apart from BLE), and it is used in primary schools for teaching beginner programming. Mu (which has serial filesystem access) and the micro:bit online editor have shown that a lot can be done with just serial access, and that it can be made very accessible with a low barrier to entry. Similarly Espruino is based around serial access with a Chrome app (AFAIK) and is very beginner friendly. From this, it makes sense to me to develop more tools around serial access, to improve mpremote, make a web-based mpremote, and maybe even a native mpremote GUI version (also lots of docs!). This is all with the aim to lower the barrier to using the serial port. A web-based mpremote could offer a zero-install workflow, and one that allows a pathway for the user to more powerful tools based on serial access. MSC only works for deployment of .py and .mpy files to a FAT filesystem. Serial access allows much more flexibility, eg a range of filesystem types, deployment of apps by downloading an entire filesystem image (like a pre-built littlefs image, or a linear filesystem with memory mappable .mpy files) and many other things. MSC is still a useful feature, and can definitely exist as another building block for the user, so it can be used when it makes sense, eg for easy configuration/editting of files in the field. But I don't think it should be promoted as the default way to develop (although it would still be possible to do that if so desired). Summary: serial is more fundamental than USB MSC. MSC can be supported by a port, but as a feature, not a fundamental part of the workflow. Want to improve, extend and promote mpremote more, eg a web-based version. Given the above, to make progress with this PR I suggest to keep the bits that add MSC support so it can be used if needed, and eventually make it possible to dynamically select MSC or not (so users don't need to rebuild the firmware to enable it). |
Would it be possible to enable it per board as it is right now ? Should the exclusive access feature be kept ? If so, it would be much cleaner to add a |
Ideally it would be dynamically selectable, eg like stm32's
I don't think so. It adds more complexity and makes it harder to document what the behaviour is. If MSC is going to be an advanced feature, then exclusive access is not needed. |
Is there something blocking this ? |
The first commit here (that adds MSC support) was merged in 507ad03. I didn't merge the exclusive access commit. As above, I think it adds too much complexity. Next step would be to add a way to select the USB configuration at runtime (so the user can enable USB MSC without rebuilding the firmware). |
@iabdalkader MSC is a great feature for me, thanks for your great work.
|
That's a good example why MSC support is first of all a source of trouble. The PC assumes it is the sole user of the file system. So it will update the file system on it's own behalf. If you modify the board's files and forget to synchronize (eject properly), then you end up with a corrupted files system. Also, never make change to the file system on the board itself while it is mounted at the PC. The PC will not notice the changes, and when doing own changes -> corrupted file system. The only way to avoid the latter is to make the file system exclusive. That's what adafruit is doing. But that is inconvenient. |
@robert-hh Thanks for your suggestion. |
@MichaelDu9226 Hi, this is a known limitation with MSC that you need to be aware of, you can't use the FS from the device/host at the same time, and you shouldn't need to just reset the board to sync the changes. Note I did not add exclusive access to OpenMV firmware for any port. |
Yes. I use a reset to sync the changes on PICO.
Another bug, I found that the MSC needs to be formatted every time Pico is powered off and restarted. I'm not sure if it's my compile problem. |
OpenMV boards use the stm32 port, there might be some minor differences with caching etc.. but this doesn't fix the sync issue as it also depends on the host flushing the changes.
I've been using it on Nano RP2040 it's working very well, check if you have a |
Hi, @iabdalkader Thanks for your reply.
|
@dpgeorge - Your summary above has a lot of insights packed into it. Yet, I think several important factors haven't been highlighted sufficiently:
|
Added M5Stack Atom U board
Any interest in adding MSC support for RP2 ?
MICROPY_HW_USB_MSC
in mpconfigboard.h