-
Notifications
You must be signed in to change notification settings - Fork 239
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
Unify the low-power peripheral names (RTC_CNTL
and LP_CLKRST
to LPWR
)
#1064
Conversation
62ce066
to
83c6557
Compare
83c6557
to
bdd63e5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for taking care of this!
@bjoernQ @MabezDev (or anybody else), if you have strong feelings about the name of this peripheral please let me know. We never came to a consensus really in the linked issue, so we just made a choice so we could move forward with this. Obviously I think the choice we made is reasonable, but we can change it if there is good reason to.
@MabezDev @jessebraham There're still a lot of places where old naming is used without a problem/ necessary, same as in |
I think I'm fine with the new naming. Those places where we internally use the old name to get a pointer is probably a non-issue I'd say. Maybe the only confusing thing about it is, that we use two different names for the same thing but even if we change it in the implementation details everyone still needs to keep it in mind since e.g. the TRMs (and the rest of the world) call it by their TRM names |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for taking care of this!
Regarding the leftover namings, some of them are intentional, for example in the places you see SYSTEM/DPORT
, that is there to help with search-ability, because if you're looking for DPORT on an esp32 and you only see SYSTEM that can be a little confusing at first. Maybe there is a better way to portray this information though 🤔.
closes #872