owned this note
owned this note
Published
Linked with GitHub
# Espflash Refactoring
- The current structure has resulted from organic growth over the years, but cracks are starting to show and it's becoming increasingly difficult to add new functionality in some cases
- One major issue is the `Flasher` struct, which has become a bit of a dumping grounds for various functions
- This probably could be split up; how we approach this is still unclear to me
- There are various cyclic dependencies between modules which should be resolved
- This makes maintenance and extension more difficult than it needs to be, and is generally just considered bad practice from an architecture viewpoint
- Some types probably need to be shuffled around a bit, as not everything lives where it probably should
- We are leaking some types from dependencies which are not `1.0` as well, which needs to be resolved
- We need to come up with one or more traits which are generic enough to add new image formats in a non-breaking way
- I think for the image format itself this is probably not too bad, APIs which return RAM/ROM segments or something is probably enough to get us there
- SM: Some formats, including esp-idf's and I guess MCU boot requires that all ROM code is sequentially. If we expose segments in this way, can we still guarantee we don't have any gaps? I suppose this might be considered a bug in the image format implementation if this happens?
- This gets tricky with metadata however, e.g. our current app/partition size, etc. metadata may not be application to all formats
- SM: I think there are quite a few solve this: assoc type in a trait, maybe tying the data to the image format enum and probably a few more. I'm not familiar enough with espflash to know what approach is best though
- I'd still like to try and make as many types/functions/etc. private as possible as well
- SM: +1, smallest possible API surface is definitely a goal here
- SM: We should also keep in mind espflash library usage, like in probe-rs, when we're thinking about this refactor
- SM: To me it seems [`FirmwareImage`](https://github.com/esp-rs/espflash/blob/e41be4f5eb6686a514292092902635b91a94597b/espflash/src/elf.rs#L23) trait, is trying to abstract at the wrong level. _All_ input firmware will be elf files, any binary or hex files etc. will be written directly to flash without needing to worry about the format the binary is. I think what we should be abstracting over is the image format, i.e how to take code segments and turn them into the specified format. We might not need a trait here, an enum might be enough.
- This is turning into a bit of a ramble (sorry), but perhaps the way to untangle this is to think about the flow of data, from inputs (cli, elf) to outputs (image format, and eventually flashing)
- `get_flash_image` is _very_ esp-idf specific, detangling this will probably untangle a bunch of other things (and also solves https://github.com/esp-rs/espflash/issues/745)