Add generic `Gpio` type

[jeandudey]

This type is useful to implement other peripherals that need access to GPIO pins.

I've added a small TODO to implement into_*_output function for this type, I'm not sure how to proceed since the esp32 with funcX_out_sel_cfg is unergonomic and the code would need to match over each pin to use the correct register. This can be resolved by making these registers an array.

[MabezDev]

This type is useful to implement other peripherals that need access to GPIO pins.

Could you explain a bit more on what you mean by this; perhaps an example would help?

[jeandudey]

One example is for the UART peripherals, applies to I2C and other peripherals that use GPIOs.

pub struct Uart<T>(T);

impl<T> Uart<T>
where
    T: Instance
{
    pub fn new(uart_periph: T, pins: Pins) -> Uart<T> {
    // Configure here the UART peripheral with the provided Pins
    Uart(uart_periph)
    }
}

// Here is the need for a generic GPIO pin type
pub struct Pins {
    pub txd: Gpio<Output<PushPull>>,
    pub rxd: Gpio<Input<Floating>>,
    pub rts: Option<Gpio<Output<PushPull>>,
    pub cts: Option<Gpio<Input<Floating>>,
}

pub trait Instance: Deref<Target = esp32::UART> {}

impl Instance for UART0 {}
impl Instance for UART1 {}

[MabezDev]

So I think this is not something I am in favour of.

Typically, we model the hardware as singletons; that way we can use Rust's ownership model to make sure we don't actually use a resources twice (See the e-r book). For example a pin can only be in one state at a time, hence a single pin cannot be used with a UART & a I2C peripheral.

By allowing this change, anyone could create a Gpio instance and potentially ruin any abstractions we make with peripherals.

In your example, your Pins definition could easily be

pub struct Pins {
    pub txd: Gpio2<Output<PushPull>>,
    pub rxd: Gpio3<Input<Floating>>,
    pub rts: Option<Gpio4<Output<PushPull>>,
    pub cts: Option<Gpio5<Input<Floating>>,
}

Instead; but a better way to approach this would be with a sealed trait.

trait UartRxPin{}
trait UartTxPin{}

impl UartTxPin for Gpio2<Output<PushPull>> {}
impl UartRxPin for Gpio3<Output<PushPull>> {}

then our UART constructor could look like this:

pub fn new<T, R>(uart: esp32::UART, tx: T, rx: R) -> Self 
where R: UartRxPin,
           T: UartTxPin
{
// ..
}

This means we can impl the trait for the other pins that the uart pins can map out to, aswell.

What do you think?

[jeandudey]

Typically, we model the hardware as singletons; that way we can use Rust's ownership model to make sure we don't actually use a resources twice. For example a pin can only be in one state at a time, hence a single pin cannot be used with a UART & a I2C peripheral.

It is a singleton, once you convert a GpioX to a Gpio only a version of that Gpio will exist. It can't be used twice.

By allowing this change, anyone could create a Gpio instance and potentially ruin any abstractions we make with peripherals.

The private _mode field prevents this, and only a Gpio can be created inside of the gpio module, that way ensures that a there's a one-to-one map from a GpioX type to a Gpio { pin: X, _mode: ... }.

Instead; but a better way to approach this would be with a sealed trait.

I sincerely prefer this approach (with the sealed trait), as the ESP32 has a mess with output pins and not all pins can be output. I can't certainly be sure that all output pins can be used as UART (or I2C, I don't remember right now) as it's hard to look at the datasheet to tell, it's a guess work. This way a select group of GpioXs can implement these traits.

Anyway in the long term, a Gpio pin is needed as there are other use cases for it, including having an array of Gpios that can't be done with traits.

[jeandudey]

This approach is not new, it's already used by the nrf52-rs project on the HAL. See this.

Not a strong opinion though.

The long term goal is to have a good set of code to implement peripherals. Either way or another ;)

[MabezDev]

It is a singleton, once you convert a GpioX to a Gpio only a version of that Gpio will exist. It can't be used twice.

Ah yes, I missed that. Apologies.

I sincerely prefer this approach (with the sealed trait), as the ESP32 has a mess with output pins and not all pins can be output. I can't certainly be sure that all output pins can be used as UART (or I2C, I don't remember right now) as it's hard to look at the datasheet to tell, it's a guess work. This way a select group of GpioXs can implement these traits.

I think I remember the datasheet having this information, not the reference manual, but I'd have to check.

including having an array of Gpios that can't be done with traits.

I see, this is something I hadn't considered.

I'm warming up to the idea, now that I understand it properly.

[jeandudey]

Ah yes, I missed that. Apologies.

Don't worry :)

I see, this is something I hadn't considered.

I'm warming up to the idea, now that I understand it properly.

This doesn't need to be merged right now but it's a good start, and maybe while we work on esp32 and polish it this PR could get in a good shape later.

[YonasJ]

I am still learning about this as i try to implement my simple project in Rust. But I wanted to comment here about what could work for me.

One thing that does not work for me about this proposal is that I can't see how to change the direction of a GPIO pin.

In the initial example, I like the way that I can pass the pins around. But it seems there should be a bi-directional type as well.

struct Me {
gpio2:Gpio,
gpio3:GpioInputFloating,
gpio4:GpioOutput,

};
self.gpio2 = Gpio2::new(2, mode:Input_Floating);

let val:u8 = gpio.get();

gpio.set_mode(Output); // Same as esp32 API pinMode(x, OUTPUT); 

self.gpio2.set(1);
self.gpio2.set(0);

// And of the others, that are read only, only expose either get or set.
self.gpio2.get();
self.gpio3.set(1);

I think an API like this would have a couple benefits, including:

1. The API about would be similar to how it works with the C API, so it would be familiar to folks getting started.
2. The API would allow for changing pin directions, which I is necessary for my application and other one-wire bus applications as well.
3. Command complete in the IDE could work, as the types would be simpler. I use intelli-j, and with the current set of macros, it can never seem to tell what is going on after the into recursive macro expansion. So for example for "gpios.gpio25.into_pull_down_input()." there are no completions.

[icewind1991]

Note that you can do something like

pub struct Pins {
    pub txd: Gpio2<Output<PushPull>>,
    pub rxd: Gpio3<Input<Floating>>,
    pub rts: Option<Gpio4<Output<PushPull>>,
    pub cts: Option<Gpio5<Input<Floating>>,
}

already statically using generics like

use embedded_hal::digital::v2::{InputPin, OutputPin};

pub struct Pins<TXD, RXD, RTS CTS>
where
    TXD: OutputPin,
    RXD: OutputPin + InputPin,
    RTS: OutputPin.
    CTS: OutputPin + InputPin,
 {
    pub txd: TXD,
    pub rxd: RXD,
    pub rts: Option<RTS>,
    pub cts: Option<CTS>,
}

which then allows using arbitrary pins that are correctly configured, bitbang-hal uses this method already.

The API would allow for changing pin directions, which I is necessary for my application and other one-wire bus applications as well.

From how understood it, open drain output allows for both input and output.

Command complete in the IDE could work, as the types would be simpler. I use intelli-j, and with the current set of macros, it can never seem to tell what is going on after the into recursive macro expansion. So for example for "gpios.gpio25.into_pull_down_input()." there are no completions.

Have you tried using the new "experimental macro engine" (Settings -> Language & Frameworks -> Rust), I have no issues with getting autocomplete to work for the gpio pins as is.

[YonasJ]

Thanks, your comments above really helped me. TLDR, would there need to be another trait to switch directions - I think you are saying no? AND how would you specify whether you wanted to use the pull up, or pull down resistor for the GPIO?

I tried to get it working with the code from from this patch. But I note as you comment at the top of this email thread, that the input side does not work. Due to the need to add an array.

The API worked for me to define a struct:

pub struct ButtonFlasher<SysLedGpio, BiDirGpio>
where
    SysLedGpio: OutputPin,
    BiDirGpio: OutputPin + InputPin,
{
    pub sys_led: SysLedGpio,
    pub button_with_led: BiDirGpio,
    pub last_val:bool
}

Which made sense to me after I figured out about the templates, and is basically the same as what you state above.

My next intuition move was to create the constructor. This actually took me quite a while, as it was a big learning curve to get the impl that I would need in this case right. Eventually this worked:

impl <SysLedGpio, BiDirGpio, E> ButtonFlasher <SysLedGpio, BiDirGpio>
where
    SysLedGpio: OutputPin<Error = E>,
    BiDirGpio: OutputPin<Error = E> + InputPin<Error = E>

My issue here was that I was fooled by the difference between the actual types and the magic types. For example, the compiler was happy to let me declare

    let mut button_with_led:Gpio15<Output<PushPull>> = gpios.gpio15.into_push_pull_output();

But to my great disappointment, would not let me use those types on the impl part.

OutputPin<Error = E>

I think is has something to do with a reference to the trait rather than the type.

If this get's accepted, I will upload my test code as demo code, so others who are just starting out can figure out this type maze with a little less effort -- I was a bit disappointed that the IDE, the compile and type complete could not help me. I had to know to use the name of the trait and there didn't seem to be any clues in the errors or complete what the correct type was.

After that I was able to finally get the constructor gong.

    fn new(sys_led:SysLedGpio, button_with_led:BiDirGpio) -> ButtonFlasher <SysLedGpio, BiDirGpio> 
    {
        let last_val = match button_with_led.is_high() { Ok(v) => v, Err(_) => false};

        ButtonFlasher {
            sys_led,
            button_with_led,
            last_val: last_val
        }
    }

So, if this change was fully implemented, then button_with_led would implement both InputPin and OutputPin, and thus I could call both directions for set_high/is_high.

If I understand you correctly:

• if the pin was in a pull down mode, writing set_low(), would effectively put the pin into "read" state.
• if the pin was in pull up mode, calling set_high() would effectively put the pin into a "read" state.

And the if I wanted different pin configurations, dynamically, it would all be handled through the trait.

Then, I guess the other question, is where in the API would it get specified whether you wanted to use the pull up or pull down resistor?

On the comments on sealed traits and singletons, I tested and the compiler does NOT accept two instances of that same GPIO.

error[E0382]: use of moved value: `gpios.gpio2`
  --> examples\bitbang.rs:91:31
   |
90 |     let mut sys_led = gpios.gpio2.into_push_pull_output();
   |                       ----------- value moved here
91 |     let mut button_with_led = gpios.gpio2.into_push_pull_output();
   |                               ^^^^^^^^^^^ value used here after move
   |
   = note: move occurs because `gpios.gpio2` has type `esp32_hal::gpio::Gpio2<esp32_hal::gpio::Input<esp32_hal::gpio::PullDown>>`, which does not implement the `Copy` trait

Anyways, thanks again for your great post. It really helped me.

[icewind1991]

Note that my experience mostly comes from working on the esp8266-hal, which has some hardware difference from the esp32 (i.e. no pull down input, only pull up)

would there need to be another trait to switch directions

No new traits should be needed I think, and instead you ask for a pin that is bi-directional. In most cases you'll want libraries to only depend on the traits from embedded_hal so your library is generic over the hardware it runs on.

how would you specify whether you wanted to use the pull up, or pull down resistor for the GPIO?

Generally, a library shouldn't need to care about whether or no an input is pull down or up, it's up to the person adapting the library for a specific hardware setup to figure out how things are best wired up.

If I understand you correctly:

• if the pin was in a pull down mode, writing set_low(), would effectively put the pin into "read" state.
• if the pin was in pull up mode, calling set_high() would effectively put the pin into a "read" state.

From how I understood things from looking into other hal implementations, Output<OpenDrain> can be used for both output and input (although esp32-hal currently doesn't implement InputPin for it, so that would need to be added), if the Output<OpenDrain> is set to low, then it's left floating and another device attached to it can set it to high.

And the if I wanted different pin configurations, dynamically, it would all be handled through the trait.

Do you have an example of when a library would need to change the pin configuration, also note that which pins can be configured for what functions is very hardware dependant.

On a more general note, one of the reasons embedded-hal implementations generally use the "one struct per pin" instead of a generic pin struct is that this way the pin abstraction becomes zero cost. There is no need to keep a u8 around for each pin, or lookup how to configure a pin in an array. The compiler can statically inline everything.

I hope that this background somewhat explains why the hal ergonomics are the way they are.

[YonasJ]

Well, I have patched arjanmels version, which is based of the old code, and that seems to work for me.

I also tried the code in this pull request.

Not sure why, but in IntelliJ I get completion and type hints. But the much more macro intensive version in the other one that arjanmels is working on (based on the base here) doesn't get the completions or the type hints. So that is a plus for this approach.

I would be willing to merge arjanmels changes into a new version of this pull request, if that was viewed as helpful. This one is missing all of the ability to output.

[YonasJ]

Not sure where this discussion can happen, but I think that the current API is too hard to use.

I understand that it is nice feature that the rust compiler can track the uses of the GPIO pins as resources, and make sure they are never used twice.

However, consider that app developers have an easy time keeping track of what pin they are using for what. I have a text file with a list of pins and what each does. If I used a wrong one, I would know immediately - it wouldn't be some bug to find down the line. So I see the benefit of the compiler detecting duplicate usages between small and tiny for me.

However, I see the cost as very high:

1. The esp32-hal/gpio has so many macros and types it's super hard to figure out/edit/refactor.
2. Because of the macros, working with GPIO's mostly breaks the IDE. In almost every case, you don't end up using the generic type that is returned but instead another generic type, which you only use to get access to the trait.
3. When working with the GPIO's, the types you are using are never the types you are using, it's all a complicated generic, to in the IDE control clicking just takes you to the top level macro. And per 1, it's super hard to figure out what happens from there.
4. People keep editing the GPIO file, there seems to be 4 versions - but they are all incompatible because each change necessitates changing all the macros to include the new information they wanted. Rather than just adding a bit to the bottom.
5. The added weight of all the templates needs to be carried around to all the final code that uses it. Consider the code I am working on. I bold all the extra code that is created because of these extra templates. And I italic bold everything that's basically the same, just harder to understand.

struct Sync<OP>### EXTRA TYPE PARAM
where### EXTRA TYPE PARAM
    OP: OutputPin### EXTRA TYPE PARAM
{
    sync_count:u8,
    next_state:StateHandlerFunctionPtr<OP>  ### TYPE PARAM MAKE CODE MORE OPAQUE
}

pub struct PjonStateMachine<OP> ### EXTRA TYPE PARAM
where ### EXTRA TYPE PARAM
    OP: OutputPin ### EXTRA TYPE PARAM
{
    ...
    sync: Sync<OP>,
    frame: Frame,
    read: ReadByte<OP>,
    ---
    pub run: StateHandlerFunctionPtr<OP>,

    // Debug pins.
    pub pin21:OP, ### TYPE PARAM MAKE CODE MORE OPAQUE
    pub pin19:OP,### TYPE PARAM MAKE CODE MORE OPAQUE
    pub pin18:OP,### TYPE PARAM MAKE CODE MORE OPAQUE
    pub pin15:OP,### TYPE PARAM MAKE CODE MORE OPAQUE

}

### This triggers that most of my nested structs that store a function pointer need the type parameters passed into them. 
type StateHandlerFunctionPtr<OP> = fn(&mut PjonStateMachine<OP>, bool, u64) -> ();

...

impl <OP, E> PjonStateMachine<OP> ### EXTRA TYPE PARAMS x2
where  ### EXTRA TYPE PARAMS
    OP: OutputPin<Error=E>  ### EXTRA TYPE PARAMS
{
    pub fn new(pin21:OP,### TYPE PARAM MAKE CODE MORE OPAQUE
               pin19:OP,### TYPE PARAM MAKE CODE MORE OPAQUE
               pin18:OP,### TYPE PARAM MAKE CODE MORE OPAQUE
               pin15:OP,) -> Self {### TYPE PARAM MAKE CODE MORE OPAQUE

It seems to me that we we just made them as regular classes that take a pin in the constructor, then everything would be easier to develop in gpio, and much easier to use.

struct Gpio {
 pin:u8;
}
impl Gpio {
 fn to_input_pull_down(self) {
  // Set to input/pull down mode.

   return GpioInput::new(pin)
 }
 fn to_analog(self) {
  // Set to analog down mode.

   return GpioAnalog::new(pin)
 }
}
struct GpioInput {
 pin:u8;
}

impl InputPin for GpioInput {
// Nice and easy... No macros. Good command complete. Easy to edit different parts seperatly.
}

struct AnalogInput {
 pin:u8;
}

As a person starting out, it feels like just using GPIO's is banging my head against the wall, because there is so much type baggage that is hard to figure out. And this is normally so simple with the other embedded development systems.

Anyways, that's my 2c. I would be happy to help do some of the work.

[MabezDev]

I appreciate you expressing your thoughts, I will try to address them below.

consider that app developers have an easy time keeping track of what pin they are using for what

I do appreciate that, but this approach offers more than just tracking what pins are in use. It also tracks the state, input/output/alternate function mode, which I think will definitely help beginners understand how to initialize peripherals properly, instead of having to look it up in the reference manual etc.

1 The esp32-hal/gpio has so many macros and types it's super hard to figure out/edit/refactor. 2 Because of the macros, working with GPIO's mostly breaks the IDE. In almost every case, you don't end up using the generic type that is returned but instead another generic type, which you only use to get access to the trait. 3 When working with the GPIO's, the types you are using are never the types you are using, it's all a complicated generic, to in the IDE control clicking just takes you to the top level macro. And per 1, it's super hard to figure out what happens from there.

I'm going to bundle the first three points together, as they're all kind of related. First of, I would agree, the macros add another layer of indirection which can make it difficult to navigate the code, though this can be easily overcome via the cargo expand tool. I can understand the annoyance of IDE related stuff not working, but if I'm being brutally honest, is that the fault of this crate, or the IDE's in general? I would argue the IDE's in general. Rust has always had a pretty meh IDE story, which is getting better, but I don't see adding a runtime cost, or changing a working solution to 'satisfy' the IDE's as a particularly good idea.

People keep editing the GPIO file, there seems to be 4 versions - but they are all incompatible because each change necessitates changing all the macros to include the new information they wanted. Rather than just adding a bit to the bottom.

I am slightly confused by this statement, AFAIK this is the only PR modifying GPIO stuff? Perhaps you could elaborate?

The added weight of all the templates needs to be carried around to all the final code that uses it. Consider the code I am working on. I bold all the extra code that is created because of these extra templates. And I italic bold everything that's basically the same, just harder to understand.

Again, slightly confused by this section, are you having issues with the generic types? You can use the concrete types to remove the type paramters, but most libraries don't because that defeats the point of the embedded hal (ie. its no longer target agnostic).

[YonasJ]

I actually wrote that a little early. Now I have it all compiling and running. But there were even more generics I had to add: Since there are 4 pins I am using, I had to change OP in every place to OP1, OP2, OP3, OP4.

I am just using OP2-4 for debug, so once I get it working I will remove all those and be able to go back to just OP1.

So my code actually looks more like this:

impl <OP1,OP2,OP3,OP4, E> PjonStateMachine<OP1,OP2,OP3,OP4>
where
    OP1: OutputPin<Error=E>,
    OP2: OutputPin<Error=E>,
    OP3: OutputPin<Error=E>,
    OP4: OutputPin<Error=E>

[YonasJ]

I appreciate you expressing your thoughts, I will try to address them below.

Thanks! I will put more clarifications in below.

consider that app developers have an easy time keeping track of what pin they are using for what I do appreciate that, but this approach offers more than just tracking what pins are in use. It also tracks the state, input/output/alternate function mode, which I think will definitely help beginners understand how to initialize peripherals properly, instead of having to look it up in the reference manual etc.

1 The esp32-hal/gpio has so many macros and types it's super hard to figure out/edit/refactor. 2 Because of the macros, working with GPIO's mostly breaks the IDE. In almost every case, you don't end up using the generic type that is returned but instead another generic type, which you only use to get access to the trait. 3 When working with the GPIO's, the types you are using are never the types you are using, it's all a complicated generic, to in the IDE control clicking just takes you to the top level macro. And per 1, it's super hard to figure out what happens from there.

I'm going to bundle the first three points together, as they're all kind of related. First of, I would agree, the macros add another layer of indirection which can make it difficult to navigate the code, though this can be easily overcome via the cargo expand tool. I can understand the annoyance of IDE related stuff not working, but if I'm being brutally honest, is that the fault of this crate, or the IDE's in general? I would argue the IDE's in general. Rust has always had a pretty meh IDE story, which is getting better, but I don't see adding a runtime cost, or changing a working solution to 'satisfy' the IDE's as a particularly good idea.

I think that this is partially an IDE issue. When I enter the actual type manually the command completion will work on the next line. However, I will never actually use that type in my code, as I want my code to work for multiple pins. So I just carry it around as a template type. And IntelliJ just seems to give up. I think it is expanding the macros correctly as I can use the variable when I tell it the type as I do below. It just can't seem to figure out the type to offer completions for.

    let x:esp32_hal::gpio::Gpio15<esp32_hal::gpio::InputOutput<esp32_hal::gpio::OpenDrain>> = gpios.gpio15.into_open_drain_output();

    x.<<COMMAND COMPLETE WORKS>>

But I don't think it's only an IDE issue.

What when I click through on any of the types or function, it just takes me to the macro definition, and if it did take me to the correct part of the marco, that may not help much, as it's pretty hard to see what is happening in those macros.

People keep editing the GPIO file, there seems to be 4 versions - but they are all incompatible because each change necessitates changing all the macros to include the new information they wanted. Rather than just adding a bit to the bottom.

I am slightly confused by this statement, AFAIK this is the only PR modifying GPIO stuff? Perhaps you could elaborate?

Different people are adding clock support, uart support, analog support. Each of these changes requires changes to those macros. I want gpio open drain, clock, interrupt and analog to digital support. But the different people working on adding those features have had to adjust those macros in different ways. So there is no easy way for me to stitch them together

The added weight of all the templates needs to be carried around to all the final code that uses it. Consider the code I am working on. I bold all the extra code that is created because of these extra templates. And I italic bold everything that's basically the same, just harder to understand.

Again, slightly confused by this section, are you having issues with the generic types? You can use the concrete types to remove the type parameters, but most libraries don't because that defeats the point of the embedded hal (ie. its no longer target agnostic).

Here I am saying that I have to create generic types and pass them along to every function and structure that may use the gpio pin. Every type and function I want to then pass the pin to now needs to be generic, unless I input the concrete type.

So, in essence, to pass a GPIO open drain around or store it in a function I need to pass the type by generic, and the value normally.

I feel the costs outweigh the benefits. Especially on the work of understanding and passing around all the generics.

When you say it would be easier for a beginner, I don't see that. For a beginner the API would be essentially the same, except with 2 differences.

1. No need to use generics all the time, most things would have concrete types.
2. You would be able to ask a PushPull Gpio it's pin number.
3. Their command complete would work.

The into stuff would still be there. You could create instances of the variables through gpio.gpio26.to_pull_down_input();

The serial peripheral would also get easier. Anybody creating a serial knows they need to pass in 2 pin numbers for in and out. This way they could say:

let inpin=1;
let outpin = 2;
let s:Serial = Serial::new(0/*uart0*/, inpin,outpin,xxx)

But with the generics, this becomes much harder. Now it is:

let inpin= gpios.gpio13.into_serial_rx_pin();  // No command complete to help here.
let outpin = gpios.gpio13.into_serial_tx_pin();
s:Serial<esp32_hal::gpio::Gpio15<esp32_hal::gpio::SerialTxPin>,esp32_hal::gpio::Gpio15<esp32_hal::gpio::SerialRxPin>> = serial.split.uart0.into_serial(inpin,outpin,xxx); // No command complete here 

And since there was no command complete working in their "meh" rust ide, this means the only way they can figure out about the .into_serial_rx_pin() method is to browse that very large macro or find a working example to start from.

I haven't had any issues over the years with re-using pins, though I have done it a few types when cut and pasting. But it's pretty obvious once I fire it up...

Anyways, those are just my 2c.

I am really hoping I can learn this and use rust for my project. It's a bit tough at the start as my C skills are pretty good, and I already know the Arduino APIs. So far, it has been tough going as nothing seems to work as I expect (setMode(pin,mode)/digitalWrite(pin,val), the IDE doesn't have completes a lot of the time, and the template errors can be a little confusing.

All that aside, keep up the good work on getting Rust going on the ESP32.

[jeandudey]

@YonasJ Using a macro is bad I agree with you, but sincerely it's the only way it can be done, I haven't updated this PR as I'm trying to get a working rustc compiler and verify that this actually works.

Also there are some things that can be fixed in the esp32 crate that can be done so this PR gets easier and more clean as I stated in the TODO on the code.

The main reason we have "type states" is to be type safe.

The serial peripheral would also get easier. Anybody creating a serial knows they need to pass in 2 pin numbers for in and out. This way they could say:

The reason for not using numbers directly is because not all output pins can be used for UART and thus we avoid this problem. It can get better if we used this type instead of writing more generics (which tend to slow down compilation).

[YonasJ]

@YonasJ Using a macro is bad I agree with you, but sincerely it's the only way it can be done, I haven't updated this PR as I'm trying to get a working rustc compiler and verify that this actually works.

Also there are some things that can be fixed in the esp32 crate that can be done so this PR gets easier and more clean as I stated in the TODO on the code.

Yes, I had looked at making the macros a little more straight forwards, and I realized I could make some other lookup macros like this:

macro_rules! get_func_in_sel_cfg {
    ($gpio:expr, pin0) => { $gpio.func0_in_sel_cfg };
    ($gpio:expr, pin1) => { $gpio.func1_in_sel_cfg };
...
    ($gpio:expr, pin39) => { $gpio.func39_in_sel_cfg };
}

And then the macro's them selves, start to look a little different (maybe a bit worse):

So this:

             gpio.$gsfIn.modify(|_, w| unsafe { w.bits(0x100) });

Could turns into this:

             get_func_in_sel_cfg!(gpio, $pin).modify(|_, w| unsafe { w.bits(0x100) });

With some context it could look more like this.

        impl<MODE> $pxi<MODE> {
            pub fn into_pull_down(self) -> $pxi<Input<PullDown>> {
                let gpio = unsafe{ &*GPIO::ptr() };
                let iomux = unsafe{ &*IO_MUX::ptr() };
                self.disable_analog();

                gpio.$en.modify(|_, w| unsafe  { w.bits(0x1 << $i) });
**             get_func_in_sel_cfg!(gpio, $pin).modify(|_, w| unsafe { w.bits(0x100) });

                iomux.$iomux.modify(|_, w| unsafe { w.mcu_sel().bits(0b10) });
                iomux.$iomux.modify(|_, w| w.fun_ie().set_bit());
                iomux.$iomux.modify(|_, w| w.fun_wpd().set_bit()); // set pull down
                iomux.$iomux.modify(|_, w| w.fun_wpu().clear_bit()); // clear pull up.
                $pxi { _mode: PhantomData }
            }

This could also be done for $iomux and $en.

This way the macro's could be refactored to more like:

impl_gpios! {
    enable_w1ts, out_w1ts, out_w1tc, in_, in_data, [
        Gpio0: ( 0,  pin0, impl_gpio_rw, impl_has_adc, impl_has_touch),
        Gpio1: ( 1,  pin1, impl_gpio_rw, impl_no_adc,  impl_no_touch),
        Gpio2: ( 2,  pin2, impl_gpio_rw, impl_has_adc, impl_has_touch),
        Gpio3: ( 3,  pin3, impl_gpio_rw, impl_no_adc,  impl_no_touch),

Which would help address a lot of the readability of the macros.

The main reason we have "type states" is to be type safe.

The serial peripheral would also get easier. Anybody creating a serial knows they need to pass in 2 pin numbers for in and out. This way they could say:

The reason for not using numbers directly is because not all output pins can be used for UART and thus we avoid this problem. It can get better if we used this type instead of writing more generics (which tend to slow down compilation).

In the UART constructor, a check that the pins are valid, would be equally good for the end user. And less work as they wouldn't have to burden their code with as many generic types.

[YonasJ]

Hi,

I can see that folks have preference for the generic types way of doing it. So I went to thinking if there was a way to have it work better with the ide.

I did a little tinkering around, and discovered that with some re-writing it would be possible to address a lot of my concerns (ide, difficulty understanding the code) while maintaining the use of generics.

What could happen is that the first level of the macro could be unrolled, and type aliases could be used. Then with-in the unrolled top-level, macros could be used to expand the capabilities. This would mean that ide clicking on the base type would show the options and documentation, and ide clicking on the functions take you to the area with functionality you are interested in. So that would mean code like this:

pub struct Gpio1<MODE> {
    _mode: PhantomData<MODE>,
}

impl Gpio1<MODE> {
  impl_to_pull_up!(...);
  impl_to_pull_down!(...);
  impl_to_push_pull!(...);
  impl_timer!(...);
}
impl_Floating(...);
impl_PullDown(...);
impl_PullUp(...);
impl_OpenDrain(...);

pub struct Gpio39<MODE> {
    _mode: PhantomData<MODE>,
}
impl Gpio39<MODE> {
  impl_to_pull_up!(...);
  impl_to_pull_down!(...);
  // read only so does not implement impl_to_push_pull!(...);
  impl_timer!(...);

}
impl_Floating(...);
impl_PullDown(...);
impl_PullUp(...);
// read only so does not implement impl_PushPull(...);

and then in macro_rules! OpenDrain a type alias to illustrate what the underlying types are and to provide better explanation of what they are so they can be used in code (i deduce these by assigning to a wrong type and looking at the compiler error):

type Goio1_OpenDrain = esp32_hal::gpio::Gpio1<esp32_hal::gpio::InputOutput<esp1_hal::gpio::OpenDrain>>;

If this is something you would support, I would be willing to make those code changes. I would also put in some comment and examples explaining the generics and how to use them.

Thoughts?