Unify examples and update hal #296

MabezDev · 2023-10-19T13:54:21Z

This PR merges the chip-specific examples into one folder, inside the esp-wifi project. To simplify building, alias' have been added to esp-wifi/.cargo/config.toml per chip to automatically select the right target and enable the chip feature. I've also used git esp-hal to benefit from the recent unification efforts there.

Before this PR:

$ cd examples-$CHIP
$ cargo run --example X --features $CHIP,etc

After this PR:

cd esp-wifi
cargo $CHIP --example X --features etc

For example, cargo esp32c3 --example dhcp --features wifi.

I think this syntax is close to what it was before, and much easier to remember than each specific target name if you just want to run a quick example. It's also important to note, this change doesn't affect downstream users of esp-wifi, just folks running the examples in this repo.

TODO

Update CI
Find a nicer solution for the timer initialization other than cfg(riscv) etc
Update examples.md
Use Xtensa executor within esp-hal instead of generic single core embassy-executor
Use a default time driver for async so it doesn't have to be specified

Unanswered questions

What to do about examples that can't run on certain chips?
- Initially I wondered if we could utilize required-features in cargo but I don't think that will work
- We could just use compiler_error! to error out on certain example/target combos

bjoernQ · 2023-10-20T08:04:31Z

Good idea to use [alias]

I guess using compile_error! if an example isn't suitable for a specific target should be good enough

MabezDev · 2023-10-20T09:01:08Z

cc: @jessebraham I'd like to get your input as to whether this is a good idea or an insane one :D

esp-wifi/Cargo.toml

jessebraham · 2023-10-20T13:47:30Z

I think this looks pretty reasonable! Interesting use of aliases, never thought of doing this before. Possibly a little insane, but when has that ever stopped us before 😁

Find a nicer solution for the timer initialization other than cfg(riscv) etc

Is there any reason not to just use embassy-time-timg0 for every chip in the examples? I recall speaking with @bjoernQ about making esp-wifi more generic about which timers/alarms it accepts, if this is the only limitation maybe something we can look into? I can't recall how feasible this is, if at all.

Other random though, does the rust-toolchain.toml file in the root not cause issues with the RISC-V chips? Should this just be removed?

MabezDev · 2023-10-20T14:03:47Z

Is there any reason not to just use embassy-time-timg0 for every chip in the examples? I recall speaking with @bjoernQ about making esp-wifi more generic about which timers/alarms it accepts, if this is the only limitation maybe something we can look into? I can't recall how feasible this is, if at all.

Good point, we should look at doing that.

Other random though, does the rust-toolchain.toml file in the root not cause issues with the RISC-V chips? Should this just be removed?

I don't think so, esp toolchain has RISCV compiled too, so in theory, it shouldn't make a difference.

jessebraham · 2023-10-20T14:05:23Z

I don't think so, esp toolchain has RISCV compiled too, so in theory, it shouldn't make a difference.

Ahh right I always forget about this detail. Disregard 😁

bjoernQ · 2023-10-23T06:45:51Z

Is there any reason not to just use embassy-time-timg0 for every chip in the examples? I recall speaking with @bjoernQ about making esp-wifi more generic about which timers/alarms it accepts, if this is the only limitation maybe something we can look into? I can't recall how feasible this is, if at all.

We could add a feature to choose the timer used by esp-wifi internally - might be a useful. Probably not really something that fits into this PR though

esp-wifi/src/ble/os_adapter_esp32c3.rs

MabezDev · 2023-10-31T17:49:12Z

This is now ready for a review, the last cfg issue IMO isn't that big of a deal and something we can sort in another PR.

bjoernQ · 2023-11-01T06:28:07Z

Unfortunately, all BLE examples (ble, embassy_ble, coex) crash on ESP32-C3 while they work on main

Exception 'Store/AMO access fault' mepc=0x4005882e, mtval=0x3fc00100
0x4005882e - r_sch_plan_offset_req_hook
    at ??:??
0x3fc00100 - _sidata
    at ??:??
...

They do work fine on ESP32-S3 (their BLE implementations are very similar)

bugadani · 2023-11-01T06:30:46Z

Unfortunately, all BLE examples (ble, embassy_ble, coex) crash on ESP32-C3 while they work on main
Exception 'Store/AMO access fault' mepc=0x4005882e, mtval=0x3fc00100
0x4005882e - r_sch_plan_offset_req_hook
    at ??:??
0x3fc00100 - _sidata
    at ??:??
...
They do work fine on ESP32-S3 (their BLE implementations are very similar)

Is it possible that this comes from the HAL update for some reason and not the rest of this monster?

bjoernQ · 2023-11-01T06:42:16Z

Unfortunately, all BLE examples (ble, embassy_ble, coex) crash on ESP32-C3 while they work on main
Exception 'Store/AMO access fault' mepc=0x4005882e, mtval=0x3fc00100
0x4005882e - r_sch_plan_offset_req_hook
    at ??:??
0x3fc00100 - _sidata
    at ??:??
...
They do work fine on ESP32-S3 (their BLE implementations are very similar)
Is it possible that this comes from the HAL update for some reason and not the rest of this monster?

I'm almost sure it's the HAL update - will be fun to identify the exact commit since the range is huge

MabezDev · 2023-11-01T11:11:51Z

The esp32c6 also works, so I guess it's not RISCV-specific.

esp-wifi/build.rs

bjoernQ · 2023-11-01T11:14:42Z

❯ git bisect bad
d81994552226ca8e69e9d774aef50aa1c0550a31 is the first bad commit
commit d81994552226ca8e69e9d774aef50aa1c0550a31
Author: jneem <[email protected]>
Date:   Fri Sep 29 10:09:50 2023 -0500

    Sleep support for esp32c3 (#795)

    * Make the example work

    * Make GPIO wakeups work

    * Add example

    * revert hello_rgb change

    * Clean up the first hacky version

    * Fix example warnings

    * Add changelog entry

    * Fix example comment

    * Stop skipping rustfmt

 CHANGELOG.md                                     |   1 +
 esp-hal-common/src/gpio.rs                       |  51 +-
 esp-hal-common/src/rtc_cntl/mod.rs               |  12 +-
 esp-hal-common/src/rtc_cntl/rtc/esp32c3_sleep.rs | 893 +++++++++++++++++++++++
 esp-hal-common/src/rtc_cntl/sleep.rs             |  18 +-
 esp-hal-common/src/soc/esp32c3/gpio.rs           |  11 +
 esp32c3-hal/examples/sleep_timer.rs              |  41 ++
 esp32c3-hal/examples/sleep_timer_rtcio.rs        |  58 ++
 8 files changed, 1077 insertions(+), 8 deletions(-)
 create mode 100644 esp-hal-common/src/rtc_cntl/rtc/esp32c3_sleep.rs
 create mode 100644 esp32c3-hal/examples/sleep_timer.rs
 create mode 100644 esp32c3-hal/examples/sleep_timer_rtcio.rs

MabezDev · 2023-11-01T11:21:02Z

❯ git bisect bad
d81994552226ca8e69e9d774aef50aa1c0550a31 is the first bad commit
commit d81994552226ca8e69e9d774aef50aa1c0550a31
Author: jneem <[email protected]>
Date:   Fri Sep 29 10:09:50 2023 -0500

    Sleep support for esp32c3 (#795)

    * Make the example work

    * Make GPIO wakeups work

    * Add example

    * revert hello_rgb change

    * Clean up the first hacky version

    * Fix example warnings

    * Add changelog entry

    * Fix example comment

    * Stop skipping rustfmt

 CHANGELOG.md                                     |   1 +
 esp-hal-common/src/gpio.rs                       |  51 +-
 esp-hal-common/src/rtc_cntl/mod.rs               |  12 +-
 esp-hal-common/src/rtc_cntl/rtc/esp32c3_sleep.rs | 893 +++++++++++++++++++++++
 esp-hal-common/src/rtc_cntl/sleep.rs             |  18 +-
 esp-hal-common/src/soc/esp32c3/gpio.rs           |  11 +
 esp32c3-hal/examples/sleep_timer.rs              |  41 ++
 esp32c3-hal/examples/sleep_timer_rtcio.rs        |  58 ++
 8 files changed, 1077 insertions(+), 8 deletions(-)
 create mode 100644 esp-hal-common/src/rtc_cntl/rtc/esp32c3_sleep.rs
 create mode 100644 esp32c3-hal/examples/sleep_timer.rs
 create mode 100644 esp32c3-hal/examples/sleep_timer_rtcio.rs

I guess its https://github.com/esp-rs/esp-hal/pull/795/files#diff-13b76e7f6d000776921729c895984b5ce03e1657df0c93a36411d63fce98cd45R216, but we need to figure out which "base setting" has messed things up... fun 🙃

bjoernQ · 2023-11-01T11:26:13Z

Yes, it's exactly that .... commenting that makes it work again

bjoernQ · 2023-11-01T12:05:15Z

Yes, it's exactly that .... commenting that makes it work again

I think I know how to make it work ... will create a PR in esp-hal when I verified it

bjoernQ · 2023-11-01T12:51:04Z

Fix in esp-hal: esp-rs/esp-hal#891

jessebraham · 2023-11-01T13:03:22Z

If we can find a couple other small, compatible changes to make in esp-hal, I wouldn't mind doing a patch release of esp-hal-common to make these changes available.

This PR merges the chip specific examples into one folder, inside the esp-wifi project. To simplify building, alias' have been added to `esp-wifi/.cargo/config.toml` per chip to automatically select the right target and enable the chip feature. Add build script detection for invalid features Fix CI, use build instead of check to detect linker errors enable async feature of hal, this breaks because of interrupt definition for systimer upgrade hal rev upgrade hal rev c6 undo binding modifications fix esp now fix esp coex use released hals, update async examples sync example names sync example names Update examples.md Document alias update build alias

MabezDev · 2023-11-01T13:55:50Z

I'm keen to get this merged, so I've gone ahead and patched the c3 hal for now. We can replace it with a new release later.

bjoernQ

Thanks! This should make maintaining the examples much easier

This comment was marked as outdated.

Sign in to view

bugadani reviewed Oct 20, 2023

View reviewed changes

esp-wifi/Cargo.toml Outdated Show resolved Hide resolved

MabezDev force-pushed the simplify-examples branch 14 times, most recently from 0f71d5f to fd174bc Compare October 26, 2023 13:32

bugadani mentioned this pull request Oct 31, 2023

Update esp-hal packages #320

Closed

MabezDev force-pushed the simplify-examples branch from 6a02b4c to 2431c19 Compare October 31, 2023 14:05

bugadani reviewed Oct 31, 2023

View reviewed changes

esp-wifi/src/ble/os_adapter_esp32c3.rs Outdated Show resolved Hide resolved

bugadani mentioned this pull request Oct 31, 2023

Update embassy-net #289

Merged

MabezDev changed the title ~~Unify examples~~ Unify examples and update hal Oct 31, 2023

MabezDev force-pushed the simplify-examples branch from ad70a57 to e1eec8a Compare October 31, 2023 17:39

bugadani mentioned this pull request Oct 31, 2023

Update HAL card-io-ecg/card-io-fw#147

Closed

MabezDev marked this pull request as ready for review October 31, 2023 17:48

MabezDev force-pushed the simplify-examples branch 2 times, most recently from 22a4540 to cf922a0 Compare November 1, 2023 11:06

MabezDev commented Nov 1, 2023

View reviewed changes

esp-wifi/build.rs Show resolved Hide resolved

MabezDev force-pushed the simplify-examples branch 2 times, most recently from dbe4b15 to 51c8abc Compare November 1, 2023 11:57

bjoernQ mentioned this pull request Nov 1, 2023

Make sure BLE and WiFi are not powered down when esp-wifi needs them esp-rs/esp-hal#891

Merged

MabezDev force-pushed the simplify-examples branch from 51c8abc to 22ec789 Compare November 1, 2023 13:55

MabezDev requested a review from bjoernQ November 1, 2023 13:55

MabezDev mentioned this pull request Nov 1, 2023

Add a benchmarking example for blocking and async #328

Merged

bjoernQ approved these changes Nov 1, 2023

View reviewed changes

MabezDev merged commit 7632e74 into esp-rs:main Nov 1, 2023
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify examples and update hal #296

Unify examples and update hal #296

MabezDev commented Oct 19, 2023 •

edited

Loading

This comment was marked as outdated.

bjoernQ commented Oct 20, 2023

MabezDev commented Oct 20, 2023

jessebraham commented Oct 20, 2023 •

edited

Loading

MabezDev commented Oct 20, 2023

jessebraham commented Oct 20, 2023

bjoernQ commented Oct 23, 2023 •

edited

Loading

MabezDev commented Oct 31, 2023

bjoernQ commented Nov 1, 2023

bugadani commented Nov 1, 2023 •

edited

Loading

bjoernQ commented Nov 1, 2023

MabezDev commented Nov 1, 2023

bjoernQ commented Nov 1, 2023

MabezDev commented Nov 1, 2023

bjoernQ commented Nov 1, 2023

bjoernQ commented Nov 1, 2023

bjoernQ commented Nov 1, 2023

jessebraham commented Nov 1, 2023

MabezDev commented Nov 1, 2023

bjoernQ left a comment

Unify examples and update hal #296

Unify examples and update hal #296

Conversation

MabezDev commented Oct 19, 2023 • edited Loading

TODO

Unanswered questions

This comment was marked as outdated.

bjoernQ commented Oct 20, 2023

MabezDev commented Oct 20, 2023

jessebraham commented Oct 20, 2023 • edited Loading

MabezDev commented Oct 20, 2023

jessebraham commented Oct 20, 2023

bjoernQ commented Oct 23, 2023 • edited Loading

MabezDev commented Oct 31, 2023

bjoernQ commented Nov 1, 2023

bugadani commented Nov 1, 2023 • edited Loading

bjoernQ commented Nov 1, 2023

MabezDev commented Nov 1, 2023

bjoernQ commented Nov 1, 2023

MabezDev commented Nov 1, 2023

bjoernQ commented Nov 1, 2023

bjoernQ commented Nov 1, 2023

bjoernQ commented Nov 1, 2023

jessebraham commented Nov 1, 2023

MabezDev commented Nov 1, 2023

bjoernQ left a comment

Choose a reason for hiding this comment

MabezDev commented Oct 19, 2023 •

edited

Loading

jessebraham commented Oct 20, 2023 •

edited

Loading

bjoernQ commented Oct 23, 2023 •

edited

Loading

bugadani commented Nov 1, 2023 •

edited

Loading