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

PAC update #2378

Merged
merged 2 commits into from
Oct 22, 2024
Merged

PAC update #2378

merged 2 commits into from
Oct 22, 2024

Conversation

bugadani
Copy link
Contributor

Ripped out of #2359

@bugadani bugadani added the skip-changelog No changelog modification needed label Oct 21, 2024
}

/// Sets pull-up enable for the pin
pub fn pullup_enable(&self, enable: bool) {
get_pin_reg(PIN).modify(|_, w| w.fun_wpu().bit(enable));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might've been worth keeping the get_pin_reg function still. 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's impossible if we use ::steal() because it would return a reference to a local now. We don't have to use steal, thoguh, but it looks better to my brain than random pointer dereferencing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

but it looks better to my brain than random pointer dereferencing

Same 😄

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.

Straight forward change (beside GitHub's rendering of the diff 😢)

lp_io.[< pin $gpionum >]().modify(|_, w| {
w.wakeup_enable().bit(wakeup).int_type().bits(level)
$(
impl $crate::gpio::RtcPin for GpioPin<$gpionum> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

With the new PAC update, this impl can be moved out of the macro. I'm hoping to see this in some future PR.

Copy link
Contributor Author

@bugadani bugadani Oct 21, 2024

Choose a reason for hiding this comment

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

That was one of the main motivators behind the relevant PAC change, and we're almost there.

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

@bjoernQ bjoernQ added this pull request to the merge queue Oct 22, 2024
Merged via the queue into esp-rs:main with commit a4bc053 Oct 22, 2024
28 checks passed
@bugadani bugadani deleted the pac branch October 22, 2024 07:23
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.

3 participants