Skip to content

Reduce macro usage in PDMA#2247

Closed
Dominaezzz wants to merge 1 commit into
esp-rs:mainfrom
Dominaezzz:less_macro_in_pdma
Closed

Reduce macro usage in PDMA#2247
Dominaezzz wants to merge 1 commit into
esp-rs:mainfrom
Dominaezzz:less_macro_in_pdma

Conversation

@Dominaezzz
Copy link
Copy Markdown
Collaborator

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 Migration Guide.
  • My changes are in accordance to the esp-rs API guidelines

Extra:

Pull Request Details 📖

Description

Pull out as much code as possible from the PDMA macros.
My IDE has a better time with code outside macros. If no one else's IDE is bothered by this I'll close the PR.

I've actually gotten the code to the point where I can probably merge the ImplI2sChannel and ImplSpiChannel macros into one macro with a few more parameters.

No changelog since this is an internal refactor.

Testing

It builds, it's just a refactor.

@Dominaezzz Dominaezzz added the skip-changelog No changelog modification needed label Sep 27, 2024
@Dominaezzz Dominaezzz closed this Sep 28, 2024
@Dominaezzz Dominaezzz deleted the less_macro_in_pdma branch September 28, 2024 12:58
@bugadani
Copy link
Copy Markdown
Contributor

Not sure why you closed this, I'm pretty sure the change is worth it, unless you have a better idea.

@Dominaezzz
Copy link
Copy Markdown
Collaborator Author

I remembered what you said in #2196 (comment) and decided to close it, and replace it with #2249 .

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

Labels

skip-changelog No changelog modification needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants