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.

Sign up for GitHub

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

Jump to bottom

migrate to embedded-hal=1.0.0.rc1 #25

Merged
merged 3 commits into from
Nov 28, 2023
Merged

migrate to embedded-hal=1.0.0.rc1 #25

merged 3 commits into from
Nov 28, 2023

Conversation

rursprung
Copy link
Collaborator

see the individual commit messages for more details

this resolves #3

@rursprung rursprung force-pushed the migrate-to-eh1.rc1 branch 2 times, most recently from 2e23e9d to 844f01c Compare November 25, 2023 09:14
@rursprung
update to embedded-hal-mock = "0.10.0-rc.2"
a152edb 
also ensure that `done()` is called on all mocks (otherwise the tests
now fail). this also unearthed 1-2 bugs where the expectations were
wrong.

since e-h-m 0.10.0 requires rust 1.63 the MSRV for this crate has been
raised accordingly as we now cannot test on older releases anymore. it
might still work there however.
@rursprung
Copy link
Collaborator Author

the newly emitted warning seems to be a false-positive: rust-lang/rust#76053

@jannic
Copy link

jannic commented Nov 26, 2023

the newly emitted warning seems to be a false-positive: rust-lang/rust#76053

To provide a little more explanation: The ? internally gets expanded into a match statement. The branches are annotated with #[allow(unreachable_code)]. As I understand it, this is because ? should be usable on Results where one of the types is !, without causing a warning. Even if that's not the case here, the #[allow(unreachable_code)] is still part of the generated code.

This crate uses #[forbid(warnings)], which should imply #[forbid(unreachable_code)], contradicting the later #[allow(unreachable_code)] from the ? expansion.

The new thing is that #[forbid(warnings)] didn't work in the past, but will probably will work in the future. See rust-lang/rust#81670 for details. As a transitional measure, it doesn't cause a compilation error, but only a future compatibility warning. The suggestion is to change #[forbid(warnings)] to #[deny(warnings)]. I tend to agree that #[forbid(warnings)] is just too strong, and that there are good reasons to allow some warnings locally.

@rursprung
migrate to embedded-hal=1.0.0.rc1
2011c21 
as alpha.10 of embedded-hal has added `SetDutyCycle` as the equivalent /
successor of the old `PwmPin` support for it can now be added here.

since e-h 1.0.0 will be released at the end of this year it's safe to
just directly migrate to the RC1 instead of adding it as an optional
support (requiring another breaking change in a later step which would
then remove e-h 0.2 support).
@rursprung
change forbid for warnings & unused to deny
204de95 
a new warning was caused by the `?` operator as it contains
`#[allow(unreachable_code)]`, which clashes with the previous `forbid`
rule.

as `forbid` does not allow `deny` to override it this would cause a
compile error, however due to a compiler bug this was previously not
reported at all and has now been changed to a warning in a recent(-ish)
version of rustc and will eventually become a compile error.

with `deny` it's still possible to use `allow`, thus there's no warning

see also rust-lang/rust#76053

thanks @jannic
@rursprung rursprung merged commit 204de95 into master Nov 28, 2023
5 checks passed
@rursprung rursprung deleted the migrate-to-eh1.rc1 branch November 28, 2023 08:36
This was referenced Nov 28, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

support embedded-hal 1.0
2 participants
@rursprung @jannic