Skip to content

Add embedded-hal v1.0.0-alpha.8 support#106

Closed
almindor wants to merge 3 commits intomasterfrom
e-h-1.0.0-alpha.8
Closed

Add embedded-hal v1.0.0-alpha.8 support#106
almindor wants to merge 3 commits intomasterfrom
e-h-1.0.0-alpha.8

Conversation

@almindor
Copy link
Contributor

@almindor almindor commented Jul 31, 2022

Fixed Delay trait for use with embedded-hal v1.0.0-alpha.8

@almindor almindor force-pushed the e-h-1.0.0-alpha.8 branch from bff75fb to 5b27b4b Compare July 31, 2022 18:29
@almindor almindor marked this pull request as ready for review July 31, 2022 18:37
@almindor almindor requested a review from a team as a code owner July 31, 2022 18:37
@almindor almindor mentioned this pull request Jul 31, 2022
Open
@almindor
Copy link
Contributor Author

almindor commented Jul 31, 2022 •
edited
Loading

@Disasm Not quite sure how we want to support pre-releases. I guess we could keep the PRs open and just push pre-release crates.io (I think that's how e-h did it?)

This is the first in a big list of deps to support all of:
rust-embedded/riscv-rt#101
riscv-rust/e310x#26
riscv-rust/e310x-hal#45

This was referenced Jul 31, 2022
Draft
Closed
@Disasm
Copy link
Member

Disasm commented Jul 31, 2022

I think only riscv and e310x-hal require update as we should be able to use multiple riscv versions simultaneously. I think it's ok to create a release from a PR branch if it works, but please don't merge these branches to master as it may cause problems in the future.

@almindor almindor added the do not merge this code should not be merged to master label Jul 31, 2022
rmsyn
rmsyn reviewed Oct 13, 2023
View reviewed changes
src/delay.rs

#[inline]
fn delay_us(&mut self, us: u64) {
fn delay_us(&mut self, us: u32) -> Result<(), Self::Error> {
Copy link
Contributor

@rmsyn rmsyn Oct 13, 2023

Choose a reason for hiding this comment

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

In the latest embedded-hal release candidate, embedded-hal-1.0.0-alpha.11 this trait no longer returns a Result, nor has an associated Error type.

Copy link
Contributor

@romancardenas romancardenas Oct 14, 2023

Choose a reason for hiding this comment

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

Shouldn't we aim at 1.0.0-rc.1?

Copy link
Contributor

@rmsyn rmsyn Oct 15, 2023

Choose a reason for hiding this comment

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

Shouldn't we aim at 1.0.0-rc.1?

Definitely, same story there: https://docs.rs/embedded-hal/1.0.0-rc.1/embedded_hal/delay/trait.DelayUs.html

Wasn't sure about the separation, so just went with the alpha line of releases.

@romancardenas romancardenas mentioned this pull request Oct 16, 2023
Merged
@romancardenas
Copy link
Contributor

romancardenas commented Oct 16, 2023

#147 points to e-h-1.0.0-rc1 and solves all the conflicts. It also bumps MSRV to 1.60.

@romancardenas
Copy link
Contributor

romancardenas commented Oct 23, 2023

Closing, as this has been addressed in #147

@romancardenas romancardenas deleted the e-h-1.0.0-alpha.8 branch November 27, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@romancardenas romancardenas romancardenas left review comments

@rmsyn rmsyn rmsyn left review comments

Assignees

No one assigned

Labels

do not merge this code should not be merged to master

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@almindor @Disasm @romancardenas @rmsyn