RFC: Eliminate cargo-espflash with the 3.0.0 release

[jessebraham]

For quite some time now, we have been maintaining a pair of applications, cargo-espflash and espflash, with very similar functionality. However, this adds additional maintenance burden, and also is the reason for some of the more problematic parts of the code base, ie. the cli feature/module.

It should be possible to add Cargo integration to espflash in the same way that we are with cargo-espflash; I don't believe there are any technical limitations preventing us from doing this, but I will obviously need to investigate a bit further. Between this functionality and Cargo Runners, I believe we can provide a similar experience to cargo-espflash.

Additionally, espflash has been downloaded significantly more times than cargo-espflash (even factoring in the downloads as a library), which suggests that it is the more popular of the two applications.

I would like to open a discussion regarding these changes, and determine whether or not this is a feasible (or desirable!) path forward.

[Vollbrecht]

From a user perspective i mostly use espflash directly and i think for newcomers its really hard to tell apart to pick what the should use ( more for people that are newer to rust and cargo).
I think it would be need, to only have one espflash with a good cargo integration.

My 2 cents from a developer standpoint to have only espflash:

PRO:

• only one codebase, so feature parity is given and we don't need to track changes in two places

CONTRA

• espflash itself would depend on something nonstable - from the cargo book itself:

  • Cargo as a library is unstable: the API may change without deprecation
  • versions of the linked Cargo library may be different from the Cargo binary

  so a potential break has a wider affect. In the current form espflash is not constrained by this

[bugadani]

Just chiming in that I don't mind either way. I'm using cargo espflash behind my xtask but that's easy to update and I don't think I absolutely need cargo integration there, either.

TIFO the cargo integration pulls in about a metric buttload of dependencies that I wouldn't mind if espflash didn't contain.

[jessebraham]

TIFO the cargo integration pulls in about a metric buttload of dependencies that I wouldn't mind if espflash didn't contain.

Yeah, this is a concern for me as well. Perhaps we can put cargo integration behind a feature, not sure. Been awhile since I've worked on this code base so will need to get reacquainted before I can say with any level of certainty.

With that said, I think it's important to find a balance; we have limited maintainers for the project, and we do provide pre-rcompiled binaries via our releases, so taking a bit longer to install from source may just need to be a trade-off we need to make.

Thanks for your input!

[Vollbrecht]

Also another foreshadowing, the current behavior of espflash is - if no partition table is given - that it will create one autosized with respect to the elf size. If cargo espflash is used an no is given for std project it will look into the .embuild dir and check if there is one it can use and use it. So no auto sizing feature in that path on default.

This feature parity problem could also surface on other features and we should think what overrule which ?

[sirhcel]

As someone flashing ESP devices only from time to time, I really like cargo-espflash flash. I'm looking at it from the perspective of an alternative to cargo run with probe-run for getting and up-to-date build and running it on a target device. I have not used espflash or cargo-espflash for flashing and preparing devices in production yet.

Starting with this assumption, cargo-espflash felt pretty much like a drop-in replacement for cargo run and I already felt seated when using it for the first time. This is something which made starting with Rust on ESP32 a pleasant experience for me.

I considered cargo-espflash to be just some chrome and spoilers to espflash in that sense: triggering a Cargo build when necessary, picking the right binary for flashing, and dropping me into a serial console. I learned the last days that there is much more to it. I have to admit that I did not study its documentation and made these assumptions because it feels that much like cargo run to me. Just in case a lot of other people are looking at it from my angle, what about making cargo-espflash this thin convenience wrapper and leaving most of the black magic aside?

[SergioGasquez]

Now that esp-rs/esp-idf-sys#264 is merged, we could try to detect if there is a booloader.bin and partition-table.bin in the current folder and if so, use them (including a log informing the user that we are using them). This could replace the feature of cargo-espflash detecting that is an esp-idf-sys project and also help esp-hal users that use a custom bootloader or partition table without having to use the args all the time.

On the downside, people may have a random bootloaders/partition table on the current folder, which may cause issues, but that should not be too common.

Edit: We could also make esp-idf-sys use an arbitrary folder to put those files, say an assets folder and espflash to read from there

[bjoernQ]

Since I personally never used cargo-espflash I'm all fine with letting it go

[kevin-june]

I do appreciatecargo espflash, specifically because it integrates with Cargo. My team uses cargo espflash in our daily development workflow.

The cargo espflash behavior that I find most useful is the --package / -p option and how that integrates with Cargo's --release option. Instead of needing to determine the path to the appropriate binary, we can simply pass a pass appropriate options to cargo espflash and let it do the work. While it is not insurmountable to calculate these paths ourselves, it is convenient to have it done automatically.

An example of commands that I often run is:

cargo +esp-1.73.0.0 espflash flash --target xtensa-esp32s3-espidf "-Zbuild-std=std,panic_abort" --port <PORT> --monitor --package <PACKAGE>

and/or:

cargo +esp-1.73.0.0 espflash flash --target xtensa-esp32s3-espidf "-Zbuild-std=std,panic_abort" --port <PORT> --monitor --package <PACKAGE> --release

(Yes, we are a bit behind on upgrading our Rust toolchain! :) )

We had more difficulty calculating the path to the bootloader and partition table, since this build artifact is placed in a directory with a varying name: target/xtensa-esp32s3-espidf/debug/build/esp-idf-sys-<SOME_SHA>. I would find it ideal if cargo espflash were aware of the paths to these artifacts, too. I could imagine that reasonable default behavior would be to flash the bootloader and partition table that were most recently built, with options to override these via the --bootloader and --partition-table options (and potentially an option to suppress flashing or the smarts to avoid flashing that are considered in #479). Perhaps this behavior has already changed? I haven't checked in some time... Update: we specify the partition table using package metadata.

I hope you find this description of our use case helpful! We appreciate the work that has gone into maintaining both of these tools.

[jnross]

Let me add a little detail to @kevin-june's comment about why we rely on cargo-espflash to select the right partition table and bootloader binaries:

Our project relies on cargo-espflash to select the partition table and bootloader from the correct esp-idf-sys build. We depend on esp-idf-svc and esp-idf-hal which depend on esp-idf-sys with different feature selections. When the build is complete there are multiple esp-idf-sys build artifacts with only opaque-to-a-human differences in their directory names. Since cargo-espflash processes the cargo output, it knows how to select the correct esp-idf-sys and pick the right partition table and bootloader binaries. This is a problem we wouldn't look forward to solving if cargo-espflash were eliminated.

[jessebraham]

Thanks for the additional input. There seems to be enough interest in cargo-espflash to keep it around, so I will just go ahead and close this.

[Vollbrecht]

Thanks for the additional input. There seems to be enough interest in cargo-espflash to keep it around, so I will just go ahead and close this.

Thanks for creating the RFC, just one last thing i want to sprinkle in. I think people would not be opposed of merging it, under the assumption that they don't loose any features they can do with cargo espflash. Though to make sure we don't miss anything on a potential merge, it probably would need lot's of work and feedback rounds.