Skip to content

Remove a bunch of generic params from GpioPin - #553

Merged
MabezDev merged 1 commit into
esp-rs:mainfrom
bugadani:gpio
May 22, 2023
Merged

MabezDev merged 1 commit into
esp-rs:mainfrom
bugadani:gpio

Conversation

@bugadani

@bugadani bugadani commented May 20, 2023 •

Copy link
Copy Markdown
Contributor
  • The code compiles without errors or warnings. - even the original code had warnings.
  • All examples work. - if they compile, they should 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). - as above.
  • Added examples are checked in CI
  • You add a description of your work to this PR.
  • You added proper docs for your newly added features and code. - no new features

PR description

We had a conversation with @MabezDev over at matrix where he mentioned that ideally GPIOs should have fewer generic parameters. Since their capabilities are tied to the hardware, this PR refactors those generic parameters into a companion trait.

@bugadani
bugadani marked this pull request as ready for review May 20, 2023 08:07
@bjoernQ

bjoernQ commented May 22, 2023 •

Copy link
Copy Markdown
Contributor

Great idea! 👍 We probably can apply this pattern to other drivers, too 🤔

@bugadani

bugadani commented May 22, 2023 •

Copy link
Copy Markdown
Contributor Author

I haven't used other peripherals yet other than Spi. There, the DMA channel config could maybe be slightly simplified (though I'm not sure about what the SuitablePeripheralX means just yet and if that fits into this or not).

@MabezDev MabezDev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a really nice PR, and definitely something I want to apply to our other type-heavy structures.

Just one small comment to address and this should be ready to go :)

Comment thread esp-hal-common/src/gpio.rs

@bjoernQ bjoernQ left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@MabezDev
MabezDev merged commit 02c7e38 into esp-rs:main May 22, 2023
@bugadani
bugadani deleted the gpio branch May 22, 2023 11:28
SergioGasquez pushed a commit to SergioGasquez/esp-hal that referenced this pull request Jun 9, 2023
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.

3 participants