Mount issues for a Windows-formatted card #475
Comments
|
Window may have formatted it GPT. SdFat only accepts cards that comply with the SD Association Standard. You can try the SD Standard Formatter or this example. I do this since PC, MAC and Linux formatters do not produce formats with optimum performance. Modern SD card have complex flash structure, RAM buffering and and algorithms. |
|
Thanks for a quick answer. I have other SD cards that SdFat reads with no issues, and I am sure I can fix the problem the way you described - i.e. by formatting the card differently. But what I am actually building is a library, to be used by other folks with their SD cards that I don't control. SdFat works much faster for me than the native ESP32 implementation when browsing over a large filesystem (thousands of files) - even in shared mode SPI. Impressive work! That's why I am quite interested in using it in my project. But my use case is to let people create their SD content on a 'big' machine (Windows being the most common, presumably) and then stick it in the microcontroller. Therefore, I do find it a bit worrying that an SD card prepared on Windows might be unreadable with SdFat. It is especially so given that Windows doesn't appear to allow to choose the partition table type when formatting the card. (I suspect this card might have been formatted to GPT at some point to make it bootable, and then Windows keeps it when re-formatting). All that considered, and assuming that it is indeed a partition table type issue, would you consider a feature request to support it in SdFat for better compatibility with the Windows/Linux world? I would be happy to run any extra diagnostic or even ship the card out to you if it helped to scope the work. Also, assuming that the feature is reasonably small scoped (i.e. doesn't affect the filesystem code), I might be able to help with the implementation, if you were willing to give pointers and some support. |
|
Maybe, at the minimum, SdFat could recognize GPT and return a more specific error message when failing to mount? |
|
I don't plan to support non standard formats. If you need that use the ESP SD library. There are now dozens of Board Support packages like ESP and I can't provide comparability with each of them. These are hundreds of Arduino compatible boards so I can't fix poorly written SPI drivers. SdFat preforms well because it is designed to match the internal format and structure of SD cards. This will become more critical as SD cards evolve and have more sophisticated flash structures. It is Windows that should format media according to standards. For example, cards now have huge flash pages that must be aligned on cluster boundaries for performance. The SD standard specifies the size and location of all file structures. That is why the SD formatter has no options. |
|
Actually, this does NOT seem to be a GPT issue. Windows says this is MBR, basic partition, FAT32 w/ 8KB allocation unit size. Let me know if it changes anything. I agree with your points on what Windows should be doing. Except that it doesn't. Taking a principled stance on this is yours choice to make, but the practical consequence of it is more pain and surprise for your users, and narrower applicability of your library. Hypothetically, say I wanted to build and sell a basic microcontroller-driven picture frame (not what I am doing, but a perfectly fine and simple application). If it doesn't work with Windows-formatted SD cards out of the box, it is DOA. It is just not practical to require people to download additional software to reformat their cards. Anyway, I respect your decision. Let me know whether the new info about the partition format changes anything here. |
|
I have no idea what block zero is. It is not an MBR.
This much is ASCCII: I would need to look at all the ways Windows, macOS, Linux, Android, ios, and other OSes format SD cards to provide sane messages. If you look at Linux USB key or SD mount code you will see dozens of special case tests for specific USB drives and SD formats. I posted the first Arduino SD library in 2008 and then SdFat in 2009, assuming Arduino would provide and maintain a standard SD library. SD.h still is based on a patched version of the 2009 beta that only supports short file names and FAT16/FAT32. Any reasonable OS should have an integrated file system for SPI flash, SD cards, USB keys, SSDs, and HDs. |
|
Huh! ASCII junk in the partition table? :) You got me quite curious, so I did some digging. Here's the TL;DR version: Apparently there is no partition table at all. Sector zero just contains FAT32 header of the assumed single partition. How to detect this?
Details: Some more analysis of the FAT partition data in partition zero, created by Windows (not sure how standard that is?): https://flylib.com/books/en/2.48.1/boot_sector.html Sector zero of my card: What do you do with all this? Maybe nothing. Feel free to close this request if you think it is infeasible , especially if there are other format differences besides the lacking MBR. I understand your disappointment with Arduino, and the burden to support historical variants of formats that Microsoft and others came up with over the years. On the other hand, you have authored an SD card library for microcontrollers that might be the best in the world, and is making quite an impact. You must, and should, be proud. Why not try to take it an extra mile and enhance compatibility with real-life media out there, to increase this impact even more? I get it when you say that the Linux SD mount code is discouraging - but my guess is that Linux accumulated tons of cases that have little practical importance today. By 80/20 rule, you should be able to cover 80% of the market by doing 20% of the work, focusing on the most popular operating systems in use today. Or 90/10 if you're lucky. Windows + macOS are 90% of desktop. Windows 10 + 11 are 94% of Windows. |
|
That format is often referred to a "Super Floppy" format. Super Floppies were removable drives with about 200-300 MB with no partition table. SdFat has code to mount this format but SDs in this format had vanished from the market. Apparently some Chinese companies are producing these. I have a to do to try this format as a last resort. Try replacing the two functions here and here with the following. I don't have an SD in this format to test it. |
|
I managed to create a SD with your format. I used the SdFormatter example to erase a SD then used Windows Disk Manager to format the SD like a floppy. Here is the output from SdInfor:
The mod above can mount this card. I need to fix SdInfo to mount a Super Floppy. |
|
Here is a better fix. Replace this function with: This also fixes SdInfo.
|
|
Thanks for such a quick response! I am still debugging things on my end. Things look very promising - I can now mount that card, and I see the 'benchmark' example passing. Also, I was able to successfully scan the directory structure, of ~2000 files, opening ~1000 ZIP files and reading their manifests correctly. I am still seeing some data errors in my app, but that is likely because of some latent bugs due to a slight API differences between SD vs SdFat. |
|
I published this mod plus other fixes and mods as SdFat-beta. After about a month I will make it the release version. |
|
Thank you, this is great! I confirm that the fix works all the way for my card. For what it is worth, in my tests it comes out slightly slower than a standards-formatted card, but the diff is marginal (2-3%). (One thing I discovered while debugging was that ESP32's standard SD library keeps the underlying file descriptors shared between the copies of a file object, so this: File f = ...; advances position also in f2. SdFat doesn't do that, which makes more sense to me.) I have one last question. I have been having some trouble with O_TRUNC. Am I crazy or are writes effectively ignored when the file is open with O_TRUNC? Specifically, what I was doing is f.open(O_WRONLY | O_CREAT | O_TRUNC); and the file seems to come out empty, as if truncation happened on close rather than on open. write() did not return any errors; it reported all the data as written. Is this expected behavior? (I was expecting to see the previous content overwritten by 'bunch of stuff'). |
|
I use O_TRUNC in many examples so I think it works:
I quickly hacked this example as a test. It was from the first version of SD.h based on SdFat. You may need to edit it for ESP32. Here is output for the first run of the example - test.txt does not exist and it has data after close.
Here is the output for the second run. test.txt exist and gets truncated to zero length but has data after close.
|
This could cause problems if you write f and f2. You should never have a file open with two file objects for write. Edit You should keep one File object for an open file and use call by reference or call by pointer in functions, not call by value.
|
|
I think I am zooming in on a problem. And it is: the copy/move for file objects isn't working properly. Small repro is this: void loop() { // Discard any input. cout << F("Type any character to start\n"); if (!sd.begin(SD_CONFIG)) { if (!f1.open("bench.dat", O_RDWR | O_CREAT | O_TRUNC)) { FsFile f2 = std::move(f1); if (f2.write(buf, BUF_SIZE) < BUF_SIZE) { cout << "File size: " << f1.size() << endl; f1.remove(); (it leaves the file truncated to zero, and shows the 'unexpected' message). But let me go back to the second issue first, though (the one about the copy constructor). In the vanilla Ardunino SD library, and also in the ESP Arduino implementation, the File object acts as a de-facto (smart, shared) pointer to a file descriptor entry. That is, it can be freely copied around as if it was a regular pointer (or a numeric index into the FD table, as in Unix), but it is 'smart' in that it counts references and triggers close when refcount reaches zero. In that model, this is perfectly valid, and fairly idiomatic: void ReadHeader(File f) { void ReadPayload(File f) { void ReadFromFile(const string* path) { The same pattern doesn't work in SdFat. Conceptually, you seem to be thinking about the 'file' object as a file descriptor structure itself, and not just a pointer to it. I think this is a reasonable design, except for two problems:
Of course, a simple fix is to pass f to helper functions by reference rather than by value. But one needs to be aware of the problem, and of the difference, in the first place, in order to do so.
Either way, I think this is a bug, and I think there are two possible ways to approach it:
I hope this is helpful! |
|
SdFat was the first SD library for Arduino and the File classes have always been file descriptor structures. Too many apps have been written since 2009 to change it. The way ESP implemented File is one of many different implementations of SD.h Arduino made a wrapper for SdFat but and File object has a SdFat file object in each SD.h file descriptor. If you open a file twice you get the same behavior with most implementations of SD.h. This is the behavior most people think is standard for SD.h. Here is the Arduino SD.h File creator: ESP is based on the C FatFS library and has the form you describe. ESP can't put the file structure in File since FatFS makes it private and returns the Linux style integer from open. Other board support packages have other versions of the File object. Teensy is based om SdFat but has yet another File object. There is a ESP2866 package base on SdFat that has yet another implementation of File. So change is not possible since I would break thousands of application and some board support packages. SdFat shows the constraints imposed by a library for a 328 Arduino with 2K RAM and 32K flash. At the time I wrote SdFat Arduino C++ didn't even have a new operator. That's why the above uses malloc. |
|
I looked at your code more carefully, and I figured I was largely wrong about the diagnosis. FsFile has a good-looking copy constructor and the assignment operator. So, generally, the object should be copyable, with the caveat that writing to both copies independently is asking for trouble (like mutating the same file in parallel in general would be). (By the way, is read problematic as well?) Not having the move constructor and assignment simply means that std::move() has no effect, and the calls invoke the copy constructor and assignment op instead. So my repro is equivalent to this: FsFile f1; FsFile f2 = f1; cout << "File size: " << f1.size() << endl; f1.remove(); I think it should work as expected, since the state is coped forth and back. (Do you agree?) And I can't see from the code why it isn't. But it consistently results in a truncated file... What is extremely surprising is that the last copy actually matters, even though it is done AFTER the file is closed after write()! That is, the following works correctly (file is non-zero): FsFile f1; FsFile f2 = f1; cout << "File size: " << f2.size() << endl; Also, worth noting that it doesn't report any errors (like file not opened etc.) but it simply results in an empty file, so it actually suggests something related to O_TRUNC. In fact, removing O_TRUNC from the original repro also removes the problem! I can't figure out what is going on, except perhaps if there is some delayed flush that gets messed up by the copies? I suspected a problem due to the FsFile's copy constructor memcopies over the default-constructed representation objects; this is tricky, but apparently not UB: 'If the objects are not TriviallyCopyable (e.g. scalars, arrays, C-compatible structs), the behavior is undefined unless the program does not depend on the effects of the destructor of the target object (which is not run by memcpy) and the lifetime of the target object (which is ended, but not started by memcpy) is started by some other means, such as placement-new.' (AFAICS the rep classes have trivial destructors and use placement-new, so the exclusions indeed apply). I will continue trying to debug this. As a side note, I still think it would be useful and safe to implement move constructors and assignment operators - they'd look very similar to the copy versions except they'd also need to set the rep pointers to nullptr. Same performance, but a more expected move behavior. |
|
I suspect it has something to do with directory state caching. In the line near the end, f1 = f2 causes f1.close(). But f1 still has the original state of the file open for truncation, and probably has some dirty cache state (no longer valid, because it was superseded by changed made via f2), which gets synced by close(), overwriting what was previously written by f2.close(). That would mean that the objects aren't actually cleanly copyable... Perhaps the assignment operator shouldn't actually call close() on the source, but simply reset the rep pointers to nullptr, discarding any unwritten state? |
|
I checked, and indeed, replacing close() with m_fFile = nullptr; in FsFile.cpp:43, does fix my repro. It seems somewhat scary not to call close() from an assignment operator, given that it is called from the destructor, but seeing that all that it does is sync(), wouldn't that be the right thing to do? Or, would that risk leaving the filesystem in an inconsistent state? Then maybe sync() on the source, to flush the cache earlier, in the copy constructor? |
|
Here's a better-motivated repro (closer to what I actually ran into), without copying back-and-forth: Interestingly, changing Test() to removes the problem, at least on ESP32 - because of copy elision. (I verified that the copy constructor is not called at all in that case). But given that copy elision is best-effort before C++17, I suspect this may also have the original issue on some other platforms. Here's what I'd propose:
or
I hope this is helpful. Looking forward to hearing from you! And once again, thank you for writing this awesome library. |
|
I really don't want to more than one open File structure in a program. what does this do? I was not happy when Arduino put open in the SD class and close in the File class of their wrapper. They took an early version of SdFat and the default assignment operator worked.
|
|
You can have a file open with two file descriptors on ESP32 and have scrambled file problems. |
|
I 100% agree that writing to the same file from two separate descriptors, either opened independently or one copied from another, can remain undefined and unsupported. But that's not what I am raising. See in particular my last example: This code looks perfectly reasonable, and I hope you agree it should just work, but it doesn't. I actually don't need or want copies of these file descriptors - I have even suggested that you allow removing the copy constructor via a preprocessor flag! What I would love is to have 'move' instead. Move will have none of these issues. More specifically, what I am trying to do is to compose my code using some additional classes like this: And then use it like that: FsFile file = OpenFileForWrite(...); So really what I'd like is to move the state of the FsFile, and not have any extra copies of it. All that is needed to make this work is adding this move constructor and move assignment operator: Then, there is never a duplication of descriptors - the original gets reset to a pristine state, and its destructor will not call close(). With that, I would not see the need for the copy constructor at all. This change wouldn't break any existing code, and you could recommend it to your users as a cleaner and more modern alternative for passing file descriptors around, compared to relying on the copy semantics (which, as you yourself point out - leads to trouble!) Then you also said that the copy constructor cannot be completely removed today, because it would break too much existing code. Fair enough. That means, though, that all that existing code actually does make copies of file descriptors. Presumably, in many cases these copies are in benign cases like mine above, where people aren't trying to really duplicate anything, but rather, to pass things around: Whoever writes a code like this might not even realize they are making a copy! In fact, what I did was like: which is even more subtle, because the intent to not make a copy is clearly stated in the code (manifested by std::move(f)), but yet - since the move constructor isn't implemented, the code automatically degenerates to calling the copy constructor instead. This all could still be fine and peachy, even without the move constructor, if the copy constructor always worked correctly. One of the scenarios that should work is this: I make a copy, I do not touch the original in any way, I keep working with the copy, and then I let them both go out of scope (causing their destructors to be called), in arbitrary order. But this scenario doesn't work correctly (specifically, when the original is 'dirty' at the time of making the copy) - because the destructor of the original is able to mess up the results made via the copy. That is what I believe is a genuine bug. I believe it can be fixed quite easily by calling sync() on the 'from' file in the copy constructor and the assignment operator. This way you disarm the 'deferred sync' that would be called later when the destructor of the 'from' file gets called. Best, |
|
I only have a vague understanding of move constructors. The cases I know about move a chunk of the heap from one object to another. FsFile does not use the heap. Nothing in SdFat uses the heap. Here is where the FAT or exFAT object is stored in a FsFile object When you open a file with open(path, oflags) this function gets called and constructs the object in the above memory. What does move mean in this case? Is it copy the object and then prevent the destructor of original from doing anything by nulling the two pointers? |
|
TL;DR: Exactly! Long answer: The main point of a move constructor (and a move assignment operator) is that they are free to change, invalidate, and generally mess up, the 'source' object. The only constraint is that it must remain valid enough for the destructor to succeed. This is the main idiom: The main advantage is that they allow passing objects by value (up and down the stack) without calling copy constructors and assignment operators. Common applications indeed use it for objects that keep some stuff allocated on the heap, in order to avoid deep copies. std::string is a good example: In this example, the heap storage for the string is only allocated once, and then passed down the stack to the instance of class X. std::move(s) reset s back to an empty string. For FsBaseFile, the entire state is kept on the stack. Move constructor, and the assignment operator, still needs to fully copy that state, so no difference here. But a big difference is in that move constructor can 'reset' the original object to the 'not-opened' state (by clearing the pointers), effectively 'moving' the state from the original to the copy. Because of that, the destructor later called on the original (when it goes out of scope) will do nothing, because it will see null pointers and conclude that the file isn't open. Before move constructors, similar functionality was often implemented using the 'swap' idiom: Move constructors, plus std::move(), are better because they are supported by the compiler, so that we can pass arguments 'by value' without actually making copies (with help of std::move()). Early C++ had the concept of a 'copyable object' which was one that could be passed by value. Move operators introduced a new concept of a 'movable object' which can still be passed by value (with help of std::move), but it doesn't get 'replicated' in the process. Some classes in the standard C++ library are movable, but non-copyable. A good example is std::unique_ptr, whose purpose is to own an object on the heap and delete it when the smart pointer goes out of scope. Since the smart pointer is movable, you can pass it around by value (with help of std::move()), e.g. move it to some other object so that the ownership is transferred there, but there is always just one valid instance that owns the original raw pointer. I think this is exactly what you'd want for file descriptor structures. If not for the legacy code, it sounds you might have liked the FsBaseFile class to be movable, but non-copyable. Hope that helps! |
|
Passing a file object by value is always a problem because the entire state of the file directory is cached in the file object. So if you read or write the file in a function, the new state is lost. It probably was a mistake to have destructors close the file. I did it since some users didn't understand that when you write a file you must call close. It didn't help since most users follow my example and make File a global object. The problem now is that if two copies exist, close can be called in the wrong order by the destructors. This causes the an incorrect directory entry to be written to the SD. I do have a macro to prevent the destructor from calling close but it is only in FatFile and ExFatFile, not in FsFile. I think I will provide a move constructor for file classes and add a macro to enable the copy constructor. I will then post a beta with the default no copy constructor. I am tempted to make the default no close in the destructor. If the copy constructor is enabled I will put a sync call in the copy constructor. If that causes problems I will change the defaults. I will experiment before making a final decision on the defaults. |
|
I have been playing with constructors and destructors. The move assignment operator looks great for return values.
This is not possible because the source argument can't be const to call sync() but const is required for return. At least for AVR. I always start with the AVR compiler since it has the least features. I didn't like a copy constructor that modified the object anyway. Examples I have tried seem to work with the copy constructor and copy assignment operator set to delete. You can't use a function like this if the copy constructor is deleted: This is O.K.
or
|
|
Sounds great! Re: constness of the source, there might be a reasonably clean way to work around this by declaring m_fileMem as 'mutable' - this tells the compiler that the member can be changed by methods of the class even if the object is const. (It is typically used for things like mutexes). It is defensible on the grounds that cache flush doesn't change the state of the object and logically makes sense even for a const object. But if you think you can just get rid of the copy constructor, I think it'd be much cleaner. |
|
One of the main reasons I found the move constructor and assignment operator interesting was to make the copy constructor optional. I thought I could use std::move() to replace a few assign statements in SdFat. Typically this is used in a loop to walk a path where the file opened becomes the directory in the next iteration. On testing I found std::move() is not available in AVR board packages. AVR is the most popular board. Notice how many AVR packages I have tested with for SdFat issues recently:
People send me boards so I can test. I have a large bookcase with plastic shoe boxes of boards. Paul J Stoffregen of Teensy set me multiple copies of every board and most shields that he sells. Sparkfun sent me a few with unusual processors. So std::move() is useless in libraries since it can't be used with many boards. I can still use move assignment for return from open() for boards I tested, since AVR gcc works, but there is no include with std::move(). I am now replacing any file assignment statements in SdFat with this move function,
becomes
I need to have several macros to provide options for copy/move constructors and assignment operators and test a lot of examples on many boards before I post a beta. |
|
That's unfortunate. My apologies; Espressif boards are my weapon of choice, and I have not much experience with AVR. I keep forgetting how restricted that environment is... But here's a question: does it not support rvalue references (&&) in the language at all, or does it just not support std::move (since it generally doesn't support standard library)? If the latter, then maybe there is still hope, because the implementation of std::move is fairly trivial; it is basically a glorified cast. UPDATE; TL;DR: this version of move() should just work: FsFile&& move(FsFile& f) { return (FsFile&&)f; } |
|
Here's a good discussion if you care about details: https://stackoverflow.com/questions/7510182/how-does-stdmove-convert-expressions-to-rvalues But TL;DR: is that if you want to be fancy, declare two functions (for each file type): FsFile&& move(FsFile& f) { return (FsFile&&)f; } the first one works for cases like FsFile f; and the second for cases like foo(move(CreateFile())) although in the second case, move() is unnecessary to begin with (since the value of CreateFile() is already an rvalue (unnamed) reference, so I guess you wouldn't even really need the second variant. Move constructor and assignment should then just work, I think: FsFile f2 = move(f1); etc. |
|
I don't think I will try to provide workaround functions for board support packages that don't have std::move. I just implemented what I needed to eliminate file assignment operators in SdFat. I have a first version of SdFat with move constructor and move assignment operator for file types. I implemented it for the three base file types and it seems to work for derived types. I think I will make the copy constructor and copy assignment operator private by default with an option to make it public or delete. I will make the move constructor and move assignment operator public with a delete option. I am attaching a slightly tested version. You might try it to see if it fails in a big way. The beginning of SdFatConfig.h has these rough macro definitions. The commented ones make it easy to test options. I tested it for a few things on a number of boards with this program. |
|
Everything works on my end. I needed to remove some remaining copy calls, but that's a good thing if you ask me :) By the way, SdFat is about twice as fast at scanning the same large filesystem (and open and look into zip files) as the vanilla ESP32 SD, for a standards-formatted card. I am really happy with the performance. |
|
Thanks for trying the first cut. I need to try other configurations of SdFat. Some people select features that enable 2009 code, like only support for short 8.3 file names. Also some board support packages use even older compilers than the one in the Arduino AVR package. If no C++11, I won't support it. For ESP performance, too bad the ESP SDIO driver is so poor. I tried it and dedicated SPI was faster for most apps. I get over 20 MB/sec read/write on Teensy 4.2 boards with a SDIO driver I wrote. The controller on the Teensy has features that match modern SD cards internal structures and algorithms. This is rare for MCUs. I have been playing with the Raspberry Pi Pico and other RP2040 boards and get even faster than 20 MB/sec with a custom PIO SDIO controller I wrote. Amazing for a chip that costs $0.75 in quantity. The Pico is Just $4.00. |
Hi Greiman,
I have a FAT32 SDHC card which works fine on ESP32 via SPI with the standard SD library, and also works fine on Windows. (I can read and write files, both on the microcontroller and in Windows). But I can't mount it using SdFat. This is the output from SdInfo:
Card type: SDHC
sdSpecVer: 3.00
HighSpeedMode: true
Manufacturer ID: 0X6F
OEM ID: ␃␃
Product: CBADS
Revision: 1.0
Serial number: 0XAA000B19
Manufacturing date: 8/2022
cardSize: 15942.03 MB (MB = 1,000,000 bytes)
flashEraseSize: 128 blocks
eraseSingleBlock: true
dataAfterErase: ones
OCR: 0XC0FF8080
SD Partition Table
part,boot,bgnCHS[3],type,endCHS[3],start,length
1,0X73,0X73,0X20,0X61,0X6E,0X79,0X20,0X6B,1948285285,1701978223
2,0X73,0X74,0X61,0X72,0X74,0XD,0XA,0X0,0,0
3,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0,0
4,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0X0,28049408,441
MBR not valid, assuming Super Floppy format.
volumeBegin failed. Is the card formatted?
(Apparently, Windows 11 formats the card creating two partition entries?)
To make sure, I backed up the contents, re-formatted the card (on Windows 11), and copied the contents over again. It did not help.
Do you have any insights on what might be wrong?
I was looking format to using SdFat, particularly because of the rename functionality...
The text was updated successfully, but these errors were encountered: