Skip to content

try to use fugit::Duraction for CountDown::Time #381

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

Merged
merged 1 commit into from
Dec 28, 2021
Merged

try to use fugit::Duraction for CountDown::Time #381

merged 1 commit into from
Dec 28, 2021

Conversation

burrbull
Copy link
Member

@burrbull burrbull commented Nov 11, 2021

Not for SysTick for now Use 1 microsecond as minimal timeout for SysTick Counter

This PR lets to use fugit units for timer including values bigger then 1 second.
Sampling of timer (time of 1 tick) now can be set in by FREQ generic constant.

CHANGELOG.md Outdated
@@ -19,9 +19,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Changed

- [breaking-change] use `fugit::Duraction` for `CountDown::Time` [#381]
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: Duraction -> Duration (also in the PR title). Maybe add "instead of a frequency" to the line?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks

CHANGELOG.md Outdated
@@ -19,9 +19,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Changed

- [breaking-change] use `fugit::Duraction` for `CountDown::Time` [#381]
- [breaking-change] Remove all deprecated
Copy link
Contributor

Choose a reason for hiding this comment

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

All deprecated what? 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

Don't pay attention. Accidentally deleted in previous PR

@burrbull burrbull force-pushed the fugit branch 2 times, most recently from f855b6d to 31f26d0 Compare November 13, 2021 14:20
@burrbull burrbull marked this pull request as ready for review November 13, 2021 14:30
@burrbull burrbull marked this pull request as draft November 13, 2021 18:18
@burrbull burrbull added the DNM Do not merge label Nov 13, 2021
@burrbull burrbull force-pushed the fugit branch 2 times, most recently from 551e2f7 to 1df2004 Compare November 14, 2021 20:32
@burrbull
Copy link
Member Author

cc @andresv I've tried to implement atat::Clock also. Is this correspond to your vision?

@andresv
Copy link
Member

andresv commented Nov 15, 2021

@burrbull yes I think currently this is what must be done, embedded-hal did not have suitable traits therefore atat::Clock was born.

It is a little bit inconvenient to use atat::Clock on embedded platforms that do not use fugit based timing (almost all HALs at the moment) because essentially you have to make a new type and impl Clock for that which probably involves reading timer counter from a specific register using unsafe.

Also last time I read that CountDown is maybe thrown away from embedded-hal v1.0, so I guess we do not have anything stable for a long time.

@burrbull
Copy link
Member Author

It is a little bit inconvenient to use atat::Clock on embedded platforms that do not use fugit based timing (almost all HALs at the moment) because essentially you have to make a new type and impl Clock for that which probably involves reading timer counter from a specific register using unsafe.

Before port implementation to other HALs (and merge to this one), I must be sure it works as expected (I'm not sure it is so).

@andresv
Copy link
Member

andresv commented Nov 15, 2021

Do you mean that fugit itself is maybe not suitable or what do you mean by that?

@burrbull
Copy link
Member Author

Do you mean that fugit itself is maybe not suitable or what do you mean by that?

I mean I need someone test this and told me if I made mistake.
And yes, I still not sure about fugit.

@burrbull burrbull removed the DNM Do not merge label Dec 28, 2021
@burrbull burrbull marked this pull request as ready for review December 28, 2021 18:00
@burrbull
Copy link
Member Author

Now it just adds new structs, so no breaking changes.

Copy link
Member

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

Very nice!

bors r+

@bors bors bot merged commit 77c2265 into master Dec 28, 2021
@bors bors bot deleted the fugit branch December 28, 2021 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants