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

Support FAT12 sectors larger than 512 bytes #30

Merged
merged 3 commits into from Jun 7, 2023

Conversation

zurcher
Copy link
Contributor

@zurcher zurcher commented Jun 6, 2023

In the mkfs function, the FAT12 cluster size of 64 sectors limits FAT12 implementations to 512-byte sectors only.
This change introduces a new table while maintaining the 4084-cluster limit for FAT12 prescribed by fatgen103.

@nathanhi nathanhi self-assigned this Jun 6, 2023
@zurcher
Copy link
Contributor Author

zurcher commented Jun 7, 2023

It seems the default tests from pyfilesystem2 include ~10MB of data, but the default FS size in test_PyFatFS.py is 4MB for FAT12. Added another table entry to allow up to 16MB for FAT12 and updated the test to allocate 15MB for FAT12.

pyfatfs/PyFat.py Outdated
(4084, 1), # disks up to 2 MB, .5k cluster
(8168, 2), # disks up to 4 MB, 1k cluster
(16336, 4), # disks up to 8 MB, 2k cluster
(32672, 8) # disks up to 16 MB, 4k cluster
Copy link
Owner

Choose a reason for hiding this comment

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

Awesome, thank you very much for your contribution as well as fixing the tests!

Do you think it would also make sense to add support for larger combinations such as:

  • 65344/16: 32MB, 8k cluster
  • 130688/32: 64MB/32k cluster
  • 261376/64: 128MB/64k cluster

Maybe even the non-standard 522752/128 (256MB, 64k cluster)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will add those. Originally I stopped the table at 4MB based on this comment from fatgen103.doc: "If your media is larger than 4 MB, do not bother with FAT12." But it should be fine to support the larger sizes.

Copy link
Owner

Choose a reason for hiding this comment

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

Thank you!

@nathanhi nathanhi merged commit f5ca9f5 into nathanhi:master Jun 7, 2023
15 of 16 checks passed
@zurcher zurcher deleted the fat12clusters branch June 7, 2023 21:18
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

2 participants