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

Feedback on examples #1432

Open
Firstyear opened this issue Apr 14, 2024 · 13 comments
Open

Feedback on examples #1432

Firstyear opened this issue Apr 14, 2024 · 13 comments
Labels
examples There is a problem with one or more of the examples status:long-term This task will be around a while

Comments

@Firstyear
Copy link

I think that while it's great there are so many examples, the issue is that because they all depend on one large Cargo.toml, it's very difficult to see which parts of the cargo.toml are actually needed to build any feature or part of the example in isolation. It would be better if the examples were broken out in some way so that it was clear which features they require, and which dependencies they rely on.

@MabezDev
Copy link
Member

Regarding dependencies, this is sadly a general Rust problem which every project with examples will run into. We are pushing people towards using https://github.com/esp-rs/esp-template and picking parts from examples across for the time being. We want to improve the template further, but there are currently some limitations with cargo-generate: cargo-generate/cargo-generate#1054.

As for features, each example has its features and supported chip listed at the top level docs. We use this metadata in our xtask subcommand to auto enable the features (instead of having to specify them yourself if using cargo directly).

Thanks for the feedback! In this case I don't think we have anything we can action at this time, but if you some concrete ideas on how to improve our current situation please file another issue :).

@github-project-automation github-project-automation bot moved this from Todo to Done in esp-rs Apr 14, 2024
@Firstyear
Copy link
Author

The problem I think is that going from an example to something standalone is really hard.

Lets take embassy_dhcp as an example. You can't just lift that file up and out to a new project. What do I need in Cargo.toml? There is a maze of features to unpick to work out what's needed because all the examples proxy via a single Cargo.toml. And when you do have all of them, you still get spurious other errors.

I think my "frustration" is that it's hard to isolate what each example actually depends on and requires. it's hard to pick it apart into what you really need.

That's why I think there needs to be a way to split this up, because while it may be convenient for tests, it's not serving as a useful example if I can't take it and action what it contains. An example is after all meant to educate on basic operation of a task and how to create and configure the ecosystem so that example can run. Especially when embassy is so "feature complex".

@Firstyear
Copy link
Author

Additionally it would be great if the cargo-template included an embassy variant too :)

@Firstyear
Copy link
Author

How about this as an idea - there needs to be a "glue" crate.

Currently, a cargo.toml to build something looks like this:

 # cat Cargo.toml
[package]
name = "no-std-template"
version = "0.1.0"
authors = ["William Brown <[email protected]>"]
edition = "2021"
license = "MIT OR Apache-2.0"

[profile.dev]
# Rust debug is too slow.
# For debug builds always builds with some optimization
opt-level = "s"

[profile.release]
codegen-units = 1 # LLVM can perform better optimizations using a single thread
debug = 2
debug-assertions = false
incremental = false
lto = 'fat'
opt-level = 's'
overflow-checks = false


[dependencies]
defmt = { version = "0.3.6", optional = true }
esp-hal = { version = "0.16.1", default-features = false }
smoltcp = { version = "0.11.0", default-features = false, features = [
    "medium-ethernet",
    "socket-raw",
], optional = true }
critical-section = "1.1.1"
log = { version = "0.4.20", optional = true }
embedded-hal = { version = "0.2.4", default-features = false }
embedded-svc = { version = "0.27.0", default-features = false, features = [], optional = true }
enumset = { version = "1.1.3", default-features = false, optional = true }
linked_list_allocator = { version = "0.10.5", default-features = false, features = [
    "const_mut_refs",
] }
embedded-io = { version = "0.6.1", default-features = false }
embedded-io-async = { version = "0.6.0", optional = true }
fugit = "0.3.7"
heapless = { version = "0.8", default-features = false, features = [
    "portable-atomic",
] }
num-derive = { version = "0.4" }
num-traits = { version = "0.2", default-features = false }

no-std-net = { version = "0.6.0", optional = true }

esp-wifi = { version = "0.4.0" }
esp-wifi-sys = { version = "0.3.0" }

embassy-sync = { version = "0.5.0", optional = true }
embassy-futures = { version = "0.1.0", optional = true }
embassy-net-driver = { version = "0.2", optional = true }
toml-cfg = "0.2.0"
libm = "0.2.7"
cfg-if = "1.0.0"
portable-atomic = { version = "1.5", default-features = false }
portable_atomic_enum = { version = "0.3.0", features = ["portable-atomic"] }

futures-util = { version = "0.3.28", default-features = false, features = [
    "portable-atomic",
] }
atomic-waker = { version = "1.1.2", default-features = false, features = [
    "portable-atomic",
] }

# Former dev deps

esp-println = { version = "0.9.0", default-features = false, features = [
    "log",
    "uart"
] }
esp-backtrace = { version = "0.11.0" , features = [
    "panic-handler",
    "exception-handler",
    "println",
] }
embassy-executor = { version = "0.5.0", package = "embassy-executor", features = [
    "nightly",
    "integrated-timers",
] }
embassy-time = { version = "0.3.0" }
embassy-net = { version = "0.4.0", features = [
    "tcp",
    "udp",
    "dhcpv4",
    "medium-ethernet",
] }
bleps = { git = "https://github.com/bjoernQ/bleps", package = "bleps", rev = "9371d7d4d510ba5c936c1eef96674f8fd4f63e8a", features = [
    "macros", "async"
] }
embedded-hal-async = { version = "1.0.0" }
static_cell = { version = "2.0", features = ["nightly"] }

[build-dependencies]
toml-cfg = "0.2.0"

[features]
default = [ "log" ]

# chip features
esp32c2 = [ "esp-hal/esp32c2", "esp-wifi-sys/esp32c2", "esp-println/esp32c2", "esp-backtrace/esp32c2" ]
esp32c3 = [ "esp-hal/esp32c3", "esp-wifi-sys/esp32c3", "esp-println/esp32c3", "esp-backtrace/esp32c3" ]
esp32c6 = [ "esp-hal/esp32c6", "esp-wifi-sys/esp32c6", "esp-println/esp32c6", "esp-backtrace/esp32c6" ]
esp32h2 = [ "esp-hal/esp32h2", "esp-wifi-sys/esp32h2", "esp-println/esp32h2", "esp-backtrace/esp32h2" ]
esp32   = [ "esp-hal/esp32",   "esp-wifi-sys/esp32",   "esp-println/esp32",   "esp-backtrace/esp32" ]
esp32s2 = [ "esp-hal/esp32s2", "esp-wifi-sys/esp32s2", "esp-println/esp32s2", "esp-backtrace/esp32s2" ]
esp32s3 = [ "esp-hal/esp32s3", "esp-wifi-sys/esp32s3", "esp-println/esp32s3", "esp-backtrace/esp32s3" ]

# async features
async = [
  "dep:embassy-sync",
  "dep:embassy-futures",
  "dep:embedded-io-async",
  "esp-hal/embassy",
  "esp-hal/async",
]

embassy-net = ["dep:embassy-net-driver", "async"]

# misc features
coex = []
wifi-logs = []
dump-packets = []
smoltcp = [ "dep:smoltcp" ]
utils = [ "smoltcp" ]
enumset = []
wifi = [ "dep:enumset", "dep:no-std-net" ]
embedded-svc = [ "dep:embedded-svc" ]
ble = [ "esp-hal/bluetooth" ]
phy-enable-usb = []
ps-min-modem = []
ps-max-modem = []
esp-now = [ "wifi" ]
ipv6   = ["wifi", "utils", "smoltcp?/proto-ipv6"]
ipv4   = ["wifi", "utils", "smoltcp?/proto-ipv4"]
tcp    = ["ipv4", "smoltcp?/socket-tcp"]
udp    = ["ipv4", "smoltcp?/socket-udp"]
icmp   = ["ipv4", "smoltcp?/socket-icmp"]
igmp   = ["ipv4", "smoltcp?/proto-igmp"]
dns    = ["udp",  "smoltcp?/proto-dns", "smoltcp?/socket-dns"]
dhcpv4 = ["wifi", "utils", "smoltcp?/proto-dhcpv4", "smoltcp?/socket-dhcpv4"]
wifi-default = ["ipv4", "tcp", "udp", "icmp", "igmp", "dns", "dhcpv4"]
defmt = [
  "dep:defmt",
  "smoltcp?/defmt",
  "esp-hal/defmt",
]
log = [
  "dep:log",
  "esp-hal/log",
]

[package.metadata.docs.rs]
features = ["esp32c3", "wifi", "ble", "coex", "async", "embassy-net", "esp-hal/embassy-time-systick", "esp-hal/default"]
default-target = "riscv32imc-unknown-none-elf"

And I had to copy paste huge parts of this from esp-wifi and it's multiple Cargo.toml files, and it still doesn't work.

The issues are:

  • There are simply so many crates it's impossible to know what version works with what other versions
  • There are so many features you don't know what features are simply needed for any combination
  • The docs are pretty sparse on the examples.
  • The examples that do exist is esp-hal and esp-wifi are so broad in what they show, they have to account for many combinations making it hard to work out "what you really need" in isolation.

Going from cargo-generate to a working build has been impossible for 4 days now. I simply haven't got anything non trivial to work, because I can't work out what's needed in the Cargo.toml.

This is why I think a "glue" crate would be useful.

This could be something like 'esp-glue'. It could have features that describe high level concepts like:

  • board type (esp32c3 in my case but you know the others)
  • wifi
  • ble
  • networking
  • logging
  • embassy
  • and probably more I am not aware of.

This way a consumer's cargo.toml could be:

[dependencies]
esp-glue = { version = 0.1.0, features = [
    "esp32c3",
    "wifi",
    "ble",
    "embassy"
] }

Examples could then be part of the glue crate since it's much easier to include.

This would allow a tokio style library https://docs.rs/tokio/latest/tokio/#feature-flags where you enable what you are using, or could just opt into "full" if you wanted.

It would also allow board selection to be much easier.

Within the glue crate it could exposed tested and coordinated sets of libraries that are known to work together, via things like esp-glue::{wifi, net, embassy} etc. That way a user never has to worry about things like "which version of embassy and embassy net worktogethr with esp-hal, and what features do I need? "

I think this would greatly improve the new user experience as well as existing users.

@MabezDev
Copy link
Member

Thank you for the feedback, it's really valuable!

Something like esp-glue is on our roadmap, but we have some other things, mainly in esp-hal, that we need to work on first.

I think my "frustration" is that it's hard to isolate what each example actually depends on and requires. it's hard to pick it apart into what you really need.

I agree, I've run into this with other projects too. My strategy is to copy all the deps, and get it building, then cut away deps one by one. This is obviously not ideal though. Having something like esp-glue and a more advanced esp-template would be great.

I think as an actionable item here, we can also try to document our deps and what they're needed for in examples/Cargo.toml, as a stop gap solution. So I'll reopen to track this.

@MabezDev MabezDev reopened this Apr 15, 2024
@github-project-automation github-project-automation bot moved this from Done to In Progress in esp-rs Apr 15, 2024
@Firstyear
Copy link
Author

It's pretty likely something like the "glue" crate could also just make it easier in general to upgrade versions too since it defines a working combination of parts.

So I'd be more than happy to test that when you start to work on it (heck, I was tempted to throw together a basic one and try to expand it a bit if I got more parts working).

@Firstyear
Copy link
Author

While this doesn't seem like a bad idea on the surface, I feel this would be a nightmare for us to maintain with the rate that breaking changes are introduced to the embedded ecosystem.

I think a comment was added and then removed, but to address this point because I think it's really important.

We have to look at this as "costs". By having a glue crate, you the maintainers of the project are in control of saying "what parts work together, and what doesn't". Yes, that does take work. Yes it does come with challenges when breaking changes are made to re-asses "what parts do work together?".

But today you already have that cost. You need to pay this cost to keep every example working under all these moving parts. So this "glue" suggestion actually helps you, because instead of having to upgrade every crate and example, you can fix the part moving puzzle in a single location rather than many locations.

In addition, think about how this feels as a consumer of the ecosystem - without the maintainers putting up some of this work (work that you already have to do because of the examples as noted), your consumers now also need to pay this cost. Not only this, they need to do so without the same level of subject matter expertise as you have. There are many smart users of your ecosystem, but they may not have all the insight you have to enable them to resolve which changes work together.

For example , every time I go to update an esp-rs project, I can't just "upgrade" it. I have to create a new project from a new version of your cargo template, and then lift and move bits at a time. Because when I do try and upgrade, everything breaks in subtle and unexpected ways that I simply don't have the knowledge to solve.

So I think while yes, a glue crate is work for you as maintainers, it's work you already have to perform in your projects, and it hugely benefits your users. The more users you have, and the more you empower and enable them to use your libraries, the more they will be able to step up and help and contribute in other ways.

@danclive
Copy link

danclive commented Apr 19, 2024

I've thought about this, and I've just created a repository, esp-examples, to try and add some standalone examples to illustrate the dependency options. I'll add them as soon as I have time.

@Firstyear
Copy link
Author

I'll check it out and try to help out if I can make some more. Thank you so much <3

@MabezDev
Copy link
Member

We've recently merged many dependencies into the esp-hal tree directly. This doesn't solve all the issues you've mentioned, and a glue crate is still something we're thinking about for the future, but we hope it will improve the situation a little bit for now.

@Firstyear
Copy link
Author

Thank you! I did notice this actually, so I'll have to try it later.

@jessebraham jessebraham added the status:needs-attention This should be prioritized label Jul 11, 2024
@tom-borcin tom-borcin added examples There is a problem with one or more of the examples status:long-term This task will be around a while and removed status:needs-attention This should be prioritized labels Jul 15, 2024
@bugadani bugadani mentioned this issue Sep 9, 2024
6 tasks
@jessebraham
Copy link
Member

We had discussed removing the current example and replacing them with standalone projects, in a similar fashion to how things are done in ESP-IDF, to try and make it easier for users to get started from examples.

With esp-generate now having its first release, I would like to discuss our plans on how (or if) we'd like to move forward with this.

@MabezDev not sure if you'd like to discuss this in a meeting or just have the conversation here, but I'll leave that decision to you. Either way, I think our current approach to examples is not sustainable (or particularly useful), demonstrating how to use specific drivers should be relegated to the documentation IMO and we should provide more interesting examples.

@bjoernQ
Copy link
Contributor

bjoernQ commented Nov 8, 2024

From a user's perspective turning the examples into standalone projects would definitely be a good thing.

I guess it would be more effort maintaining them but if we reduce the number of examples (by removing the "not so awesome" ones - maybe turning them into tests if we can) - that's maybe not much of a problem 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples There is a problem with one or more of the examples status:long-term This task will be around a while
Projects
Status: In Progress
Development

No branches or pull requests

6 participants