- Notifications
You must be signed in to change notification settings - Fork149
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Refactor IWDG driver and add initialize function for prescaler with downcounter#1127
Conversation
8abf5f9 to5a4a9bbCompareThere 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.
Thanks!
| out_index = idx; | ||
| } | ||
| }; | ||
| for (uint32_t idx{0}; const auto prescaler : prescalers) |
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.
Maybe
| for (uint32_t idx{0};constauto prescaler : prescalers) | |
| for (constauto[idx,prescaler] :enumerate(prescalers)) |
?
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.
oh,std::ranges::enumerate_view exists, so we can alias this tomodm::enumerate.
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.
👍, but that requires gcc 13. Mac OS hosted and AVR currently have 12. Let's upgrade the compilers for the next release and do it afterwards.
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.
We need to find a workaround for the Protothread macro issue to upgrade to GCC13…
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.
I don't think this is fixable. The whole construct relies on jumping into a statement expression with a case label. According to the gcc documentation this was never valid but is now properly diagnosed since gcc 13. It just happened to work often enough ...
The only acceptable solution I see is abandoning resumable functions.
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.
This doesn't seem to work due to an (for me) incomprehensible compiler error aboutunsigned int& lvalue vsunsigned int rvalue… help (see CI for issue)
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.
It's a limitation inmodm::enumerate. The iterator types you have defined (linear_range, etc.) return r-values, here plain integers, not l-value references when you dereference them and the implementation can't deal with that. I'll push a fix tonight.
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.
It's fixed now. I've also pushed a fixup for a typo in the Nucleo G070module.lb file.
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.
Thank you!
3ec43f7 to893a7f3Compare893a7f3 to23036e3Compare
Re-opening this since I prematurely merged the previous PR#1122 due to pushing the wrong branch (PEBCAK). Addresses the last review. Original description:
Ok, this escalated a little. I was just looking through the code and saw that the Iwdg class was not inlined, so I quickly refactored it and then noticed that the prescaler was manually computed and then I had to program a constexpr algorithm for it.
modm::platformnamespace.Rcc::LsiandRcc::Hsiclock.modm.digsepfilter for nicely formatting long integers.SystemClock::Iwdgto every board.assertBaudrateInToleranceimplementation.seconds_t,milliseconds_t,microseconds_tfor template parameters.percent_tcan be float since C++20.from_rangetofrom_linear.from_iteratortofrom_rangeand implement it to be compatible withstd::ranges::forward_range.