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

Remove pins from Io #2508

Merged
merged 3 commits into from
Nov 12, 2024
Merged

Remove pins from Io #2508

merged 3 commits into from
Nov 12, 2024

Conversation

bugadani
Copy link
Contributor

@bugadani bugadani commented Nov 8, 2024

This PR has two implications:

  • Users who don't need to work with GPIO interrupts no longer have to construct Io
  • Users who need to work with GPIO interrupts can now configure them even when pins are in use. Previously this was impossible because pins were moved out of Io so users could no longer call methods on it.

This is a slight variation on the idea originally raised by #2010 (comment)

Sorry, this touches literally every example we have.

@bugadani bugadani force-pushed the io branch 5 times, most recently from 7234bbc to 61ac0d2 Compare November 8, 2024 18:10
@bugadani bugadani marked this pull request as ready for review November 8, 2024 18:12
@bugadani bugadani force-pushed the io branch 5 times, most recently from 8319f71 to bf59f7a Compare November 8, 2024 19:26
@bugadani bugadani marked this pull request as draft November 9, 2024 11:55
@bugadani bugadani force-pushed the io branch 4 times, most recently from 0cf07f8 to ec8583e Compare November 9, 2024 12:54
@bugadani bugadani marked this pull request as ready for review November 9, 2024 13:15
Copy link
Collaborator

@Dominaezzz Dominaezzz left a comment

Choose a reason for hiding this comment

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

LGTM

This caught my eye but I don't think it's important enough to change.

peripherals: [
    // comma separated
],
pins: [
    // not comma separated
]

(Mildly surprised the S2's GPIO alternative functions haven't been mapped out yet)

@bugadani
Copy link
Contributor Author

This caught my eye but I don't think it's important enough to change.

I didn't want to mess with the macro syntax, this is a straight-up cut and paste.

Copy link
Contributor

@bjoernQ bjoernQ left a comment

Choose a reason for hiding this comment

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

LGTM - lots of changes but a lot of that is unavoidable noise

@bugadani bugadani added this pull request to the merge queue Nov 12, 2024
Merged via the queue into esp-rs:main with commit fbc5754 Nov 12, 2024
42 of 44 checks passed
@bugadani bugadani deleted the io branch November 12, 2024 10:53
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