esp-rs / esp-idf-template

A "Hello, world!" template of a Rust binary crate for the ESP-IDF framework.
Apache License 2.0
436 stars 52 forks source link

Support building for native targets #168

Closed xobs closed 4 months ago

xobs commented 1 year ago

Motivations

The default project builds for a special version of Rust with a set of special packages to support Espressif targets. These packages mostly wrap esp-idf, but also include features such as linking using a special linker.

While this supports targeting embedded projects well, it does not support running software on the user's host machine. For example, a simple "Hello, world!" program fails to compile when built for the user's target system.

Projects should be cross-compatible, and should work both with Espressif's custom toolchains as well as standard Rust toolchains. Without supporting both native- and cross-builds, Rust projects that target Espressif chips will never be compatible with anything else, making them less attractive.

Solution

Patches can be added to move the dependency on esp-idf-svc to only targets that are target_os = "espidf". Additionally, the main loop can be modified to gate patch installation behind #[cfg(target_os = "espidf")].

A more complex issue is that embuild needs to explicitly have the espidf feature enabled. Alternately, the sysenv path could be exposed even on non-ESP targets, because code cannot be turned off in build.rs with #[cfg()] macros.

Alternatives

An alternative approach would be to state that esp-rs projects will never be cross-compatible, and encourage users to build their projects as libraries. Rather than having one frontend that supports all targets, esp-rs projects will only ever target a single chip, and multiple chips require multiple projects. Furthermore, "native" and standard Rust builds require other frontends.

Additional context

It may be desirable to modify embuild as described in "Solution" in order to avoid needing to manually set feature flags inside build-dependencies.

xobs commented 1 year ago

As an example, I can use both stable and esp to build for two different targets with the same codebase:

image

ivmarkov commented 1 year ago

The problem with what you are suggesting is that your code base will be littered with #[cfg(target_os = "espidf")] which should guard those pieces of your code that call into esp-idf-svc. As these APIs are ESP IDF specific.

I wonder if you are not better off by just isolating your cross platform code in a separate library crate which is then consumed by the esp32-specific binary crate? That's what I practice e.g. here.

Otherwise I don't mind I think putting all ESP dependencies behind a target_os = "espidf" gate in Cargo.toml and wherever else necessary (i.e. build.rs). But I need to see a little bit the delta of your changes, so I would appreciate indeed if you can drive a PR.

ivmarkov commented 1 year ago

Some small corrections / clarifications in your assessment of the status quo wihich might be helpful to you when driving the PR:

The default project builds for a special version of Rust

Nothing special in the version of Rust, aside from it supporting the xtensa targets which are not merged in upstream Rust and LLVM yet. If you are using the newer riscv Espressif MCUs, just use upstream clang and nightly Rust, and you should be fine.

with a set of special packages to support Espressif targets. These packages mostly wrap esp-idf, but also include features such as linking using a special linker.

The special linker (ldproxy) is only proxying GCC. I won't mention why it is necessary (too long topic), but ldproxy is only used for Espressif targets - exactly as you actually want I guess? Look at .cargo/config.toml.

While this supports targeting embedded projects well, it does not support running software on the user's host machine. For example, a simple "Hello, world!" program fails to compile when built for the user's target system.

Projects should be cross-compatible, and should work both with Espressif's custom toolchains as well as standard Rust toolchains. Without supporting both native- and cross-builds, Rust projects that target Espressif chips will never be compatible with anything else, making them less attractive.

The problem when coding for an MCU is that you need a lot of additional stuff beyond what is available in the Rust standard libraries, and which happens to be MCU specific. Sure, there is embedded-hal and also my embedded-svc efforts, but unless you have experience in the embedded Rust echosystem (and Rust itself), abstracting over the MCU is anything but easy.

...which does not mean we cannot / should not try to do the esp-idf-template-generated project cross platform, if that's a small effort.

Patches can be added to move the dependency on esp-idf-svc to only targets that are target_os = "espidf". Additionally, the main loop can be modified to gate patch installation behind #[cfg(target_os = "espidf")].

Indeed.

A more complex issue is that embuild needs to explicitly have the espidf feature enabled.

Big deal. You can continue depending on embuild - including with the espidf feature enabled - even for your cross-platform builds. Nothing wrong will happen because of that.

Alternately, the sysenv path could be exposed even on non-ESP targets, because code cannot be turned off in build.rs with #[cfg()] macros.

Easiest is to just look at the value of CARGO_BUILD_TARGET and if it is not a target ending on -espidf skip the call to embuild::espidf::*. In build.rs, that is.

xobs commented 1 year ago

Admittedly, part of the issue was due to me misreading things. I thought the os field was espressif, and was keying off of invalid cfg strings. I spent a lot of time trying to figure out where --ldproxy-linker was set, as that's what the build was complaining about, and couldn't find it anywhere. As soon as I got the correct os value to key off of, things started working much better.

The problem when coding for an MCU is that you need a lot of additional stuff beyond what is available in the Rust standard libraries, and which happens to be MCU specific.

I think that most of what I'd need can be made common between desktop and embedded. Perhaps some explanation is in order. I'm working on a Rust port of the Blackmagic project. Previously I used esp-idf, but the web build system uses node and python, both of which are in various stages of rot. I'm looking to move to Rust because of its stability, and I think I can make it work.

In terms of MCU-specific features, I can think of:

The majority of the project will be the same for both -- namely the web terminal interface, handling the debugging requests, the serial interface, and all of the UI stuff. Websockets and the web server would also be common, unless I decide to reuse those present in esp-idf (which is what I'm doing now).

Having said that, your approach seems very viable, and embodies the "Alternatives" approach.

Easiest is to just look at the value of CARGO_BUILD_TARGET and if it is not a target ending on -espidf skip the call to embuild::espidf::*. In build.rs, that is.

Unfortunately that doesn't work because you can only check for environment variables through normal calls in build.rs -- you can't do fancy preprocessor macro stuff:

[7:59:15 pm] E:/r> more .\build.rs
fn main() {
    if let Ok("espidf") = std::env::var("CARGO_CFG_TARGET_OS").as_deref() {
        embuild::espidf::sysenv::output();
    }
}
[10:30:13 pm] E:/r> cargo +stable run
   Compiling windows-sys v0.48.0
   Compiling filetime v0.2.22
   Compiling embuild v0.31.4
   Compiling farpatch-rs v0.1.0 (E:\r)
error[E0433]: failed to resolve: could not find `espidf` in `embuild`
 --> build.rs:3:18
  |
3 |         embuild::espidf::sysenv::output();
  |                  ^^^^^^ could not find `espidf` in `embuild`

For more information about this error, try `rustc --explain E0433`.
error: could not compile `farpatch-rs` (build script) due to previous error
[10:30:18 pm] E:/r>

The workaround is to modify Cargo.toml:

[build-dependencies]
embuild = { version = "0.31.3", features = ["espidf"] }
ivmarkov commented 1 year ago

Just two small nits:

fn main() { if let Ok("espidf") = std::env::var("CARGO_CFG_TARGET_OS").as_deref() { embuild::espidf::sysenv::output(); } }

The above call is wrong, hopefully you realize that. You have to check that the build target ends with -espidf, NOT that the build target is = espidf (which it isn't).

Scratch that. What you suggest is even better, I admit! :)

The workaround is to modify Cargo.toml:

[build-dependencies]
embuild = { version = "0.31.3", features = ["espidf"] }

That's exactly what I was trying to suggest: nothing wrong with the fact that (a) embuild would be enabled even for native targets and (b) its espidf feature would be enabled for native targets.

xobs commented 1 year ago

That's exactly what I was trying to suggest: nothing wrong with the fact that (a) embuild would be enabled even for native targets and (b) its espidf feature would be enabled for native targets.

Would it make sense to enable the espidf features by default? Or what are the drawbacks of doing so?

My modified main looks like this:

fn main() {
    // It is necessary to call this function once. Otherwise some patches to the runtime
    // implemented by esp-idf-sys might not link properly. See https://github.com/esp-rs/esp-idf-template/issues/71
    #[cfg(target_os = "espidf")]
    {
        esp_idf_svc::sys::link_patches();

        // Bind the log crate to the ESP Logging facilities
        esp_idf_svc::log::EspLogger::initialize_default();
    }
    #[cfg(not(target_os = "espidf"))]
    simple_logger::SimpleLogger::new().env().init().unwrap();

    log::info!("Hello, world!");
}

Which is nice in that it works on both targets. You're right in that a lot of stuff will go into that guarded section, but that seems no more different from what e.g. libstd does with conditional mod statements for various sys crates.

ivmarkov commented 1 year ago

That's exactly what I was trying to suggest: nothing wrong with the fact that (a) embuild would be enabled even for native targets and (b) its espidf feature would be enabled for native targets.

Would it make sense to enable the espidf features by default? Or what are the drawbacks of doing so?

Yes, why not? The espidf feature is "inert" in that if you don't use it, it won't do anything. The crate that uses it is esp-idf-sys, to build itself. Since you don't (in)directly depend on esp-idf-sys for native targets, it will be simply discarded by the compiler.

@N3xed did it as a feature (that one and a bunch of others) only to get faster builds for the cases where embuild is used without these features.

My modified main looks like this:

fn main() {
    // It is necessary to call this function once. Otherwise some patches to the runtime
    // implemented by esp-idf-sys might not link properly. See https://github.com/esp-rs/esp-idf-template/issues/71
    #[cfg(target_os = "espidf")]
    {
        esp_idf_svc::sys::link_patches();

        // Bind the log crate to the ESP Logging facilities
        esp_idf_svc::log::EspLogger::initialize_default();
    }
    #[cfg(not(target_os = "espidf"))]
    simple_logger::SimpleLogger::new().env().init().unwrap();

    log::info!("Hello, world!");
}

Which is nice in that it works on both targets. You're right in that a lot of stuff will go into that guarded section, but that seems no more different from what e.g. libstd does with conditional mod statements for various sys crates.

Yeaaah... except from the point of view of the libstd users, it is a one single API. They don't see the cfg ugliness which is the libstd actual implementation.

But then, I agree - this is not black and white. Using trait-based abstractions (e-hal and e-svc) is not all roses either, as it tends to lift the complexity of the code significantly.

Vollbrecht commented 4 months ago

Closing this as the discussion reached an end point, feel free to reopen if you think there is still stuff related to this template to discuss.

ivmarkov commented 4 months ago

Also and BTW the use case described here (support building for native targets) IS supported after all. You just need to cfg-out all esp-idf specific APIs in your crate when building for the host. And touch the build.rs file to only spit out esp idf linker args if you are building for the esp idf and not the host.

Mentioning because one of my private binary crates has precisely such a hybrid configuration now. Which allows for very easy host-based running for manual testing.

xobs commented 4 months ago

Great! Happy to hear it's now documented. Thanks for the help.

ivmarkov commented 4 months ago

Great! Happy to hear it's now documented. Thanks for the help.

No it is not documented, sorry if I confused you. But is completely possible, by just following the steps described in this thread. I.e.

That's all there is to it!

xobs commented 4 months ago

The issue was originally created as a suggestion for incorporating it into the default template.

I'm guessing it's been decided that Espressif doesn't want to do this, which is why this issue was closed. In which case this thread will serve as documentation to anyone in the future who wants to do that after they have the template project cloned locally.

ivmarkov commented 4 months ago

Espressif does not have any strong feelings on that as the esp-idf-* crates are a community-driven effort (for now).

So it is between you, me and @Vollbrecht to decide.

We certainly can incorporate that in the template (perhaps, under the "advanced" settings) though I wonder if even this is necessary?

With all due respect, this use case is a very niche one:

But if you still want to do it... just open a PR? As long as this is not what is generated by default (i.e., it is triggered only when the user selects "advanced configuration", and then selects an option which says something like "Support cross-compilation for the host target") y/n (with default = n), I think we can merge that.

xobs commented 4 months ago

Got it.

I appreciate that it's a niche usecase, given how most users will use this repo. In that case, I think this Issue (and the helpful discussion that follows!) serves as enough documentation for people to find a workable solution.

ivmarkov commented 4 months ago

Fwiw, here's an excerpt from my (private / commercial) binary crate that has such a hybrid setup:

Cargo.toml

...
[target.'cfg(target_os = "espidf")'.dependencies]
esp-idf-svc = { version = "0.48", features = ["embassy-sync", "embassy-time-driver", "critical-section"] }
esp-idf-matter = { git = "https://github.com/ivmarkov/esp-idf-matter", branch = "mdns" }

[target.'cfg(not(target_os = "espidf"))'.dependencies]
futures-lite = "1"
libc = "0.2"
rs-matter = { version = "0.1", default-features = false, features = ["os"] }
rs-matter-stack = { git = "https://github.com/ivmarkov/rs-matter-stack", branch = "mdns", features = ["os", "nix", "edge-mdns"] }
materialize = { git = "https://github.com/ivmarkov/materialize", features = ["std", "edge-mqtt"] }
async-compat = "0.2"
env_logger = "0.11"

[build-dependencies]
embuild = { version = "0.31.3", features = ["espidf"] }

build.rs

fn main() {
    let os = std::env::var_os("CARGO_CFG_TARGET_OS");

    if matches!(
        os.as_deref().and_then(std::ffi::OsStr::to_str),
        Some("espidf")
    ) {
        embuild::espidf::sysenv::output();
    }
}

main.rs

...
mod led;
mod matter;
mod mdns;
mod mqtt;
#[cfg(target_os = "espidf")]
#[path = "runesp.rs"]
mod run;
#[cfg(not(target_os = "espidf"))]
#[path = "runhost.rs"]
mod run;
mod service;
mod spiram;
mod stack;
mod web;

fn main() -> Result<(), StackError> {
    run::initialize();

    info!("Starting...");

    // Run in a higher-prio thread to avoid issues with `async-io` getting
    // confused by the low priority of the ESP IDF main task
    // Also allocate a very large stack (for now) as `rs-matter` futures do occupy quite some space
    let thread = std::thread::Builder::new()
        .stack_size(40 * 1024)
        .spawn(|| {
            run_wrapper()
        })
        .unwrap();

    thread.join().unwrap()
}

#[inline(never)]
#[cold]
fn run_wrapper() -> Result<(), StackError> {
    let result = run::run();

    if let Err(e) = &result {
        error!("LB-Mini aborted execution with error: {:?}", e);
    } else {
        info!("LB-Mini finished execution successfully");
    }

    result
}

.cargo/config.toml

Here I'm cheating a bit as I currently have two different config.toml files because I'm lazy, but a single one should do, like the one below... BUT: this has to be checked and is the biggest risk to coming up with a common cross-build template: (Look at the comments inside though.)

[build]
# If the user wants to build for e.g. the host, she needs to type `cargo build ---target x86_64-unnown-linux-gnu`
# ... or just make it the default target below
# ... or just comment ALL targets below; then cargo would pick up the host one
#target = "x86_64-unknown-linux-gnu"
target = "xtensa-esp32-espidf"
#target = "riscv32imc-esp-espidf"

[target.xtensa-esp32-espidf]
linker = "ldproxy"
rustflags = ["--cfg", "espidf_time64"]

[target.xtensa-esp32s2-espidf]
linker = "ldproxy"
rustflags = ["--cfg", "espidf_time64"]

[target.xtensa-esp32s3-espidf]
linker = "ldproxy"
rustflags = ["--cfg", "espidf_time64"]

[target.riscv32imc-esp-espidf]
linker = "ldproxy"
rustflags = ["--cfg", "espidf_time64"]

[target.riscv32imac-esp-espidf]
linker = "ldproxy"
rustflags = ["--cfg", "espidf_time64"]

[net]
git-fetch-with-cli = true

[env]
ESP_IDF_SDKCONFIG_DEFAULTS = "./sdkconfig.defaults"
ESP_IDF_VERSION = "v5.1.2"

# TODO: This is a bit annoying, as we cannot have it per-target. What that means is that STD would be re-built
# when building against the host target too.
# Maybe not such a big deal?
[unstable]
build-std = ["std", "panic_abort"]