Skip to content

Add half_duplex_write_async method to SpiDmaBus#5135

Draft
BestClaws wants to merge 4 commits intoesp-rs:mainfrom
BestClaws:spidma_half_duplex_write_async
Draft

Add half_duplex_write_async method to SpiDmaBus#5135
BestClaws wants to merge 4 commits intoesp-rs:mainfrom
BestClaws:spidma_half_duplex_write_async

Conversation

@BestClaws
Copy link
Copy Markdown

@BestClaws BestClaws commented Mar 8, 2026

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • I have updated existing examples or added new ones (if applicable).
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • I have added necessary changes to user code to the latest Migration Guide.
  • My changes are in accordance to the esp-rs developer guidelines

Extra:

Pull Request Details 📖

Description

SpiDmaBus has convenience async methods but lacks half duplex write async version. the existing version does a busy poll which is not desired in async environments

Testing

Verified on my hardware. no test code in place.

@BestClaws BestClaws changed the title Add async half-duplex write method to SpiDmaBus Add half_duplex_write_async method to SpiDmaBus Mar 8, 2026
@H01001000
Copy link
Copy Markdown

I think you can also add the read async too

@bugadani
Copy link
Copy Markdown
Contributor

bugadani commented Mar 9, 2026

Testing

HIL tests

I think you should add some if you want to make this part of the PR description true ;)

Comment thread esp-hal/src/spi/master.rs
@BestClaws
Copy link
Copy Markdown
Author

Testing
HIL tests

I think you should add some if you want to make this part of the PR description true ;)

I may have misunderstood what HIL means from other reference PRs. Just tested it on hardware. let me update the pr description.

On a side note: both sync/async half_duplex_write methods seem to rendering the single SPI methods like transaction buggy giving bad data without error. although I need to verify that fact is actually the case

@bugadani
Copy link
Copy Markdown
Contributor

bugadani commented Mar 9, 2026

Declaring "HIL test" on a PR (besides being lazy wording) means that the relevant change is tested in CI. In this case, that needs a new test case - and this PR will eventually need a new test case. You don't need to add any, but we would appreciate if you did.

@github-actions github-actions Bot added the merge-conflict Merge conflict detected. Automatically added/removed by CI. label Mar 25, 2026
@github-actions
Copy link
Copy Markdown

New commits in main has made this PR unmergable. Please resolve the conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-conflict Merge conflict detected. Automatically added/removed by CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants