Bitbang i2c: switch scl to input for reading

[prplz]

Some ports can't read a digital io while it is in output mode, see in the sda_read function it is switched to input, read, then switched to output again.

I used PULL_UP because the sda_read function also uses PULL_UP, but I'm not sure if this is the right thing to do. Should it depend on CIRCUITPY_I2C_ALLOW_INTERNAL_PULL_UP? And if so, should check for pullups during construction like the busio version does?

[dhalbert]

Which ports can't read while in output mode? I'm guessing you encountered this in a particular situation.

[prplz]

Espressif, nrf, maybe stm and samd. I just found it while reading through the bitbang i2c source while trying to understand more about i2c.

[dhalbert]

Espressif, nrf, maybe stm and samd.

We have used bitbangio successfully on STM and SAMD, including with devices that need clock-stretching. So does it not work on Espressif? I mean, have you tested it?

Should it depend on CIRCUITPY_I2C_ALLOW_INTERNAL_PULL_UP?

This was added in #5237, due to a particular Espressif dev board that didn't include external pullups despite having on-board sensors. In general, the internal pullups are often marginal for I2C, or in cases like RP2040 (internal pullups are 60-80kohms) not really suitable at all.

I used PULL_UP because the sda_read function also uses PULL_UP, but I'm not sure if this is the right thing to do.

This may just be because it doesn't hurt to specify the internal pullups.

And if so, should check for pullups during construction like the busio version does?

That would be nice, but it needs to be done in a port-independent way.

[prplz]

We have used bitbangio successfully on STM and SAMD, including with devices that need clock-stretching. So does it not work on Espressif? I mean, have you tested it?

I don't have any devices that I know for sure need clock stretching, but I can get it to do a lot of weird stuff (on espressif and nrf). One example is if I were to ground SCL and do a scan, I get a list of 100+ addresses instead of the expected TimeoutError: Clock stretch too long that I get with this PR.

[tannewt]

This makes sense to me. I wouldn't expect the get_value to return the line state when doing output anyway. I'd expect the value we're set to output.

[dhalbert]

OK by me too. I just wanted to make sure someone had tested it on hardware.