Consider restructuring the project to simplify adding support for new chips

[jessebraham]

Currently due to the limited functionality of the templating system used by cargo-generate, our template files are quite messy. It's also rather difficult to add support for a new chip, and in general I just feel that from a maintainer's perspective this project still leaves a lot to be desired.

I'm proposing we consider restructuring the project to simplify things for us. I have two possible ideas; both of these have the unfortunate side-effect of requiring a fair bit of duplication, however I feel they are both cleaner and will give us much more flexibility than we currently have.

I will outline my ideas below. It may turn out that sticking with our current approach is the right way forward, however I would at least like to discuss these other options.

cc @SergioGasquez

Per-architecture Templates

One possibility is splitting things up into two subdirectories, riscv and xtensa, much in the same way that esp-idf-template is split into cargo and cmake directories.

This removes a lot of the conditional code from our templates, and we are fairly certain that no new Xtensa devices will be released so we can be reasonably confident that this will not change much. Adding support for new devices will always require modifying the riscv template.

Per-device Templates

Taking this even a step further, we could simply have a directory for each supported device. This results in a lot of duplication, however the templates become much simpler and cleaner as a result and we have maximum flexibility. This also makes it trivial to add support for a new device.

My only concern here is that I'm not sure this level of duplication is worth the benefits.

[bjoernQ]

I think the scripting capabilities are not that bad (however I don't have experience with it ... so maybe I'm wrong)

I see two things where scripting can help a lot

• have a "pre" script which populates variables which are later used in the templates - making decisions based on the input seems much nicer in Rhai (the script could either generate code blocks which just get inserted and/or set derived variables like an "architecture" so that the conditionals in the template look much cleaner - like we do in esp-hal-common where we set cfg values based on the target)
• have a "post" script which deletes / renames files - we could have files for each chip or architecture and the script can remove the unwanted files and rename files (e.g. "main_esp32c3.rs" to "main.rs")

Certainly both can be combined - maybe worth to try that?

[bjoernQ]

Even better: With my upcoming changes for https://github.com/esp-rs/esp-hal/issues/382 the template will get much simpler.

• ESP32-S2 atomic-emulation will be automatically handled by the HAL (like we do for ESP32-C2 / ESP32-C3 already)
• the entry macro is re-exported on the same path regardless of the architecture and will even be included in the prelude
• rt crates should not be added as a dependency in binary crates
• riscv/xtensa-lx are re-exported from the HAL and shouldn't be added as dependencies
• we can alias the concrete HAL as hal - no need to import from differently named crates

[SergioGasquez]

Even better: With my upcoming changes for esp-rs/esp-hal#382 the template will get much simpler.

• ESP32-S2 atomic-emulation will be automatically handled by the HAL (like we do for ESP32-C2 / ESP32-C3 already)
• the entry macro is re-exported on the same path regardless of the architecture and will even be included in the prelude
• rt crates should not be added as a dependency in binary crates
• riscv/xtensa-lx are re-exported from the HAL and shouldn't be added as dependencies
• we can alias the concrete HAL as hal - no need to import from differently named crates

Those are great new! I had already started working on splitting the template, and today I did a bit more of work, here is the current state: https://github.com/esp-rs/esp-template/compare/main...SergioGasquez:esp-template:feature/architectore-refactor. Again, I am more than happy to ditch that work and do it on a better way!

[SergioGasquez]

I do like the approach @bjoernQ suggested (#55), we would end up with a bunch of ifs in some places (like Cargo.toml for the hal versions) but we reduce a lot of ifs and we keep the template in a single folder (instead of having xtensa and riscv templates duplicating a bunch of code)

[bjoernQ]

Maybe all those ifs can even get moved to the pre-hook and then it's mostly just using variables (e.g. the script could populate the HAL package-name and the HAL version)

Maybe that Rhai code can even get written in a nicer way (unfortunately I am not fluent in Rhai script)

[SergioGasquez]

Maybe all those ifs can even get moved to the pre-hook and then it's mostly just using variables (e.g. the script could populate the HAL package-name and the HAL version)

That could improve readability as we would have all the logic inside the rhai script, which is indented, and not mixing two different programming languages. We could even try to get the latest version of each hal published on crates.io and use it instead of hard-coding the version (even though this can be “risky”)

[bjoernQ]

I updated the mentioned PR and I'm more than happy with the result. Adding support for a new chip will only be a matter of editing pre-script.rhai and adding the description of the target like this

...
    esp32c6: #{
        arch: "riscv",
        has_swd: true,
        sys_peripheral: "PCR",
        toolchain: "nightly",
        rust_target: "riscv32imac-unknown-none-elf",
        atomic_emulation: "none",
        gcc_target: "riscv32-esp-elf",
    },
...

Plus adding the option to the [placeholders.mcu] obviously

Also, that PR is now almost in a mergeable state (just waiting for the next HAL release)

We could even try to get the latest version of each hal published on crates.io and use it instead of hard-coding the version (even though this can be “risky”)

I think that is indeed too risky since at least in the forseeable future we will introduce breaking changes. Once we are at 1.x.x versions that however might be a good idea - in the meantime probably something like #53 is the better option

[SergioGasquez]

Closed by #55