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

Unify: Remove the chip-specific HAL packages, adapt esp-hal for direct use [1/?] #1196

Merged
merged 24 commits into from
Feb 27, 2024

Conversation

jessebraham
Copy link
Member

@jessebraham jessebraham commented Feb 21, 2024

After months of working towards this, I've finally pulled the trigger and unified the HAL packages (rejoice!).

There is still a fair bit of work left to do here, however I'm opting to leave that for a subsequent PR. Namely, documentation and READMEs need updating, but I'm sure there is a bunch of small stuff that needs fixing/cleaning.

In addition to the direct work on the HAL, I've also pretty much completely rewritten the CI workflow at this point. There is still some work to be done on it, but it's much smaller and simpler than it was before at least.

All examples for a chip can be built using the xtask package:

cargo xtask build-examples esp-hal esp32c6

You can run examples using one of the provided cargo aliases:

cd examples/
cargo +esp esp32 --example=hello_world

I think most of these changes should be pretty straight forward, the bulk of it is just updating the CI and examples. I've made a lot of assumptions here, and just sort of went for it with regards to making decisions, so if you see anything questionable please don't hesitate to let me know, I'm happy to make whatever changes are necessary.

We will need plenty of eyes on this, plus it would be nice to test as many examples as reasonably possible on hardware. Would appreciate some help with that aspect! One thing I'm a little worried about in particular is the default features; I think I have managed to do the same we were doing before, but hard to say at this point 😅

For context, see #446

@jessebraham jessebraham mentioned this pull request Feb 21, 2024
4 tasks
Copy link
Member

@SergioGasquez SergioGasquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making this huge effort of unification! LGTM, just left a few small comments and typos.

examples/src/bin/lp_core_uart.rs Outdated Show resolved Hide resolved
examples/src/bin/pcnt_encoder.rs Outdated Show resolved Hide resolved
examples/src/bin/psram.rs Outdated Show resolved Hide resolved
examples/src/bin/sleep_timer_rtcio.rs Show resolved Hide resolved
examples/src/bin/qspi_flash.rs Outdated Show resolved Hide resolved
@bjoernQ
Copy link
Contributor

bjoernQ commented Feb 22, 2024

The "bluetooth" feature is gone - it was used to reserve memory for bluetooth on ESP32

@bjoernQ
Copy link
Contributor

bjoernQ commented Feb 22, 2024

We used to re-export riscv and xtensa-lx before. riscv is gone but we should do it. It was a constant source of pain before and should be aligned with Xtensa

@bjoernQ

This comment was marked as outdated.

esp-hal/Cargo.toml Outdated Show resolved Hide resolved
esp-hal/Cargo.toml Outdated Show resolved Hide resolved
Co-authored-by: Björn Quentin <bjoernQ@users.noreply.github.com>
@MabezDev
Copy link
Member

Not sure how to solve this, but when running an example on an unsupported chip you get an confusing error, e.g dac on esp32c3:

error[E0432]: unresolved import `esp_hal::dac`
  --> src/bin/dac.rs:17:5
   |
17 |     dac::{DAC1, DAC2},
   |     ^^^ could not find `dac` in `esp_hal`

I hope we can find a nice way to say this isn't supported on that chip.

examples/src/bin/ecc.rs Show resolved Hide resolved
examples/src/bin/hello_rgb.rs Show resolved Hide resolved
examples/src/bin/i2c_display.rs Outdated Show resolved Hide resolved
examples/src/bin/mcpwm.rs Show resolved Hide resolved
examples/src/bin/rmt_tx.rs Outdated Show resolved Hide resolved
examples/src/bin/sha.rs Outdated Show resolved Hide resolved
@jessebraham
Copy link
Member Author

Not sure how to solve this, but when running an example on an unsupported chip you get an confusing error, e.g dac on esp32c3:

error[E0432]: unresolved import `esp_hal::dac`
  --> src/bin/dac.rs:17:5
   |
17 |     dac::{DAC1, DAC2},
   |     ^^^ could not find `dac` in `esp_hal`

I hope we can find a nice way to say this isn't supported on that chip.

Yeah, I don't have a good solution for this unfortunately.

* Fix RMT examples

* Remove logger-init
@jessebraham
Copy link
Member Author

Clippy is going to fail today, we're just going to ignore it for now.

bjoernQ and others added 2 commits February 27, 2024 13:47
* Avoid UART0 and SPI flash pins

* Fix spi_eh1_device_loopback for non-ESP32

* Update examples/src/bin/gpio_interrupt.rs

Co-authored-by: Juraj Sadel <jurajsadel@gmail.com>

---------

Co-authored-by: Juraj Sadel <jurajsadel@gmail.com>
Copy link
Contributor

@bjoernQ bjoernQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - Thanks for all the effort

@jessebraham
Copy link
Member Author

:shipit:

@jessebraham jessebraham added this pull request to the merge queue Feb 27, 2024
Merged via the queue into esp-rs:main with commit 6a663f8 Feb 27, 2024
17 of 18 checks passed
@jessebraham jessebraham deleted the feature/unify-1 branch February 27, 2024 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants