Skip to content

Conversation

@jessebraham
Copy link
Member

@jessebraham jessebraham commented Aug 21, 2023

  • Bumps the versions of embedded-hal, embedded-hal-async, and embedded-hal-nb to 1.0.0-rc.1
  • Adds the embedded-io and embedded-io-async packages
  • Implements the Read and Write traits from embedded-io{-async} for UART and USB Serial JTAG

Needed to use some git dependencies for now, but we'll get that all sorted prior to the next release.

I've done some very quick testing on a couple chips and things seem to look okay so far, but I will continue to test more thoroughly.

Did a bit of cleanup in the drivers as well, so sorry for the noise there.

Closes #727
Closes #735

embedded-hal-nb = { version = "=1.0.0-alpha.3", optional = true }
embedded-hal-1 = { version = "=1.0.0-rc.1", optional = true, package = "embedded-hal" }
embedded-hal-nb = { version = "=1.0.0-rc.1", optional = true }
embedded-io = "0.5.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be optional and only get included if we target eh1 / async?

Copy link
Member Author

Choose a reason for hiding this comment

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

The embedded_io trait implementations are not feature gated currently. Should they be? I didn't see any reason to, they're not really following the 1.0.0-xxx release cycle of the other packages. But I can change this if there's a reason to.

Copy link
Contributor

Choose a reason for hiding this comment

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

well we pull an unused dependency this way 🤷‍♂️

Copy link
Member Author

@jessebraham jessebraham Aug 22, 2023

Choose a reason for hiding this comment

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

embedded-hal and embedded-dma should both be optional too then, right? I'm not really sure where we're drawing the line here. I'm not against feature gating it, I'd just like to come up with actual criteria for this I guess.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think code implementing embedded-hal and embedded-dma is not feature gated.
But embedded-hal-1 is and it's optional because of that.

@jessebraham
Copy link
Member Author

Thanks for the reviews! @MabezDev I believe I have addressed your comments now.

Copy link
Member

@MabezDev MabezDev 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!

@MabezDev MabezDev merged commit 7fce6e3 into esp-rs:main Aug 22, 2023
@jessebraham jessebraham deleted the fixes/eh1 branch August 22, 2023 14:22
playfulFence pushed a commit to playfulFence/esp-hal that referenced this pull request Sep 26, 2023
* Update a bunch of dependencies

* Implement `embedded-io` and `embedded-io-async` traits for USB Serial JTAG

* Implement `embedded-io` and `embedded-io-async` traits for UART

* Fix `embassy_serial` examples

* Update CHANGELOG

* Address review comments
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.

Implement the embedded-io{-async} traits for Uart and UsbSerialJtag drivers Update embedded-hal{-async,-nb} to version 1.0.0-rc.1

3 participants