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

add initial embedded-hal v1 implementations for pins, delays & PWM #470

Merged
merged 5 commits into from
Dec 24, 2023

Conversation

rursprung
Copy link
Contributor

@rursprung rursprung commented Dec 23, 2023

this is a first step towards implementing embedded-hal v1 traits.

see the individual commit messages for further details.

this is as far as i'll take it in terms of implementation, as i only have an arduino uno lying around and currently don't have any peripherals to test with. however, this is a starting point for others to add the trait implementations which they'll need.

namely the SPI & I2C traits are not implemented. all other traits from embedded-hal should be implemented with this PR. note that some traits from v0 were moved to other crates (e.g. embedded-io) or were removed in v1.

see also #468

@Rahix
Copy link
Owner

Rahix commented Dec 24, 2023

Hi,

sorry, I was too fast about merging the dependabot PRs :( Can you please rebase your patch series ontop of latest main and resolve the conflicts?

this does not yet implement any 1.0.0 APIs, this is just a first step to
make them available for implementation.

the existing v0.2 dependency is being kept but is internally renamed to
`embedded-hal-v0`. the re-export as `avr_hal_generic::hal` still points
to v0.

v1 APIs can now be implemented step-by-step. once all APIs are
implemented the v0 APIs can be removed as a breaking change.
the implementation for nanoseconds will only do so with
microsecond-accuracy as there's currently no implementation for ns.
this is ok(-ish), as the trait allows to delay for longer than
specified. if an implementation with us-accuracy can be done this can be
done so later as a follow-up.

the ns-implementation is done explicitly as the blanket implementation
would otherwise transform it into us-calls which then lose precision.
the ms-implementation was left to the blanket-implementation as it
correctly steps it down to ns, as is also done for the old v0
implementations.
this is the same as `uno-blink.rs`, however the actual blinking is done
by a function which does not make any use of any `avr-hal` specific
APIs, instead it solely relies on `embedded-hal` v1 traits.

note that the current v1.0.0-rc.3 version of `embedded-hal` still has
`toggle` as a separate trait fn on `ToggleableOutputPin`, however that
has been removed and v1.0.0 will ship with a blanket impl of `toggle` in
`StatefulOutputPin`. so when updating to v1.0.0 the manual
`set_low`/`set_high` calls can be removed, making the implementation
equivalent to `uno-blink.rs`.
this also ships with an example making use of it. note that the fading
of the new example is faster than the existing `uno-simple-pwm.rs`
example, since the latter fades with 255 steps while the new example
uses 0 - 100% percent (and thus only sleeps 200 * 10ms instead of
512 * 10ms).
@rursprung
Copy link
Contributor Author

Hi,

sorry, I was too fast about merging the dependabot PRs :( Can you please rebase your patch series ontop of latest main and resolve the conflicts?

no worries. i've rebased & pushed it again.

Copy link
Owner

@Rahix Rahix left a comment

Choose a reason for hiding this comment

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

Looking very good indeed, thanks so much for picking up work on this. And thanks for the clean commits, it is much appreciated :)

Let's merge this as is and continue iterating on the support for the new e-h in main.

Comment on lines -15 to +18
[dependencies.embedded-hal]
[dependencies.embedded-hal-v0]
version = "0.2.3"
package = "embedded-hal"
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for doing it this way, it makes the transition so much easier :)

@Rahix Rahix merged commit cca5972 into Rahix:main Dec 24, 2023
23 checks passed
@rursprung rursprung deleted the add-embedded-hal-v1 branch December 26, 2023 09:52
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.

2 participants