Esp32-S2 I2C fixes

[dhalbert]

Fixes #3846.
Fixes #4046.

• Reverts I2C driver management to use i2c_driver_install() and i2c_driver_delete().
• Makes a change in i2c.c in the ESP-IDF about how a semaphore is deleted.
• Uses a mutex semaphore for the I2C lock, as recommended by @igrr in Crash when repeatedly creating and destroying busio.I2C object #3846 (comment).

Tagging @askpatrickw, @microdev1, @skieast for testing.

This is a draft because it depends on adafruit/esp-idf#1, which is not yet done. This build temporarily uses https://github.com/dhalbert/esp-idf/tree/circuitpython-i2c-cmd_mux-fix.

Tests that now work:

import board,busio

i2c = busio.I2C(board.SCL, board.SDA)
i2c.deinit()

import wifi

import time
import board
from busio import I2C

def scan(deinit=True):
    i2c = I2C(board.SCL, board.SDA)
    i2c.try_lock()
    results = i2c.scan()
    if deinit:
        i2c.deinit()
    return results

print(scan())
print(scan())
print(scan())

import board,busio,adafruit_sht31d

import wifi

i2c = busio.I2C(board.SCL, board.SDA)
i2c.deinit()

i2c = busio.I2C(board.SCL, board.SDA)
s = adafruit_sht31d.SHT31D(i2c)
print(s.temperature)

i2c.deinit()

[askpatrickw]

@dhalbert I have some time and will test this now

[askpatrickw]

I tested this using the CI artifacts on a MetroS2 an a Feather S2 PreRelease.
I ran dan's second code sample, and my own script using the BME_680. Both worked as expected.

I then copied over a script that connects to wifi and then prints the BME values.
It failed to safemode, when I disconnected USB and connected again the script worked.
I tried to reproduce that auto-load failure 10 times and could not.

Unless other people see that behavior, I would call this good.

[skieast]

tested on UM FeatherS2.
Script uses wifi, AdaFruit_requests, then setup two i2c devices on different busses. AHt20 and SGP30
Loop pinging google and reading both devices. Works. Interrupt with CTL-C, restart wih CTRL-D, works
From the repl create two i2c busses, i2c and i2c2. try_lock and unlock work on both.
Looks great.

[dhalbert]

Thank you very much for the testing! I've now moved this up to esp-idf merge commit (code is identical), and turned off draft status.

[dhalbert]

When #4378 is changed and merged, it will change from using bitbangio for 0, 1, or 2-byte writes to just 0-byte writes. I will try to test the equivalent of that change with some of the failing peripherals above.

[microdev1]

Thanks! @dhalbert.
I tested all three provided examples on my microS2 and they work flawlessly. Just one question...

[microdev1]

@dhalbert I think we can get the idf patch in v4.3 since its a pre-release at the moment.

[dhalbert]

@dhalbert I think we can get the idf patch in v4.3 since its a pre-release at the moment.

I agree, it would be good to PR upstream, but I am still somewhat bothered by the whole thing because I don't see why the IDF fix works. I did some more experiments yesterday and re-read a bunch of our code and their code. I came to no new conclusion. I haven't gotten feedback from igrr about it. Also I inquired with FreeRTOS: https://forums.freertos.org/t/vsemaphoredelete-immediately-after-xsemaphoretake/11956.

Ideally I would like to reproduce this in a simple ESP-IDF program, because they might ask for a test case, but I don't have any experience writing one. I could submit a PR and see what happens, but the fix seems a bit superstitious. There are other examples of Take followed by Delete in the ESP-IDF, mostly in test code. There are also a couple of examples of Take-Give-Delete. Do you have an intuition about this?

[microdev1]

Thanks! for making the changes.

[microdev1]

Do you have an intuition about this?

No clue at the moment, I'll try to replicate this behavior in an IDF program.

[dhalbert]

Failed build was a network problem. Merging.

[dhalbert]

No clue at the moment, I'll try to replicate this behavior in an IDF program.

When I look at the multiple threads running in a "crashed" program vs one running this fix, they look the same. In other words, it is not a hard crash, but some kind of loop or other problem. The clue with my simple tests above is that the REPL stops working, and eventually disconnects. So perhaps some program that does some USB serial activitiy would hang up. Perhaps also some wifi or I2C activity would fail - that might be easier to test.

[dhalbert]

This is the simplest example that reliably fails without the fix. It prints >>> after the import wifi but then the REPL hangs.

import board,busio

i2c = busio.I2C(board.SCL, board.SDA)
i2c.deinit()

import wifi

[microdev1]

It prints >>> after the import wifi but then the REPL hangs.

Yes, I get the same behavior... it hangs and no backtrace. From the debug print I see that wifi starts initializing but stops abruptly.
This also happens on V4.3. I am going to replicate the sequence of code that is executed with these commands.

[dhalbert]

So the essence of the above is (you can double check this):

// From busio.I2C().
i2c_driver_delete(NULL);   // does nothing
i2c_param_config();
i2c_driver_install();
// i2c.deinit().
i2c_driver_delete();

// Below from common_hal_wifi_init() and common_hal_wifi_radio_set_enabled().
esp_netif_init();               
esp_event_loop_create_default();
 xEventGroupCreateStatic();
esp_event_handler_instance_register();
esp_event_handler_instance_register();
esp_wifi_init();
esp_wifi_set_mode();
esp_wifi_start();

I tried moving the esp_event_loop_create_default() to earlier, doing it during port_init(), but that was worse.

[microdev1]

So the essence of the above is

Yup! that's it... you want to go ahead with the IDF program? If not I can run this sequence tomorrow.

[dhalbert]

I'll try to get the time; I'll let you know. I guess I can gut one of their examples and put that in.

[skieast]

Glad someone is also wondering why it works. I just got in and all my questions have been posed.

[dhalbert]

Moving this discussion to a new issue, #4401.