Note, that if the hold keyword argument is given at all then my implementation disables hold, makes any config changes and then applies the hold if hold=True. This allows you to do things like:

pin = Pin(NUMBER, Pin.OUT, value=1, hold=True)
...
pin.init(value=0, hold=True)

and have these always work – including the original config working on a watchdog/internal reset. Without this, one would have to explicitly disable the hold before doing anything else, i.e.:

pin = Pin(NUMBER, hold=False)
pin.init(Pin.OUT, value=1, hold=True)
...
pin.init(hold=False)
pin.init(value=0, hold=True)

which both looks clunky to me and feels like it'd be forgotten and result in subtle bugs creeping in.

I also fear crazy race conditions like a MicroPython task switch occurring between the line releasing the hold and the one changing the config and reapplying the hold, then that other task triggering a reset. This way isn't foolproof – it doesn't use a critical section – but it feels safer.

However, it still means that casual use of something like:

Pin(NUMBER, Pin.IN)

or:

pin.value(0)

on a held pin would be (silently) ignored.

I am basically saying that if you use the hold keyword, I will assume that you really meant this config to apply.

I'm also happy to put back in some kind of legacy compatibility for pull=Pin.PULL_HOLD if there's concern that this is being used in the wild for what it actually does (rather than what it's documented as doing).

Actually, I thought a bit more about this but I can't see a clean way of adding this compatibility behaviour that doesn't defeat the new hold keyword.

Making pull=Pin.PULL_HOLD the same as hold=True is easy, but the only way to currently disable the hold is to set pull to some other value, and having that imply hold=False is a bit messier. It would mean that:

Pin(NUMBER, Pin.IN, pull=Pin.PULL_UP)

would reconfigure a held pin, but:

Pin(NUMBER, Pin.IN)

would be ignored.

Useful to note: iirc it is possible (and IDF docs seem to confirm) to change the pin configuration on a held pin, then call gpio_hold_dis(), which will cause the configured values to be applied immediately.

So there is no need to call gpio_hold_dis() in the init helper before configuring the pin; we can do so at the end just before (re-)enabling the hold.

It would probably also be very nice to add an extra method to allow setting the hold after the Pin is initialized, which would be more explicit than having to re-init the entire pin. So we could do things like:

# Initialize an (possibly already held) pin
mypin = Pin(0, Pin.OUT)

# Set it how I want it
mypin.value(1)

# Apply the value and hold again
mypin.hold(False)
mypin.hold(True)

As for PULL_HOLD, while I agree that removing the constant would be cleaner than just changing its behaviour, since at least we'll get a clear error message instead of silently changed behaviour, a quick search across all of GitHub shows PULL_HOLD being used, even in this repository.

So there is no need to call gpio_hold_dis() in the init helper before configuring the pin; we can do so at the end just before (re-)enabling the hold.

Ah! Good spot. I hadn't realised that the docs were implying this behaviour, but have been able to confirm it in my hardware. So I've simplified my code to move the disable back to the end. I'm still doing it even if hold is then to be immediately re-enabled so that any config changes will be latched.

It would probably also be very nice to add an extra method to allow setting the hold after the Pin is initialized, which would be more explicit than having to re-init the entire pin

The Pin.init() method doesn't reinitialise anything that you don't specify in the arguments so pin.init(hold=True) is equivalent to your pin.hold(False); pin.hold(True).

I thought about adding a method, but felt that being able to do everything in one hit felt more useful. One could obviously add a method anyway the same as pin.value(level) is (almost) equivalent to pin.init(value=level).

As for PULL_HOLD, while I agree that removing the constant would be cleaner than just changing its behaviour, since at least we'll get a clear error message instead of silently changed behaviour, a quick search across all of GitHub shows PULL_HOLD being used, even in this repository.

Yeah, looking at that code I can't see what purpose the use of PULL_HOLD is serving. Unless I'm missing something, it looks like it has been added based on the current documentation in the belief that it will affect deep-sleep current draw.

I'm gonna have a hunt around and see if I can find anyone using PULL_HOLD for its actual pad holding behaviour…

The Pin.init() method doesn't reinitialise anything that you don't specify in the arguments so pin.init(hold=True) is equivalent to your pin.hold(False); pin.hold(True).

My bad, I didn't read the read the code before commenting and blindly assumed there'd be a downside.

I thought about adding a method, but felt that being able to do everything in one hit felt more useful. One could obviously add a method anyway the same as pin.value(level) is (almost) equivalent to pin.init(value=level).

A better reason might be parity (in addition to .value()) with the .mode() and .pull() methods. Which I just noticed are not implemented in the esp32 port either, currently, but they do exist in other ports. So perhaps that's something for a separate change.

A better reason might be parity (in addition to .value()) with the .mode() and .pull() methods. Which I just noticed are not implemented in the esp32 port either, currently, but they do exist in other ports. So perhaps that's something for a separate change.

Interesting. I'd never noticed those methods before (I mainly use MicroPython on ESP32).

In the docs it says:

The following methods are not part of the core Pin API and only implemented on certain ports.

So does this indicate that they are counted as legacy? Or incomplete implementations in the other ports?

Other than endless forks of the tinypico.py module, I've only found a few other GitHub repos using PULL_HOLD. Most of these seem to be for the "save power in deep-sleep" reason, but one looks tantalisingly like it is trying to actually latch the pin state.

It looks like it wants to hold the level of a bunch of keypad row output pins through deep-sleep to allow the column input pins to wake the chip on an ext1 interrupt. However, in my own tests this won't work without that new method I've added to call gpio_deep_sleep_hold_en(). I'm tempted to track down the repo owner and ask them where they are getting to with this code…

Note to self: me2d13/cml@40dc5b0

I've pulled the drive strength commits out of here into #8313 in the hope that they can be accepted faster on their own.

I've put the PULL_HOLD constant back in so that existing code won't break. It will behave exactly the same as using pull=None, which is sufficient to disable RTC-pin pull resistors going into deep-sleep (see discussion in linked issue). I've also updated all of the associated documents.

I've tried to do all of this as separate commits that can be cherry-picked – e.g., the new power-saving documentation in 41c86f7 stands alone even if nothing else is accepted. However, my documentation of gpio_deep_sleep_hold() assumes that the pin hold keyword is there.

Rebased this on top of #8313 as the merge conflicts in my local repo were driving me crazy.

I'm now using this in the wild for a project that requires careful control over pins in deep-sleep to correctly disable external hardware and avoid unnecessary power drain.

Please rebase on latest master, to pick up the merge of the pin drive strength commits.

I've put the PULL_HOLD constant back in so that existing code won't break. It will behave exactly the same as using pull=None, which is sufficient to disable RTC-pin pull resistors going into deep-sleep (see discussion in linked issue). I've also updated all of the associated documents.

If PULL_HOLD doesn't work as expected (as it was previously documented in the quickref before this PR) then IMO we should just remove it completely. Then anyone using it will see their code break very visibly and they can investigate and fix it properly. If we leave PULL_HOLD and subtly change its behaviour then that is worse.

If PULL_HOLD doesn't work as expected (as it was previously documented in the quickref before this PR) then IMO we should just remove it completely. Then anyone using it will see their code break very visibly and they can investigate and fix it properly. If we leave PULL_HOLD and subtly change its behaviour then that is worse.

Fine with me! I've removed the constant completely.

I think I'd be happier if this code – and my thinking – could be reviewed by someone with knowledge of the ESP32 architecture. What I've documented seems to work for me in my current project, but I'm nervous about having missed something key in my understanding.

Quick search for PULL_HOLD on the repo to check for other uses:

• PULL_HOLD is actually listed in the machine.Pin docs, though no behaviour is given for it.
• The mimxrt port uses the PULL_HOLD constant. On that port it is used to enable the pad "keeper". As far as I can make out this is a mechanism for disabling the output driver while keeping a weak pull up/down to hold the output level. It seems to be a power-reduction device allowing the pad to be connected to a capacitive load (including a long wire I guess) that requires a fair bit of current when transitioning level, but none after that to "keep" it. PULL_HOLD makes some degree of sense here as configuration of the pad keepers is bound up in the pull configuration.
• Also, obviously, my patch here will break the tinypico.py module. Should I add a fix for that?

Thanks @jonathanhogg for all the effort on this, with nice clean commits and documentation.

I have re-read through this PR and issue #8283, and also looked again at the IDF, and I think your reasoning and implementation here are correct. The API here exposes the IDF functionality in a clean and orthogonal manner, you can enable and disable hold on the GPIO, and enable and disable the hold-during-deep-sleep.

Merged in 5887dfe through 3b9de19