-
Notifications
You must be signed in to change notification settings - Fork 139
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
RTIC Monotonic for RTC and TIMER #427
Conversation
Thanks! I've updated this to work with current master and fixed a bunch of small things. It looks good, except that it doesn't build for nrf51 or nrf52832. Is that expected? |
This should not be the case, at least not for the nrf52832 chip as the docs specify the same field, I think that it is simply a matter of the field names either not existing in the pac or being different from that of the nrf52840 that we tested on. I will look into it |
Thanks for working on this! I'm easily confused. Should this trait be named |
@qwandor it seems that we might need to modify the bits manually or re-write parts of the PAC. As this is a small feature I think that it is better to simply unsafely modify the bits and not re-write the PAC for this. I implemented a short fix that should be tested before including as it adds three extra bits of unchecked unsafe code. I do, however, not have any direct way of testing this. So if you have time it would be great if you looked over the modifications so we at least have two sets of eyes on the unsafe bits before calling it good. |
This is probably a good idea as it describes the feature better. |
Thanks! We should probably fix the PAC to add the missing fields, but I'm alright with this for now. |
Do you mean the feature flag? |
If at all possible it would be great to test it on these devices though, I only have access to 52840 devkit so I cannot test it on any other devices. |
* Test monotonic feature in CI. * needs testing * remove excessive unsafe * Fix build. * correct docs --------- Co-authored-by: Andrew Walbran <[email protected]>
Excuse the force push, I resolved the conflicts with a PR instead. I also renamed the user-facing feature to rtic-monotonic as I find it describes the feature better. The feature in nrf-hal-common is still simply Did not know feat:... was a thing, the more you know! |
@qwandor Yeah, meant the feature flag. I think we should probably have someone test any |
Testing the unsafe parts should be as simple as porting the blinky example to the affected devices and running it, does it work it is correct. |
Unfortunately I don't have any nRF51 or nRF52832 board either. I've tested it on an nRF52833 and that works at least, and the code looks reasonable to me. |
This PR aims to add rtic monotonic implementations for the RTC and TIMER.
As #384 requested. This would allow users of the hal to use native implementations
of the Monotonic trait rather than third-party ones and therefore ensure future support for the
feature.
Added
--features monotonic
Features
Usage
There exists an example of how to instantiate the monotonic rtc and timer in
nrf-hal/examples/monotonic-blinky
.RTC
The RTC cannot at compile time guarantee that the frequency is correct.
It does however check this at initiation time, so for any invalid
frequency it will throw an error leaving the user to correct the frequency.
By taking a reference to the clock object we can ensure that the
lfclk
is enabled.Timer
The timer abstraction supports frequency gating since there are only 9 different prescaler values.
This ensures that the
MonotonicTimer
object is not constructible for any frequency that doesnot yield a valid prescaler. The documentation for the
MonotonicTimer
lists allvalid frequencies in a table with their corresponding time until overflow.
Recommended actions
Squash and merge to remove the long commit history.
Alternatives
There are already a few third-party implementations of these features that are less configurable