Skip to content
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.

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

Make esp-idf-template The source of Truth w.r.t. Detailed Installation (Was: Cannot compile generated templates:) #237

Open
tsauvajon opened this issue Sep 24, 2024 · 24 comments

Comments

@tsauvajon
Copy link

tsauvajon commented Sep 24, 2024

Bug description

Newly generated templates do not compile, as esp_idf_svc seems to require an incompatible version of esp_idf_hal.

  • Would you like to work on a fix? [yes, sure]

To Reproduce

Steps to reproduce the behavior:

  1. Create a new project with cargo generate esp-rs/esp-idf-template cargo:
✔ 🤷   Which MCU to target? · esp32
✔ 🤷   Configure advanced template options? · true
✔ 🤷   Enable STD support? · true
✔ 🤷   ESP-IDF version (master = UNSTABLE) · v5.2
✔ 🤷   Configure project to use Dev Containers (VS Code and GitHub Codespaces)? · false
✔ 🤷   Configure project to support Wokwi simulation with Wokwi VS Code extension? · false
✔ 🤷   Add CI files for GitHub Action? · true
  1. cargo build
  2. It fails to compile esp-idf-svc:
   Compiling myapp v0.1.0 (/xxx/myapp)
error[E0063]: missing field `rssi_5g_adjustment` in initializer of `esp_idf_hal::sys::wifi_scan_threshold_t`
   --> /Users/xxx/.cargo/registry/src/index.crates.io-6f17d22bba15001f/esp-idf-svc-0.49.1/src/wifi.rs:189:24
    |
189 |             threshold: wifi_scan_threshold_t {
    |                        ^^^^^^^^^^^^^^^^^^^^^ missing `rssi_5g_adjustment`

error[E0063]: missing field `duty_cycle_pos` in initializer of `esp_idf_hal::sys::sdspi_device_config_t`
  --> /Users/xxx/.cargo/registry/src/index.crates.io-6f17d22bba15001f/esp-idf-svc-0.49.1/src/sd/spi.rs:41:29
   |
41 |         let configuration = sdspi_device_config_t {
   |                             ^^^^^^^^^^^^^^^^^^^^^ missing `duty_cycle_pos`

For more information about this error, try `rustc --explain E0063`.
error: could not compile `esp-idf-svc` (lib) due to 2 previous errors

Expected behavior

Generating a new project and building it works without compilation errors

Environment

Additional context

Cargo.lock

[[package]]
name = "esp-idf-svc"
version = "0.46.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6765d2a6fd1e8850f28f80e00997e2456fcc41483836b206dc90df7ceba334ff"

....

[[package]]
name = "esp-idf-sys"
version = "0.33.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0d793b605e8037907f0e6697b0995a355238f065f321dd389fdf866aa9f8d140"

...

[[package]]
name = "esp-idf-hal"
version = "0.41.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ea0ed6e41edf280787a5325aae0c82d39b1fb5ae3199871cd164ae1a650704d6"
@ivmarkov
Copy link
Collaborator

Hm, that's very weird, because the esp-idf-template nightly CI does not have any complaints at all (and one of the build targets it uses is precisely against ESP IDF 5.2): https://github.com/esp-rs/esp-idf-template/actions/runs/11009400680

Are you absolutely sure you are building against ESP IDF 5.2? Are you sure that you don't have an activated ESP IDF in the same shell where you are trying to build?

@tsauvajon
Copy link
Author

tsauvajon commented Sep 24, 2024

Activating ESP-IDF 5.4

I think I'm on 5.4, is that the probable cause of the issue? And how can I downgrade?

@ivmarkov
Copy link
Collaborator

Activating ESP-IDF 5.4

I think I'm on 5.4, is that the probable cause of the issue? And how can I downgrade?

Yes, that's the cause. 5.4 is not even released yet.
From where is this "Activating ESP-IDF 5.4" message even coming from?

@tsauvajon
Copy link
Author

tsauvajon commented Sep 24, 2024

Activating ESP-IDF 5.4

It is coming from https://github.com/espressif/esp-idf 's . ./export.sh.

I guess solving it is just a matter of setting my local clone of esp-idf to v5.2.2: https://github.com/espressif/esp-idf/tags

Thanks for your help, I'll try immediately with v5.2.2

@ivmarkov
Copy link
Collaborator

Using an activated ESP IDF is possible, but is an advanced setting.
If you are coming from the ESP IDF C world, no prob - you are an expert in ESP IDF C anyway. :-)
Just make sure you have a properly activated ESP IDF, with IDF_PATH and friends.

But if you are just starting with ESP-IDF: you don't have to download, configure etc. ESP IDF. esp-idf-sys does this for you, during the build.

In other words, see this note in the esp-rs book: https://docs.esp-rs.org/book/installation/std-requirements.html

@tsauvajon
Copy link
Author

Thanks a lot. I'm indeed just getting started with ESP-IDF: does that mean RISC-V and Xtensa targets most likely doesn't apply to me? I had no idea what they were so I figured I probably needed them, as they were appearing before the std requirements

@ivmarkov
Copy link
Collaborator

ivmarkov commented Sep 24, 2024

Btw if you are using an activated edp idf, make sure the export.sh env vars are also visible by rust analyzer, or else the IDE build will fight with your command-line build in that they'll use different IDFs.

@tsauvajon
Copy link
Author

What does activated mean in that context?

FYI I followed https://docs.esp-rs.org/book/installation/std-requirements.html -> macOS: See [macOS ESP-IDF prerequisites](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/get-started/linux-macos-setup.html#for-macos-users) ->
https://docs.espressif.com/projects/esp-idf/en/latest/esp32/get-started/linux-macos-setup.html#step-2-get-esp-idf which suggests getting latest master of https://github.com/espressif/esp-idf.git . That is how I ended up with v5.4

@ivmarkov
Copy link
Collaborator

Thanks a lot. I'm indeed just getting started with ESP-IDF: does that mean RISC-V and Xtensa targets most likely doesn't apply to me? I had no idea what they were so I figured I probably needed them, as they were appearing before the std requirements

I don't find a single place in the esp-rs Book that suggests that you have to manually clone and activate an ESP IDF. While that's possible and we support it, we don't actively suggest it as this is another variable, and also most Rust users don't want to deal explicitly with ESP IDF anyway.

Would you agree that the Book does not suggest this, or can you point me to the location in the book which suggests that, so that we can erase it?

@tsauvajon
Copy link
Author

Would you agree that the Book does not suggest this, or can you point me to the location in the book which suggests that, so that we can erase it?

I followed https://docs.esp-rs.org/book/installation/std-requirements.html -> macOS: See macOS ESP-IDF prerequisites ->
https://docs.espressif.com/projects/esp-idf/en/latest/esp32/get-started/linux-macos-setup.html#step-2-get-esp-idf which suggests getting latest master of https://github.com/espressif/esp-idf.git

@ivmarkov
Copy link
Collaborator

Would you agree that the Book does not suggest this, or can you point me to the location in the book which suggests that, so that we can erase it?

I followed https://docs.esp-rs.org/book/installation/std-requirements.html -> macOS: See macOS ESP-IDF prerequisites -> https://docs.espressif.com/projects/esp-idf/en/latest/esp32/get-started/linux-macos-setup.html#step-2-get-esp-idf which suggests getting latest master of https://github.com/espressif/esp-idf.git

The link you are referring to ( https://docs.espressif.com/projects/esp-idf/en/latest/esp32/get-started/linux-macos-setup.html#for-macos-users ) is to one specific paragraph in the whole page (part of Step 1), which is asking you to install cmake and ninja. And in fact even this is not necessary.

@SergioGasquez @Vollbrecht can we remove this link in the STD book and just copy paste explicitly the tools one needs to install?

@ivmarkov
Copy link
Collaborator

@SergioGasquez I have an even better proposal: how about we just link - from the STD book - to the README.md of esp-idf-template itself? It has detailed installation instructions which are always up to date.

This way we have a single place to worry about w.r.t. installations and hopefully less users being confused.

@Vollbrecht
Copy link
Collaborator

Vollbrecht commented Sep 24, 2024

Circular dependency inside the esp-idf-template README here 😂 Quote "For detailed instructions see Setting Up a Development Environment chapter of The Rust on ESP Book. "

Yeah i think the prerequest tools were added before i know that everything essentially is installed via python installer anyway.

@ivmarkov
Copy link
Collaborator

Circular dependency inside the esp-idf-template README here 😂 Quote "For detailed instructions see Setting Up a Development Environment chapter of The Rust on ESP Book. "

Yeah i think the prerequest tools were added before i know that everything essentially is installed via python installer anyway.

I know but we'll remove the "For detailed instructions" too. I think esp-idf-template should be The Source of Truth "for detailed instructions", not the Book. In my mind, the Book should contain material which is more conceptual and thus is aging better without constant changes.

@tsauvajon
Copy link
Author

FYI it was not clear at all to me that it was either:

  • follow the ESP Rust book instructions
  • follow the ESP-IDF template README instructions

I thought I had to follow all the ESP Rust book instructions, and then the template was plugged onto it using my existing installation. I had no idea I was able to follow only the template README and make it work

@ivmarkov ivmarkov changed the title Cannot compile generated templates: Make esp-idf-template The source of Truth w.r.t. Detailed Installation (Was: Cannot compile generated templates:) Sep 24, 2024
@ivmarkov
Copy link
Collaborator

ivmarkov commented Sep 24, 2024

OK, let's keep this issue open (I renamed it appropriately) and track here

  • the cleanup of the book in that it should refer to the template for "detailed installation instructions"
  • the cleanup of the template where any referral to the Book for "detailed installation instructions" should be removed

@tsauvajon
Copy link
Author

Using an activated ESP IDF is possible, but is an advanced setting. If you are coming from the ESP IDF C world, no prob - you are an expert in ESP IDF C anyway. :-) Just make sure you have a properly activated ESP IDF, with IDF_PATH and friends.

But if you are just starting with ESP-IDF: you don't have to download, configure etc. ESP IDF. esp-idf-sys does this for you, during the build.

In other words, see this note in the esp-rs book: https://docs.esp-rs.org/book/installation/std-requirements.html

Doesn't that section of the README suggest to use an activated ESP IDF too?

@ivmarkov
Copy link
Collaborator

Using an activated ESP IDF is possible, but is an advanced setting. If you are coming from the ESP IDF C world, no prob - you are an expert in ESP IDF C anyway. :-) Just make sure you have a properly activated ESP IDF, with IDF_PATH and friends.
But if you are just starting with ESP-IDF: you don't have to download, configure etc. ESP IDF. esp-idf-sys does this for you, during the build.
In other words, see this note in the esp-rs book: https://docs.esp-rs.org/book/installation/std-requirements.html

Doesn't that section of the README suggest to use an activated ESP IDF too?

How?
It talks about installing Rust and Clang toolchains, where you need a small portion of the latter so that the unsafe Rust bindings to ESP IDF can be generated and that's all.

ESP IDF uses GCC. And this section does not even mention ESP IDF.

@Vollbrecht
Copy link
Collaborator

the "export-esp.sh" here is not the same "export.sh" from esp-idf and is used for espup specifics around the xtensa rust toolchain.

In other words it exports this

export PATH="~/.rustup/toolchains/esp/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/bin:$PATH"
export LIBCLANG_PATH="~/.rustup/toolchains/esp/xtensa-esp32-elf-clang/esp-16.0.4-20231113/esp-clang/lib"

so its not related here

@ivmarkov
Copy link
Collaborator

You are confusing esp idfs export.sh script with the unfortunately similarly named export-esp.sh script which is not even necessary on anything but Windows.

@ivmarkov
Copy link
Collaborator

the "export-esp.sh" here is not the same "export.sh" from esp-idf and is used for espup specifics around the xtensa rust toolchain.

In other words it exports this

export PATH="~/.rustup/toolchains/esp/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/bin:$PATH"
export LIBCLANG_PATH="~/.rustup/toolchains/esp/xtensa-esp32-elf-clang/esp-16.0.4-20231113/esp-clang/lib"

so its not related here

And not to mention that this script modifying the path variable is completely unnecessary for esp-idf-* but - alas - still necessary for baremetal which uses the gcc linker. :-(

@SergioGasquez
Copy link
Member

@SergioGasquez I have an even better proposal: how about we just link - from the STD book - to the README.md of esp-idf-template itself? It has detailed installation instructions which are always up to date.

This way we have a single place to worry about w.r.t. installations and hopefully less users being confused.

What about making the book(rust on esp book) the source of truth and then point to it from esp-idf-template and std-training book/material? I think the book is the initial place for most of our users

@ivmarkov
Copy link
Collaborator

@SergioGasquez I have an even better proposal: how about we just link - from the STD book - to the README.md of esp-idf-template itself? It has detailed installation instructions which are always up to date.
This way we have a single place to worry about w.r.t. installations and hopefully less users being confused.

What about making the book(rust on esp book) the source of truth and then point to it from esp-idf-template and std-training book/material? I think the book is the initial place for most of our users

I'm not sure for the very same reasons I outlined - in my mind, a "book" needs to be a tad more conceptual.

Also, I'm still hoping that in future we'll have all esp-idf-* crates (including esp-idf-template) under a single GIT repo (just like baremetal did).

This means, any change to the build process of esp-idf-sys would be possible to immediately reflect/document in the template as well, because that would be - ideally - even a single commit.

The book on the other hand is a separate repo. If you wish, we can also discuss where to go on the forthcoming meeting in Oct.

@SergioGasquez
Copy link
Member

The book on the other hand is a separate repo. If you wish, we can also discuss where to go on the forthcoming meeting in Oct.

Yes, let's discuss this on the community meeting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

4 participants