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

Refactor IWDG driver and add initialize function for prescaler with downcounter #1127

Merged
merged 12 commits into from
Feb 6, 2024

Conversation

salkinium
Copy link
Member

@salkinium salkinium commented Feb 3, 2024

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.

  • Inline IWDG functions
  • Move into modm::platform namespace.
  • Add prescaler + downcounter algorithm.
  • Add unittest for prescaler + downcounter algorithm.
  • Add Rcc::Lsi and Rcc::Hsi clock.
  • Add modm.digsep filter for nicely formatting long integers.
  • Add SystemClock::Iwdg to every board.
  • Add a generic assertBaudrateInTolerance implementation.
  • Add documentation for prescaler calculators.
  • Simplify tolerance assertion functions.
  • Added seconds_t, milliseconds_t, microseconds_t for template parameters.
  • percent_t can be float since C++20.
  • Fix minimum prescaler value for STM32 UART (8 or 16).
  • Rename from_range to from_linear.
  • Rename from_iterator to from_range and implement it to be compatible with std::ranges::forward_range.

@salkinium salkinium added this to the 2024q1 milestone Feb 3, 2024
@salkinium salkinium added the ci:hal Triggers the exhaustive HAL compile CI jobs label Feb 4, 2024
Copy link
Member

@chris-durand chris-durand left a comment

Choose a reason for hiding this comment

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

Thanks!

src/modm/board/nucleo_g071rb/board.hpp Show resolved Hide resolved
src/modm/math/algorithm/prescaler.hpp Show resolved Hide resolved
src/modm/math/algorithm/prescaler_counter.hpp Outdated Show resolved Hide resolved
out_index = idx;
}
};
for (uint32_t idx{0}; const auto prescaler : prescalers)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe

Suggested change
for (uint32_t idx{0}; const auto prescaler : prescalers)
for (const auto [idx, prescaler] : enumerate(prescalers))

?

Copy link
Member Author

@salkinium salkinium Feb 5, 2024

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 to modm::enumerate.

Copy link
Member

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.

Copy link
Member Author

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…

Copy link
Member

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.

Copy link
Member Author

@salkinium salkinium Feb 5, 2024

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 about unsigned int& lvalue vs unsigned int rvalue… help (see CI for issue)

Copy link
Member

@chris-durand chris-durand Feb 6, 2024

Choose a reason for hiding this comment

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

It's a limitation in modm::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.

Copy link
Member

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 G070 module.lb file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you!

src/modm/math/algorithm/prescaler_counter.hpp Outdated Show resolved Hide resolved
src/modm/math/algorithm/prescaler.hpp Outdated Show resolved Hide resolved
@salkinium salkinium force-pushed the feature/init_iwdg_by_time branch 3 times, most recently from 3ec43f7 to 893a7f3 Compare February 6, 2024 20:56
@salkinium salkinium merged commit 23036e3 into modm-io:develop Feb 6, 2024
12 checks passed
@salkinium salkinium deleted the feature/init_iwdg_by_time branch February 6, 2024 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants