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

Async read implementation for usb serial jtag #889

Merged
merged 8 commits into from
Nov 17, 2023

Conversation

jneem
Copy link
Contributor

@jneem jneem commented Oct 31, 2023

This is an attempt at adding an async read implementation for usb_serial_jtag to complement the existing async write implementation. I'm pretty unsure about whether I'm doing the interrupt handling correctly, but I did at least check that a basic implementation works.

  • 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.

@jessebraham
Copy link
Member

Thanks for the PR, apologies for taking so long to get to this! I will try to get this tested and reviewed in the next couple days.

@jessebraham
Copy link
Member

jessebraham commented Nov 15, 2023

I needed to rebase this branch in order to get it building, however after doing so it seems to be working as expected!

So, if you wouldn't mind rebasing (there is unfortunately a conflict with CHANGELOG.md, anyway) then I think this is probably good to go!

Additionally, if you're willing to add examples for the other chips that would be great, but I can do so in a subsequent PR so it's not huge deal.

@jneem
Copy link
Contributor Author

jneem commented Nov 17, 2023

Ok, I've added examples for all c6, h2, and s3 (basically all the ones that already had a non-embassy usb_serial_jtag example). They're untested, though, because I don't have any of them.

I also took the liberty of removing the scary comment from s3's usb_serial_jtag example because that issue is fixed, apparently.

Copy link
Member

@jessebraham jessebraham 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 again for this!

@jessebraham jessebraham added this pull request to the merge queue Nov 17, 2023
Merged via the queue into esp-rs:main with commit 5503121 Nov 17, 2023
17 checks passed
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.

None yet

2 participants