stm32-rs / stm32h7xx-hal

Peripheral access API for STM32H7 series microcontrollers
BSD Zero Clause License
215 stars 101 forks source link

Type-erased GPIO pin modes #132

Open mattico opened 4 years ago

mattico commented 4 years ago

I have a number of drivers where I need to change the mode of a GPIO pin during normal operation. However the type-state GPIO design makes this very annoying. For example:

enum ReadyPin {
    PullDown(gpioe::PE1<Input<PullDown>>),
    PullUp(gpioe::PE1<Input<PullUp>>),
}

pub struct Outgoing {
    i2c: i2c::I2c<pac::I2C1>,
    ready: ReadyPin,
    _aux: gpioe::PE0<Analog>,
}

I just want to store the pin in the driver struct but it's hard because its type changes all the time. I want to enable and disable interrupt generation on the pin but now I have to match every time I want to access the pin even though nothing changes but the type. It would be nice to have a GPIO pin type which erases the MODE type.

I'm also questioning the value of the MODE type-state altogether.

In user code the gpio pin is probably wrapped up in a driver which implements the desired behavior. Drivers in my experience either:

  1. Are built on top of embedded-hal, which doesn't care about our MODE types at all.
  2. Are built for a specific board which will use specific pins for specific peripherals. I found in this case it's cleaner to have your drivers just take Analog pins and do the configuration themselves to remove that pin-configuring clutter from main() and into the code that cares about it.

Am I missing something that these types help in user code?

I could only find one place in the HAL that actually used a mode type for anything. internal_pull_up(&mut self, bool) is only implemented for Output<OpenDrain> and Alternate<MODE>. I don't think it's particularly important that the type system prevents you from calling it on a push-pull output. All of the peripherals take Alternate<> pins which already erases the PushPull/OpenDrain and PullUp/PullDown/Floating types. Perhaps there are cases where the AF* typestates disambiguate things so those could be kept? If not, just have the peripherals do the pin configuration themselves and save us the trouble.

I don't have much hope that this will happen. It would be a huge breaking change in the HAL and a huge break with the other STM32 HALs, so I'll settle for adding a type-erased GPIO pin. With all the 1.0 discussions happening though, maybe I should make a last-chance push to remove the needless (?) complexity.

richardeoin commented 4 years ago

There is the downgrade method to remove the pin number from the type, but this isn't quite what you want.

The use case of the MODE generic type is when writing board or hardware specific drivers, and the abstraction level is lower than embedded-hal. You could write a driver for the CustomLEDDriver on your board, and specify a gpioi::PI<Output<PushPull>> type. Then whoever writes the top level program has to construct a Output<PushPull> type, and so ensures the pin is in the right state. However, I agree this use case doesn't appear very often in single-person hobby projects.

I think really your issue is that into_floating_input()/into_pull_down_input()/into_pull_up_input()/... use a fixed return type. The really obvious solution is just not to use those methods, but I guess you still want the HAL to provide you with the functionality.

A type like PXxUnchecked that implements all of floating_input()/pull_down_input()/pull_up_input()/... would be interesting to see, and could work alongside the existing types.