Git Product home page Git Product logo

Comments (9)

ivmarkov avatar ivmarkov commented on May 31, 2024

Working on that.

from esp-idf-sys.

rgcottrell avatar rgcottrell commented on May 31, 2024

Instead of checking for tooling on the path, I would suggest adding a new feature, let's say "system", to the existing "pio" and "native" features. If the "system" feature is species, the the installation should be skipped. If the tools are found on the path, then they can be used. If the tools are not present, however, I think it would be better fail immediately.

I know that I often forget to activate the IDF environment before running a build. It would be undesirable and unexpected in that case to try to install a new version of the SDK and tools.

from esp-idf-sys.

ivmarkov avatar ivmarkov commented on May 31, 2024

Instead of checking for tooling on the path, I would suggest adding a new feature, let's say "system", to the existing "pio" and "native" features. If the "system" feature is species, the the installation should be skipped. If the tools are found on the path, then they can be used. If the tools are not present, however, I think it would be better fail immediately.

I know that I often forget to activate the IDF environment before running a build. It would be undesirable and unexpected in that case to try to install a new version of the SDK and tools.

IDF_PATH is not about using and installing your tooling, but about using your esp-idf.

Introducing another feature is not such a great idea for a variety of reasons. However if you really really want to configure it so that external tooling is always expected and never installed automatically, you can set ESP_IDF_INSTALL_LOCATION to frompath and that would achieve the effect you want.

from esp-idf-sys.

rgcottrell avatar rgcottrell commented on May 31, 2024

IDF_PATH is not about using and installing your tooling, but about using your esp-idf.

Introducing another feature is not such a great idea for a variety of reasons. However if you really really want to configure it so that external tooling is always expected and never installed automatically, you can set ESP_IDF_INSTALL_LOCATION to frompath and that would achieve the effect you want.

What is the ESP_IDF_INSTALL_LOCATION parameter? I don't see it documented and a quick search of the crate didn't reveal any references to it.

I understand that adding new features if not necessary is undesirable. However, it feels like the crate is a bit overloaded and is taking on two separate tasks: 1) generating bindings for the SDK, 2) installing and configuring the IDF tools. These two goals seem to be orthogonal to each other.

Maybe if instead of adding a new feature, the pio and native options are made explicitly opt in. In the absence of specifying either feature then an activated toolchain would be expected to be present. This would minimize the surprise of having an entire SDK downloaded and installed unexpectedly.

My main, concern, however, was to add esp-idf-sys as a dependency on a CMake project and that seems to already be a supported use case with a little extra CMake configuration.

from esp-idf-sys.

rgcottrell avatar rgcottrell commented on May 31, 2024

Another use case for not installing the toolchain automatically is to allow projects to be built from within a Docker image that has all of the tools already installed as part of the base image. It would be undesirable and probably counterproductive to attempt to download and install another version of the toolchain. In this case it's probably the responsibility of the container to ensure that everything is properly installed and configured, but I wonder if there is anything else the crate could do to help prevent mistakes.

from esp-idf-sys.

ivmarkov avatar ivmarkov commented on May 31, 2024

IDF_PATH is not about using and installing your tooling, but about using your esp-idf.
Introducing another feature is not such a great idea for a variety of reasons. However if you really really want to configure it so that external tooling is always expected and never installed automatically, you can set ESP_IDF_INSTALL_LOCATION to frompath and that would achieve the effect you want.

What is the ESP_IDF_INSTALL_LOCATION parameter? I don't see it documented and a quick search of the crate didn't reveal any references to it.

Sorry, I really meant ESP_IDF_TOOLS_INSTALL_DIR, where frompath is the new thing introduced by this PR.

I understand that adding new features if not necessary is undesirable. However, it feels like the crate is a bit overloaded and is taking on two separate tasks: 1) generating bindings for the SDK, 2) installing and configuring the IDF tools. These two goals seem to be orthogonal to each other.

It is also 3) building the EDP-IDF and 4) Generating linker flags for the Rust linking process of the binary crate.

Now, I'm all for "single responsibility", "good architecture" and so on CS blah blah (please don't take offense!), but it is not that we have not tried to separate these tasks as much as reasonable. Please study the code (including embuild) and if you have concrete suggestions how to further split/modularize these otherwise closely inter-related tasks, I would gladly return feedback.

Maybe if instead of adding a new feature, the pio and native options are made explicitly opt in. In the absence of specifying either feature then an activated toolchain would be expected to be present. This would minimize the surprise of having an entire SDK downloaded and installed unexpectedly.

I hear you and I feel from purely CS point of view that might be the best option.

Yet, we also have to think for whom we are optimizing the user experience.

If I do what you are suggesting, then average Joe Embedded Hobbyist coming with only initial (if any) Rust experience and zero embedded programming knowledge (let alone ESP-IDF!) will have to type cargo build --features pio instead of just cargo build (that is, unless we can put features in .cargo/config.toml, but even then...). So the thing is - for whom are we optimizing the experience here?

My main, concern, however, was to add esp-idf-sys as a dependency on a CMake project and that seems to already be a supported use case with a little extra CMake configuration.

Indeed. Now - to resonate a bit with your feelings that this crate is doing too many things including "5) support cmake-first bindings-only build", we can try to unify a bit the env variables the crate expects when called from a cmake-first build with those used for cargo-first build. I can't promise 100% unification, but I can give it a shot once I'm back.

from esp-idf-sys.

ivmarkov avatar ivmarkov commented on May 31, 2024

Another use case for not installing the toolchain automatically is to allow projects to be built from within a Docker image that has all of the tools already installed as part of the base image. It would be undesirable and probably counterproductive to attempt to download and install another version of the toolchain. In this case it's probably the responsibility of the container to ensure that everything is properly installed and configured, but I wonder if there is anything else the crate could do to help prevent mistakes.

This use case would of course be supported by this PR. As for preventing mistakes, putting a ESP_IDF_TOOLS_INSTALL_DIR=frompath in a .cargo/config.toml file located in a (grand-)parent dir of the binary crate might help. Not sure how all directory mappings would work with a docker image though.

And then again, aren't we optimizing the user experience of a niche use case and worsening the user experience of the main use case, where the user would just download Rust and Clang (if it is not already installed by the distro) and then let the crate do its job by downloading and doing whatever necessary automatically?

from esp-idf-sys.

rgcottrell avatar rgcottrell commented on May 31, 2024

Sorry, I really meant ESP_IDF_TOOLS_INSTALL_DIR, where frompath is the new thing introduced by this PR.

Okay, that sounds great. We are currently exploring whether we should use a cargo-first approach, a CMake-first approach, or some other build architecture. It sounds like the frompath option will give us a lot of flexibility here.

It is also 3) building the EDP-IDF and 4) Generating linker flags for the Rust linking process of the binary crate.

I see. There's a lot of extra work that a cargo-first build needs to do that a CMake-first build doesn't. Hopefully having these steps won't introduce too much build time overhead if we don't need them, since keeping build times as short as possible would be very valuable.

If I do what you are suggesting, then average Joe Embedded Hobbyist coming with only initial (if any) Rust experience and zero embedded programming knowledge (let alone ESP-IDF!) will have to type cargo build --features pio instead of just cargo build (that is, unless we can put features in .cargo/config.toml, but even then...). So the thing is - for whom are we optimizing the experience here?

That's a good point, but I think you could still optimize the experience the first user experience by setting the default features (such as having pio set in the default features like it is now). Then more advanced users can disable the defaults and configure more advanced options as they get more experience with the crate.

Anyway, I think this is pretty low priority once there's an easy way of configuring the crate to use an already installed ESP-IDF environment.

Indeed. Now - to resonate a bit with your feelings that this crate is doing too many things including "5) support cmake-first bindings-only build", we can try to unify a bit the env variables the crate expects when called from a cmake-first build with those used for cargo-first build. I can't promise 100% unification, but I can give it a shot once I'm back.

Thanks. I think supporting the CMake-first use case will add a significant amount of value to this crate and anything that can help unify the two approaches will only help.

from esp-idf-sys.

rgcottrell avatar rgcottrell commented on May 31, 2024

This use case would of course be supported by this PR. As for preventing mistakes, putting a ESP_IDF_TOOLS_INSTALL_DIR=frompath in a .cargo/config.toml file located in a (grand-)parent dir of the binary crate might help. Not sure how all directory mappings would work with a docker image though.

That's a great idea. I think it's likely that we will want the actual directory to be configurable, but we could do something like setting ESP_IDF_TOOLS_INSTALL_DIR=frompath:RememberToSetTheEspIdToolsInstallDir in .cargo/config.toml for example and then make sure that the environment variable is overriden when building.

What would the behavior be if an invalid path is set? Hopefully it would error out and not fallback to trying to download and install the tools.

from esp-idf-sys.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.