PinDriver::output causes gpio to unexpectedly go high about esp-idf-hal HOT 12 CLOSED

There might not always be a previous state however. Personally I would be for having a parameter as part of initializing the pin that requires defining the "reset" and "drop" direction. That is, if dropping a pin/resetting a pin forces the pin to go high or low, we should be able to pick which direction it goes.

from esp-idf-hal.

We close the issue as soon as it hits master, or shortly after. That's what a lot of other projects do as well, with GH's simplistic issue tracking system.

As for ETA - perhaps by end of this month latest, if not earlier. Is there any reason, if you really need this ASAP, not to use [patch.crates-io] as most folks do?

from esp-idf-hal.

if you suspect that the drop call is the culprit, can you test it out what happens if its not called? you can simply use [patch-crates.io] in your Cargo.toml to point to a modified copy of esp-idf-hal

from esp-idf-hal.

commenting out drop still allows it to compile, but it quite thoroughly breaks gpio it seems.

from esp-idf-hal.

Alright, seems I've run this down, summary below:

I am using esp-idf v5.1.1
It looks like the PinDriver, as part of setting up an output, calls drop() which itself calls gpio_reset_pin() from esp-idf.
This function in esp-idf (located here) cause the pin to be pulled up as part of the reset, causing an undesirable output which can cause some severe damage depending on what the gpio is connected to.

As a short-term for my specific use-case, I simply replaced this with a pull-down instead of a pull-up, but think that a better long-term solution would be to replace this drop(self); in the PinDriver with a gpio configuration which either doesn't pull the pin or allows users to specify which direction to pull the pin.

from esp-idf-hal.

Alright, seems I've run this down, summary below:

I am using esp-idf v5.1.1 It looks like the PinDriver, as part of setting up an output, calls drop() which itself calls gpio_reset_pin() from esp-idf. This function in esp-idf (located here) cause the pin to be pulled up as part of the reset, causing an undesirable output which can cause some severe damage depending on what the gpio is connected to.

As a short-term for my specific use-case, I simply replaced this with a pull-down instead of a pull-up, but think that a better long-term solution would be to replace this drop(self); in the PinDriver with a gpio configuration which either doesn't pull the pin or allows users to specify which direction to pull the pin.

What you suggest would be a half solution only, as if you decide to drop the driver itself, you'll get the very same glitch.
So I wonder, maybe we need a different reset routine altogether?

from esp-idf-hal.

well @ivmarkov i already addressed that in the linked PR?

from esp-idf-hal.

So I wonder, maybe we need a different reset routine altogether?

What about saving the previous pull-up/down state as a private field of PinDriver on initialization, and restoring that state on drop?

I think any unexpected behavior on PinDriver creation could be avoided by making the user of the API choose the pull mode explicitly, e.g. by having a required Pull argument in the input and output methods, or by giving PinDriver a generic argument (with no default value) to the same effect.

The current API enforces at compile time that .set_pull() can only be called on pins that do actually have internal pull resistors available, which is good. I'm not sure whether it'd currently be possible to use the type system so that the user
a) must choose a pull mode for those pins that support one, and
b) must not choose one for the pins that don't, or alternatively, must explicitly "choose" Pull::Floating.

from esp-idf-hal.

So it seems this was merged but we are still waiting for it to be released before updating via crates.io? Would pointing to master right now include the fix?

from esp-idf-hal.

Of course?

from esp-idf-hal.

I guess I am confused then.

If master is treated as a prerelease branch, why is this issue closed? Shouldn't it remain open until the next release since the issue is still present in the latest release of the crate? Just trying to understand dev life cycle here.

Is there an ETA on the next release?

from esp-idf-hal.

Cool, thanks for the insight!

from esp-idf-hal.