search

esp-rs / esp-hal

no_std Hardware Abstraction Layers for ESP32 microcontrollers
https://docs.esp-rs.org/esp-hal/
Apache License 2.0
683 stars 189 forks source link

Implement the `embedded-hal-async` Traits once stabilized #70

Closed jessebraham closed 1 year ago

jessebraham commented 2 years ago

I believe it's still too early to begin work on this, but once a stable release (or at the very least a more stable alpha release) is available we can revisit this topic.

In embedded-hal-async@0.2.0-alpha.1 the traits to implement are:

bjoernQ commented 2 years ago

I did that for another HAL and I agree it's too early since there are a lot of moving parts, still. But once it's (more) stable I'm happy to see this implemented

Commented on the wrong issue 😄

bjoernQ commented 2 years ago

I think we could and should do this in a separate crate which will use esp-hal under the hood.

That way

For I2C and SPI we will need interrupt support which we don't have yet. But probably we could implement delay and wait for digital inputs any time since we have support for it

bjoernQ commented 2 years ago

I tried something here: https://github.com/bjoernQ/esp32c3-async-rs-experiment Really just only async gpio input but at least it seems that implementing async HAL in a separate crate is possible and makes sense. The code is terrible but it proves what I wanted to check

jessebraham commented 2 years ago

Cool, thanks for sharing!

Should we just add an esp-hal-async package or something to this repo, or how did you want to go about doing this?

bjoernQ commented 2 years ago

Didn't even think about that - probably a good idea. I'm just not sure if we want one Async-HAL with features for each chip or one Async-HAL for each chip (similar to what we have for esp-hal). The later would be more consistent with esp-hal - really not sure what the best way would be. Also interesting what the progress for #57 and #80 is

MabezDev commented 1 year ago

Working on embedded_hal_async::digital::Wait.

jessebraham commented 1 year ago

digital::Wait implemented in #333.

MabezDev commented 1 year ago

Working on embedded_hal_async::spi::*

MabezDev commented 1 year ago

Async SPI (except spi device) implemented in #385

JurajSadel commented 1 year ago

I'm planning to start working on embedded_hal_async::i2c::I2c soon.

konkers commented 1 year ago

@JurajSadel Any progress here? I'm looking into working on this and would be happy to pick up any work in progress and/or hear any insight.

JurajSadel commented 1 year ago

Hi, @konkers I haven't spent much time on this yet. Feel free to pick it up.

JurajSadel commented 1 year ago

@konkers Do you have any update regarding async i2c?

MabezDev commented 1 year ago

Marking DelayUs as complete, it's implemented for embassy-time via the time driver modules.

konkers commented 1 year ago

@JurajSadel I didn't make too much progress before getting sidetracked. I did some initial digging into the code. The i2c driver doesn't have interrupt support and the current code relies on busy waiting to feed/drain the fifo. The i2c peripheral does not support DMA so the code would need to be refactored into something a bit more state machine like in order to feed/drain the fifo in response the the appropriate fifo interrupts. That's all before adding any of the async support. I'm heading out on vacation next week and may take my esp32 c3 rust board with me to hack on this.

MabezDev commented 1 year ago

Working on SpiDevice with the new eha async release. Added serial::Write to the list too.

JuliDi commented 1 year ago

Marking DelayUs as complete, it's implemented for embassy-time via the time driver modules.

Is there a way to use this with esp-hal-common at the moment? I would like to pass something that implements embedded_hal_async::delay::DelayUs from esp32c3 hal to an async library function. From what I see, this requires embassy-time version 0.1.1, which requires embedded-hal 1.0.0-alpha.10, which is incompatible with esp-hal-common 0.8.

I presume that using both, alpha.9 and alpha.10 at the same time is not an option. But maybe someone know a workaround?

Details

``` error: failed to select a version for `embedded-hal`. ... required by package `esp-hal-common v0.8.0` ... which satisfies dependency `esp-hal-common = "^0.8"` of package `sk6812-esp32c3-example v0.1.0 (/Users/jdickert/Documents/Git/SK6812/examples/esp32c3)` versions that meet the requirements `=1.0.0-alpha.9` are: 1.0.0-alpha.9 all possible versions conflict with previously selected packages. previously selected package `embedded-hal v1.0.0-alpha.10` ... which satisfies dependency `embedded-hal-1 = "=1.0.0-alpha.10"` of package `sk6812-esp32c3-example v0.1.0 (/Users/jdickert/Documents/Git/SK6812/examples/esp32c3)` failed to select a version for `embedded-hal` which could resolve this conflict ```

jessebraham commented 1 year ago

@MabezDev did you ever make any progress with SpiDevice, or did that work get back-burnered?

MabezDev commented 1 year ago

@jessebraham back-burnered, I didn't really make a start so feel free to take over :)

MabezDev commented 1 year ago

I'm working on embedded_hal_async::serial::Write

jessebraham commented 1 year ago

Forgot to comment but I'm working on embedded_hal_async::i2c::I2c

elpiel commented 1 year ago

Hello, I want to raise another issue of the async traits. They've decided not to include an async read crate for UART. This means that there should be some other impl in the hal crates that supports async reads.

Another point that they've mentioned is using embedded-io Read & Write for UART instead.

See this comment for more details: https://github.com/rust-embedded/embedded-hal/pull/349#issuecomment-1499465364

jessebraham commented 1 year ago

Thanks for adding that, we do have a separate tracking issue already for other peripherals: https://github.com/esp-rs/esp-hal/issues/361

I suppose we should identify which peripherals should/can have async drivers and list them there, though. Haven't updated it in awhile 😁

jessebraham commented 1 year ago

Just as a small update, there is talk of removing the SpiDeviceRead and SpiDeviceWrite traits from embedded-hal-async. For this reason, we're holding off on implementing these final traits until a new release is available.