Skip to content

Move MakeFatImage out of esp.cpp#2269

Merged
Databean merged 1 commit intogoogle:mainfrom
Databean:split_esp
Mar 16, 2026
Merged

Move MakeFatImage out of esp.cpp#2269
Databean merged 1 commit intogoogle:mainfrom
Databean:split_esp

Conversation

@Databean
Copy link
Copy Markdown
Member

Bug: b/492644615

@Databean Databean requested a review from 3405691582 March 14, 2026 02:07
@Databean Databean enabled auto-merge March 14, 2026 02:07
@Databean Databean changed the title Move MakeFatImage out of esp.cc Move MakeFatImage out of esp.cpp Mar 14, 2026
Copy link
Copy Markdown
Collaborator

@3405691582 3405691582 left a comment

Choose a reason for hiding this comment

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

I have some thoughts but since the change is primarily code motion we should revisit them later.

@Databean
Copy link
Copy Markdown
Member Author

No rush on this change, what are your thoughts?

@3405691582
Copy link
Copy Markdown
Collaborator

Specifically: one (easier), we should label the flags to NewfsMsdos and MkfsFat so they'll be shared and read together, and two (harder), we should just not shell out to these commands and see if there's a library that does this for us.

Again: neither necessary for this change if you don't want to, since this is just code motion.

@Databean
Copy link
Copy Markdown
Member Author

Databean commented Mar 16, 2026

From a quick look Elm FatFs seems like a promising library and has already been imported internally as well. I think we can also use it to replace the invocations to mmd and mcopy which are used to place files into the filesystem.

The relevant part for integration is that it expects users to implement a few functions declared in the FatFs headers, e.g. dread:

DRESULT disk_read (
  BYTE pdrv,     /* [IN] Physical drive number */
  BYTE* buff,    /* [OUT] Pointer to the read data buffer */
  LBA_t sector,  /* [IN] Start sector number */
  UINT count     /* [IN] Number of sectros to read */
);

My preference would be an integration point around function pointers and opaque context objects rather than this BYTE pdrv to distinguish between different instances, but it's workable.

I'll file an issue to track this conversion: b/493268636

@Databean Databean added this pull request to the merge queue Mar 16, 2026
Merged via the queue into google:main with commit 1bae68a Mar 16, 2026
26 checks passed
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.

2 participants