atsamd
atsamd copied to clipboard
Add initial support for `embedded-hal` version 1.0
Summary
This adds a dependency and feature for embedded-hal v1.0 and a basic implementation for the DelayNs trait. I think it is best to start with this small subset to make sure CI and everything are set up properly, then add other traits one at a time.
Part of #332
Checklist
- [x]
CHANGELOG.mdfor the BSP or HAL updated - [ ] All new or modified code is well documented, especially public items
- [ ] No new warnings or clippy suggestions have been introduced - CI will deny clippy warnings by default! You may
#[allow]certain lints where reasonable, but ideally justify those with a short comment.
If Adding a new Board
- [ ] Board CI added to
crates.json - [ ] Board is properly following "Tier 2" conventions, unless otherwise decided to be "Tier 1"
If Adding a new cargo feature to the HAL
- [ ] Feature is added to the test matrix for applicable boards / PACs in
crates.json
What is the best way to make sure this gets tested in CI? I am not sure if I should update the matrix for this.
Thanks @tgross35! I'll create an embedded-hal-1 branch in the atsamd-hal repo, so that we can split the effort in multiple PR, and merge that once we're ready. Can you please change this PR to use that as its base branch?
I'll have to review your code bit more in detail concerning the timer stuff, but I don't think we should hide embedded-hal v1.0 behind a feature. Implementations for the 0.2 and 1.0 versions can easily coexist for one release, after which I plan on removing 0.2 if everyone else agrees. It think it's also a good opportunity to remove the unproven feature, which imp doesn't really serve a purpose at this point. Added advantage is that CI will check the new ehal v1.0 additions without further configuration.
As far as naming, I think I would reexport embedded-hal v1.0 as ehal, and v0.2 as ehal_0_2. To me it makes it a bit clearer that 1.0 is the "default". What do you think?
Hey, thanks for looking @jbeaurivage! The reason I started doing it in this additive way, rather than replacing, is that v1.0 has a list of removed traits that https://github.com/rust-embedded/embedded-hal/blob/master/docs/migrating-from-0.2-to-1.0.md#removed-traits. In particular, CountDown, Periodic, and Pwm all provide pretty important functionality.
But maybe it is okay to switch to eh1.0 as the primary implementation while still keeping eh0.2 around to fill in the missing traits. In this case, I think it would be better to not gate either of them behind a feature because some of the eh1.0 relies on eh0.2 (e.g. in this PR eh1::DelayNs makes use of CountDown). What are your thoughts?
Yeah, what I meant is for the transition release, we should provide implementations for both versions of ehal in parallel.
Furthermore, I would suggest we take the actual logic out of the ehal v0.2 implementations, and instead put them in inherent methods on the peripherals (or on the ehal v1.0 traits of you prefer) . Implementing v0.2 traits then becomes a matter of forwarding to the correct method call. The idea being that 1.0 implementations should be totally independent from 0.2 impls (the reverse isn't necessary, however). Then, when we'll want to remove 0.2, we can simply delete everything related to 0.2 without affecting anything else.
superceded by #723