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 type for gpio::AnyPin #1067

Merged
merged 1 commit into from
Feb 8, 2024
Merged

Conversation

Volkalex28
Copy link
Contributor

@Volkalex28 Volkalex28 commented Jan 9, 2024

This makes it possible to safely implement the InputPin and OutputPin traits for AnyPin. Now you can convert any pin to AnyPin with the appropriate type and use it in other library modules

Added:

  • Peripheral implementation for AnyPin
  • Implementation of Pin for AnyPin
  • Implementation of OutputPin for AnyPin with type IsOutputPin
  • Implementation of InputPin for AnyPin with type IsInputPin
  • Upgrade types for AnyPin (for example InputOutputAnalogPinType -> InputOutputPinType)
  • Implementation of From for AnyPin with the appropriate type

Changed:

  • The Gpio::degrage method returns AnyPin with the appropriate type

A small example where this can be useful:

#![no_std]
#![no_main]

use esp32s3_hal::{clock::ClockControl, gpio::IO, i2c::I2C, peripherals::Peripherals, prelude::*};
use esp_backtrace as _;
use esp_println::println;

enum BoardRevision {
    Dev,
    Release
}

// Take data about the device on which the program is running. 
// This is not known at the compilation stage, but is extracted at runtime,
// for example from the device serial number
fn get_board_revision() -> BoardRevision {
    //...
}

#[entry]
fn main() -> ! {
    let peripherals = Peripherals::take();
    let system = peripherals.SYSTEM.split();
    let clocks = ClockControl::boot_defaults(system.clock_control).freeze();

    let io = IO::new(peripherals.GPIO, peripherals.IO_MUX);
    
    // Select pins depending on BoardRevision
    let (sda, scl) = match get_board_revision() {
        BoardRevision::Dev => (io.pins.gpio43.degrade(), io.pins.gpio44.degrade()),
        BoardRevision::Release=>(io.pins.gpio41.degrade(), io.pins.gpio38.degrade()),
    }

    // Create a new peripheral object with the described wiring
    // and standard I2C clock speed
    let mut i2c = I2C::new(peripherals.I2C0, sda, scl, 100u32.kHz(), &clocks);

    loop {
        let mut data = [0u8; 22];
        i2c.write_read(0x77, &[0xaa], &mut data).ok();

        println!("{:02x?}", data);
    }
}

@Volkalex28 Volkalex28 force-pushed the anypin-impl-input-output branch 2 times, most recently from 2f963a0 to 9836fd3 Compare January 9, 2024 10:56
@Volkalex28 Volkalex28 marked this pull request as ready for review January 9, 2024 10:57
@bjoernQ
Copy link
Contributor

bjoernQ commented Jan 10, 2024

Thanks! This is very useful

Maybe the .degrade().degrade() looks a bit weird in the example - probably .degrade().into() would look better

Other than that, this looks really good to me

@Volkalex28 Volkalex28 force-pushed the anypin-impl-input-output branch from 9836fd3 to cb3feac Compare January 18, 2024 21:53
@brychanrobot
Copy link
Contributor

Any chance of this getting merged soon? I'm really excited about the architectural options it will enable.

@bjoernQ
Copy link
Contributor

bjoernQ commented Jan 26, 2024

It looks good - just needs a rebase

@jessebraham
Copy link
Member

Not a fan of the API honestly (led3.degrade().into() means absolutely nothing to me looking at the examples, I can only imagine how confusing this will be for beginners), but I also don't really have any better suggestions so no point blocking it I guess.

@Dominaezzz Dominaezzz mentioned this pull request Jan 28, 2024
16 tasks
@jessebraham
Copy link
Member

@Volkalex28 if you're able to rebase this, then I think we're good to merge.

@Volkalex28
Copy link
Contributor Author

Sorry for delay. I'll rebase this tomorrow.

@Volkalex28 Volkalex28 force-pushed the anypin-impl-input-output branch 5 times, most recently from 58143d4 to 717c55e Compare February 6, 2024 15:13
@Volkalex28
Copy link
Contributor Author

@bjoernQ Please check it after rebase

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@bjoernQ bjoernQ left a comment

Choose a reason for hiding this comment

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

LGTM - needs the changelog fixed before merging

@bjoernQ
Copy link
Contributor

bjoernQ commented Feb 6, 2024

Thanks! We can merge this after fixing the changelog

@Volkalex28 Volkalex28 force-pushed the anypin-impl-input-output branch from 717c55e to 45a6297 Compare February 7, 2024 18:09
This makes it possible to safely implement the InputPin and OutputPin
traits for AnyPin. Now you can convert any pin to AnyPin with the
appropriate type and use it in other library modules

Added:
- Peripheral implementation for AnyPin
- Implementation of Pin for AnyPin
- Implementation of OutputPin for AnyPin with type IsOutputPin
- Implementation of InputPin for AnyPin with type IsInputPin
- Upgrade types for AnyPin (for example InputOutputAnalogPinType ->
InputOutputPinType)
- Implementation of From<Gpio> for AnyPin with the appropriate type

Changed:
- The Gpio::degrage method returns AnyPin with the appropriate type
@Volkalex28 Volkalex28 force-pushed the anypin-impl-input-output branch from 45a6297 to da7a390 Compare February 7, 2024 18:11
@Volkalex28
Copy link
Contributor Author

Volkalex28 commented Feb 7, 2024

Thanks! We can merge this after fixing the changelog

Let me know when I can merge this or just do it 😉

@Volkalex28 Volkalex28 requested a review from bjoernQ February 7, 2024 18:14
@bjoernQ
Copy link
Contributor

bjoernQ commented Feb 8, 2024

Sorry the changelog entry should have been added to Unreleased/Added but I probably wasn't clear about it. Going to merge it as is and I'll fix the changelog afterwards. Thanks again

@bjoernQ bjoernQ added this pull request to the merge queue Feb 8, 2024
Merged via the queue into esp-rs:main with commit 3a456bb Feb 8, 2024
17 checks passed
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