Docstrings for GPIO/PinDriver #473
Conversation
Signed-off-by: Iván Sánchez Ortega <[email protected]>
IvanSanchez
force-pushed
the
gpio-docstrings
branch
from
5cd2de9
to
06778a6
Compare
src/gpio.rs
Outdated
| //! | ||
| //! Interface for the input/output pins. | ||
| //! | ||
| //! `Gpio1` through `Gpio39` represent the *physical* pins of the ESP chip. |
src/gpio.rs
Outdated
| //! | ||
| //! The ESP architecture has a I/O multiplexer, which means that (almost) any | ||
| //! physical pin can be used for any logical function (i.e. GPIO, I2C, SPI, ADC, etc). | ||
| //! Even though it's possible to use a pin for several functions at once, this |
There was a problem hiding this comment.
Is that really possible? Even if it is possible, it is not possible with the safe API of esp-idf-hal for sure, as the pin peripheral (what you call above "physical" pins) can only be used by one driver at a time.
There was a problem hiding this comment.
From what I'm reading at https://hackaday.io/page/21183-hacking-esp32-multiplexing-i2c , it seems that it's possible to use the same pin peripheral ("physical") as the CLK pin in two different I2C interfaces at once.
Which means one of the I2C buses is working as expected, and the other one is receiving all ones (or all 0xFFs) on the SDA pin, which is innocuous enough.
There was a problem hiding this comment.
See also the notes on pages 51 and 53 of https://www.espressif.com/sites/default/files/documentation/esp32_technical_reference_manual_en.pdf#iomuxgpio :
One input pad can be connected to multiple input_signals.
and
The output signal from a single peripheral can be sent to multiple pads simultaneously.
There was a problem hiding this comment.
Interesting. How would you do that with the ESP IDF C API?
src/gpio.rs
Outdated
| //! | ||
| //! Interface for the input/output pins. | ||
| //! | ||
| //! `Gpio1` through `Gpio39` represent the *physical* pins of the ESP chip. |
There was a problem hiding this comment.
Also maybe "... represent the physical pins or in other words - the pins peripherals of the ESP chip".
There was a problem hiding this comment.
Espressif's documentation uses the "physical pin" nomenclature, so I believe it's important to keep it. I'll rephrase this part.
There was a problem hiding this comment.
I never said to remove the physical notion, but to add to it that we mean a peripheral, which is the notion we use throughout the hal.
src/gpio.rs
Outdated
| //! `Gpio1` through `Gpio39` represent the *physical* pins of the ESP chip. | ||
| //! | ||
| //! *Logical* pins (compatible with `embedded_hal::digital::InputPin`/`OutputPin`) | ||
| //! are implemented through `PinDriver`. |
There was a problem hiding this comment.
Maybe add: "... just like other embedded_hal traits are implemented on their corresponding *Driver structs and not on the peripherals themselves"
There was a problem hiding this comment.
You can also sprinkle a mention of the embedded_hal_async pin traits as these are also supported by PinDriver.
src/gpio.rs
Outdated
| //! | ||
| //! // The logical pin implements some embedded_hal traits, so it can | ||
| //! // be used in crates that rely on those traits, e.g.: | ||
| //! use one_wire_bus::OneWire; |
There was a problem hiding this comment.
Can you come up with a different example from one_wire_bus? The reason why esp-idf-hal got a onewire driver just recently is to avoid the generic busy-looping one_wire_bus & similar drivers, as they simply do not work reliably. Under ESP IDF at least. So it might be weird to suggest in an example an API which we would otherwise advise against using.
|
@IvanSanchez Thanks for contributing. If you could address my feedback. In general, I do understand that in older hals there was a mix-up between the notion of pin peripheral and the notion of a pin driver - yet - in all "more modern" hals supported by the It is just that these other hals tend to come up with "inventive" names for their drivers (which I definitely dislike) while |
Signed-off-by: Iván Sánchez Ortega <[email protected]>
src/gpio.rs
Outdated
| @@ -2,17 +2,29 @@ | |||
| //! | |||
| //! Interface for the input/output pins. | |||
| //! | |||
| //! `Gpio1` through `Gpio39` represent the *physical* pins of the ESP chip. | |||
| //! `Gpio1` through `GpioNN` represent the pin peripherals of the ESP chip. You | |||
There was a problem hiding this comment.
Nitpick: gpio's start at gpio0
src/gpio.rs
Outdated
| //! use a pin for several functions at once, this should be avoided. In | ||
| //! practice, `esp-idf-hal` should prevent most instances of pin reuse. | ||
| //! | ||
| //! If you *really* need to mux I/O pins, you might need to drop any function |
There was a problem hiding this comment.
I think this part would be confusing for people as it doesn't cleary describe what you mean here.
For example most of our logical drivers accept also pin references. E.g you can give a PinDriver a &mut gpio0 and if you drop the PinDriver you still have access to gpio0 instance. No need for fancy Mutex etc.
let mut foo = per.pins.gpio0;
let driver = PinDriver::input(&mut foo);
drop(driver);
let driver2 = PinDriver::input(&mut foo);Signed-off-by: Iván Sánchez Ortega <[email protected]>
A bit of documentation for the GPIO module. This should help people who (like me) struggle a bit to understand why a
esp_idf_hal::gpio::AnyInputPinis not anembedded_hal::digital::Input.