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

Inverting GPIOs #178

Closed
har7an opened this issue Sep 2, 2022 · 4 comments · Fixed by #912
Closed

Inverting GPIOs #178

har7an opened this issue Sep 2, 2022 · 4 comments · Fixed by #912
Labels
peripheral:gpio GPIO peripheral

Comments

@har7an
Copy link
Contributor

har7an commented Sep 2, 2022

Why?

Today I came across a SPI device that has an inverted CS line. With the current HAL implementation, this situation cannot easily be handled: The SpiDevice trait as implemented on SpiBusDevice requires an IO that implements the OutputPin trait from the esp-hal, hence something like inverted-pin doesn't work. This leaves me with having to forego SpiBusDevice in favor of manually handling the CS line which isn't nice.

What to do about it?

The esp32 has hardware support for inverting both input and output GPIOs in hardware, via the GPIO_FUNCx_(OUT|IN)_INV_SEL bit.
Hence, I think it would be possible to add GPIO inversion at zero cost by extending the Pin trait from esp-hal. What are your thoughts on this? Would you accept a PR that implements this?

@jessebraham
Copy link
Member

I think in general if the hardware supports it we will accept PRs, so go nuts :)

Just as a note I am in the process of completely rewriting the GPIO module, however I'm not sure how long this will take so you can likely implement this and get it merged before I'm done. I will of course propagate your changes to my rewrite if this does happen.

@bjoernQ
Copy link
Contributor

bjoernQ commented Feb 17, 2023

We have everything needed to do that in

  • fn connect_input_to_peripheral_with_options(
    &mut self,
    signal: InputSignal,
    invert: bool,
    force_via_gpio_mux: bool,
    ) -> &mut Self;
  • fn connect_peripheral_to_output_with_options(
    &mut self,
    signal: OutputSignal,
    invert: bool,
    invert_enable: bool,
    enable_from_gpio: bool,
    force_via_gpio_mux: bool,
    ) -> &mut Self;

Only thing we are missing is a way to expose it in the API. That is everywhere we want to have this feature we need a special constructor for the peripheral driver that allows to specify if a pin should be inverted or not.

@georgik
Copy link

georgik commented May 22, 2023

@bjoernQ How we can invoke the inversion?

E.g. here it's used in SmartLED which should have inverted signal: https://github.com/georgik/esp32-buddy-rs/blob/feature/update-hal-2023-05/examples/rainbow.rs#L89

  // Configure RMT peripheral globally
    let pulse = PulseControl::new(
        peripherals.RMT,
        &mut system.peripheral_clock_control,
        ClockSource::APB,
        0,
        0,
        0,
    )
    .unwrap();


    let mut led = <smartLedAdapter!(1)>::new(pulse.channel1, io.pins.gpio25);

Should be just GPIO25 womehow wrapped or do we need to make whole driver inverted?

@bjoernQ
Copy link
Contributor

bjoernQ commented May 22, 2023

Pins get passed into the drivers and the drivers configure the pins accordingly (e.g. here

// Configure Pin as output anc connect to signal
pin.set_to_push_pull_output()
.connect_peripheral_to_output($output_signal);
)

In this case we would need to call connect_peripheral_to_output_with_options instead and set the options to invert the pin.

This then needs to be reflected in the user-facing API (e.g. alternative constructors with an option to invert a pin)

@jessebraham jessebraham added the peripheral:gpio GPIO peripheral label Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
peripheral:gpio GPIO peripheral
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants