Skip to content

Conversation

@alexiionescu
Copy link
Contributor

serial uart read for embassy async

Tested on ESP32C3 only.

NOTE: embassy_hal_async does not have a Read trait for serial, only Write traits, so I could not follow the same implementation pattern as in the write case. The new function is called read, not sure if it is Ok, there was not a read function for nb implementation. If you think is to generic, I can change it to read_async

Also fix an issue with async write on large buffers, was panicking with Err Would Block. Discovered when testing reading and echoing back a large reader buffer.

NOTE: this is a reopen of the #615 PR.

Must

  • The code compiles without errors or warnings.
  • All examples work.
  • cargo fmt was run.
  • Your changes were added to the CHANGELOG.md in the proper section.
  • You updated existing examples or added examples (if applicable).
  • Added examples are checked in CI

Nice to have

  • You add a description of your work to this PR.
  • You added proper docs for your newly added features and code.

@MabezDev
Copy link
Member

Could you address the remaining review comments from #615 here please.

@alexiionescu
Copy link
Contributor Author

I did address them in commit 6fd1f34
Also update the PR number in latest commit e38de41

@alexiionescu
Copy link
Contributor Author

Sorry, you are right I lost some code on commit. I will put it back

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.

Thanks for sticking with me during the review process @alexiionescu, this is looking good now!

Could you fix the conflict in the changelog (sorry, I just merged another PR)?

@MabezDev MabezDev merged commit bce7210 into esp-rs:main Jun 26, 2023
@alexiionescu alexiionescu deleted the async_read branch June 26, 2023 15:59
SergioGasquez pushed a commit to SergioGasquez/esp-hal that referenced this pull request Jun 28, 2023
* implement embassy async uart read

* Add embassy async read support for uart

* changes based on review

* fix CI failures

* change review #2

* fixed re-opened PR number

* changes review no.3

---------

Co-authored-by: Scott Mabin <[email protected]>
MabezDev added a commit that referenced this pull request Jul 12, 2023
* implement embassy async uart read

* Add embassy async read support for uart

* changes based on review

* fix CI failures

* change review #2

* fixed re-opened PR number

* changes review no.3

---------

Co-authored-by: Scott Mabin <[email protected]>
playfulFence pushed a commit to playfulFence/esp-hal that referenced this pull request Sep 26, 2023
* implement embassy async uart read

* Add embassy async read support for uart

* changes based on review

* fix CI failures

* change review #2

* fixed re-opened PR number

* changes review no.3

---------

Co-authored-by: Scott Mabin <[email protected]>
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.

2 participants