Skip to content

extmod/vfs_fat: Allow setting label of partition when creating FAT fs #15495

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

extmod/vfs_fat: Allow setting label of partition when creating FAT fs #15495

wants to merge 4 commits into from

Conversation

tstenvold
Copy link
Contributor

Summary

During a factory reset, a label for the flash drive can be set via MICROPY_HW_FLASH_FS_LABEL. However, it would be convenient to be able to set this value when using vfs.VfsFat.mkfs(p1). This PR allows an optional label argument to vfs.VfsFat.mkfs(p1, "label")

Testing

Tested on a PYBV11 for creating a new label for the mounted flash with no label and with a label

Trade-offs and Alternatives

Slight additional overhead for mkfs(). Incompatible function call for littlefsX.
An alternative, would be to add a vfs.VfsFat.setlabel("label"), I decided against this as it would be an additional function call adding more overhead.

@dpgeorge dpgeorge added the extmod Relates to extmod/ directory in source label Jul 19, 2024
@dpgeorge
Copy link
Member

Thanks for the contribution.

This feature has actually been requested a few times, and was implemented as a separate .label() method in #13470. There I argued for the C-time macro to be added to the rp2 port, instead of a new function.

I actually think a separate function is more useful than adding a keyword argument to mkfs() (as done here). Because what if you want to relabel the filesystem without formatting it? Or maybe that's not a common use case, and that labelling it during formatting is all that's needed? After all, in #13470 the use-case was to label during formatting.

If we did go with a separate .label() method, it could be possible to retrieve the label if no arguments were specified, eg if vfs.label() == "ABC": ....

@tstenvold
Copy link
Contributor Author

This was the trade-off I was also thinking. The microcontroller rarely cares (or should) about the volume label, so it's more a quality of life for when the controller is plugged into a computer, and is likely not changed often.

A separate function to set/get the label is more overhead than I think is required, and getting the label has many edge cases that for when volume labels aren't supported, but yes there isn't a straight forward solution.

@tstenvold
Copy link
Contributor Author

I'm closing this PR because it's not the way forward.

@tstenvold tstenvold closed this Jul 22, 2024
@tstenvold tstenvold deleted the allow-setting-vfs-fat-label-with-mkfs branch July 22, 2024 09:16
@github-actions GitHub Actions
Copy link

Code size report:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
extmod Relates to extmod/ directory in source
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tstenvold @dpgeorge