Skip to content

Embedded-Hal PR - incoorparate e-hal master changes in spi / i2c / gpio / delay #224

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 26 commits into from
May 8, 2023

Conversation

Vollbrecht
Copy link
Collaborator

@Vollbrecht Vollbrecht commented Mar 27, 2023

This PR is in preparation of this PR
It will also resolve issue rust-lang/cargo#99 and will supersede PR rust-lang/cargo#223 & PR rust-lang/cargo#138

The linked embedded-hal PR also already implements changes from current master embedded-hal::delay & embedded-hal::digital, therefore this PR made also changes to gpio.rs and mod.rs to make it work.

Edit1: also incorporate the i2c ehal master changes merged rust-embedded/embedded-hal#440

Edit2: also impl pwm traits inside ledc.rs

@Vollbrecht Vollbrecht marked this pull request as draft March 27, 2023 18:22
src/spi.rs Outdated
bus.transfer_in_place(words)
}
}?;
for op in operations {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This still needs work. Should just convert the 0.2 Operation to 1.0 Operation, otherwise it will not do it in one transaction.

where
T: Borrow<SpiDriver<'d>> + 'd,
{
type Bus = SpiBusDriver<()>;
fn read_transaction(&mut self, operations: &mut [&mut [u8]]) -> Result<(), Self::Error> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Its a lot of duplication here from transaction. The problem is the transaction takes an &mut [Operation<'_, u8>] but this read_transaction is &mut [&mut [u8]]. We would need something that converts the &mut [u8] in an Operation::Read

where
T: Borrow<SpiDriver<'d>> + 'd,
{
fn write_transaction(&mut self, operations: &[&[u8]]) -> Result<(), Self::Error> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same as read_transaction

@Vollbrecht
Copy link
Collaborator Author

keep track with e-hal master -> i2c iter fn trait imps where removed rust-embedded/embedded-hal#440

@Vollbrecht Vollbrecht changed the title Embedded-Hal PR - SpiDevice transaction take an operation slice instead of a closure Embedded-Hal PR - incoorparate e-hal master changes in spi / i2c / gpio / delay Apr 1, 2023
fn to_gpio_err(err: EspError) -> GpioError {
GpioError::other(err)
}

impl<'d, T: Pin, MODE> embedded_hal::digital::ErrorType for PinDriver<'d, T, MODE> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is kinda hacky... the embedded_hal_error! doesnt work for the ulp_processor .... so a lot of feature flags. Ideas how to do that better welcome.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe try to make embedded_hal_error compatible with the ulp mini-HAL? EspError does exist when the mini-HAL is enabled, so perhaps you can conditionalize the macro body itself, rather than conditionalizing all places where it is used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

did it in fe4875c though i have a follow up question, it builds and seem to work, but i thought that one should use absolute path, inside a macro, though it works if i use the cfg feature like that outside the macro. What is your opinion on that? This way the macro is cleaner to read but i am not sure if this is correct.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i duplicate the macro now if used in ulp context d877faa to circumvent possible macro errors

break;
}
}
let mut bus = SpiBusDriver {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if unstable chunked_iter would be available this could be all gone.

@Vollbrecht Vollbrecht marked this pull request as ready for review April 6, 2023 11:29
@Vollbrecht Vollbrecht mentioned this pull request Apr 6, 2023
@Vollbrecht
Copy link
Collaborator Author

ci fails duo to rust-lang/crates.io#6306

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