Improve native wifi API compatibility

[anecdata]

• connect now matches (subset of) native wifi API, and deprecates the old secrets-dict-based API, but does still return the esp.status instead of the native wifi None and (usually) returns None.

• new ipv4_address matches native wifi API

• new connected matches native wifi API

This allows the following code, regardless of whether the board has native wifi, or an ESP32SPI (Airlift) co-processor:

radio.connect(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"))
radio.connected
radio.ipv4_address

esp32spi_simpletest.py works fine: it puts settings.toml credentials into a secrets dict, then passes the elements of that dict to the connect_AP() param fields. If we want to encourage the native methods, or deprecate the old methods someday, the examples could be changed.

[anecdata]

Tested on: Adafruit CircuitPython 9.1.0-beta.1-10-gc92bb9b3cc on 2024-04-25; FeatherS3 with ESP32S3

import time
import os
import board
import digitalio
import wifi
from adafruit_esp32spi import adafruit_esp32spi

time.sleep(3)

# native wifi
print("\nNative wifi...\n")

radio = wifi.radio
# radio.disconnect() not implemented; use radio.enabled = False or radio.stop_station()
print(f'{radio.connect(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"))=}')
print(f'{radio.connected=}')
print(f'{radio.ipv4_address=}')

# ESP32SPI
print("\nESP32SPI...\n")

spi = board.SPI()
esp32_cs = digitalio.DigitalInOut(board.D13)
esp32_reset = digitalio.DigitalInOut(board.D12)
esp32_ready = digitalio.DigitalInOut(board.D11)
radio = adafruit_esp32spi.ESP_SPIcontrol(spi, esp32_cs, esp32_ready, esp32_reset)

# new APIs
print(f'NEW {radio.connect(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"))=}')
print(f'{radio.connected=}')
print(f'{radio.ipv4_address=}')

print(f'\nradio.disconnect()=')
print(f'NEW {radio.connect(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"), timeout=10)=}')
print(f'{radio.connected=}')
print(f'{radio.ipv4_address=}')

# compatibility:
print(f'\nradio.disconnect()=')
print(f'ORIG {radio.connect_AP(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"))=}')
print(f'{radio.connect_AP(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"))=}')
print(f'{radio.is_connected=}')
print(f'{radio.ip_address=}')
print(f'{radio.pretty_ip(radio.ip_address)=}')
print(f'{radio.status=}')  # 3 == WL_CONNECTED

print(f'\nradio.disconnect()=')
print(f'ORIG {radio.connect_AP(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"), timeout_s=10)=}')
print(f'{radio.is_connected=}')
print(f'{radio.ip_address=}')
print(f'{radio.pretty_ip(radio.ip_address)=}')
print(f'{radio.status=}')  # 3 == WL_CONNECTED

# these should never disagree:
# esp.connected
# esp.is_connected
# esp.status == 3:  # "WL_CONNECTED"

code.py output:

Native wifi...

radio.connect(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"))=None
radio.connected=True
radio.ipv4_address=192.168.6.183

ESP32SPI...

NEW radio.connect(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"))=None
radio.connected=True
radio.ipv4_address=192.168.6.74

radio.disconnect()=
NEW radio.connect(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"), timeout=10)=None
radio.connected=True
radio.ipv4_address=192.168.6.74

radio.disconnect()=
ORIG radio.connect_AP(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"))=3
radio.connect_AP(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"))=3
radio.is_connected=True
radio.ip_address=bytearray(b'\xc0\xa8\x06J')
radio.pretty_ip(radio.ip_address)=192.168.6.74
radio.status=3

radio.disconnect()=
ORIG radio.connect_AP(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"), timeout_s=10)=3
radio.is_connected=True
radio.ip_address=bytearray(b'\xc0\xa8\x06J')
radio.pretty_ip(radio.ip_address)=192.168.6.74
radio.status=3

[anecdata]

It would grow during the overlap interval. Yes, there are a number of functions that could be normalized ...though not all params would necessarily match in either direction. When it gets into the socket stuff... that's a whole other ballgame.

[anecdata]

NINA could be a bottleneck for some missing functions (most NINA functions are by now exposed in ESP32SPI, but not 100%). For example, no stop_AP in NINA, but maybe could fudge it by setting an invalid AP and ignore the error (or just esp.reset) ...could get a little weird.

[justmobilize]

@dhalbert do you know if we added the wrapper if it would fit in all the places it's frozen?

[dhalbert]

@dhalbert do you know if we added the wrapper if it would fit in all the places it's frozen?

Yes, I'd be worried about that, so maybe we have to do it all at once. But we'd need to measure the sizes.

[dhalbert]

@anecdata Another thing I was thinking about was, for a transition period, making connect() take either a dict or SSID and password. It could look at the args and choose what to do. I'm not sure if this is worthwhile or not.

[anecdata]

for a transition period, making connect() take either a dict or SSID and password

I had thought about that, and it was my first inclination. Then it seemed like the other vestiges of the secrets dict had been removed from the API and settings.toml is k-->v only without dicts, so I just changed the function. But I don't have a strong feeling about it. If there are learn guides or examples out there, or people still using secrets, then maybe we don't cut the cord so fast.

[dhalbert]

I will take a look at the guides tomorrow and see how much needs to be changed.

[anecdata]

I did a quick Github search on the guides repo, and it looks pretty prevalent there. I should be able to work up a code option tomorrow for handling both cases, I've seen it in libraries for other situations.

[dhalbert]

I am willing to change all the guides examples coincident with the library release.

[justmobilize]

Hmmm, I'm thinking. Do we really want to name it radio? Would we do:

from adafruit_esp32spi import radio
??? = radio(spi_bus, esp32_cs, esp32_ready, esp32_reset)

or add a __init__.py with imports so we could do:

imoprt adafruit_esp32spi
radio = adafruit_esp32spi.radio(spi_bus, esp32_cs, esp32_ready, esp32_reset)

And this started as a thought if we want to rename adafruit_esp32spi_socketpool to just socketpool?

[anecdata]

Just in case we want to support secrets stragglers I tested this with and without the timeout kwarg, plus a few error scenarios (it's not committed to the pr at the moment):

    def connect(self, *args, timeout=10):
        """Connect to an access point with given name and password."""
        if len(args) == 2:
            ssid, password = args
        elif len(args) == 1:
            if isinstance(args[0], dict):  # secrets
                ssid, password = args[0]["ssid"], args[0]["password"]
            elif isinstance(args[0], str):  # open AP
                ssid, password = args[0], None
            else:
                raise ConnectionError("Invalid credentials format")
        else:
            raise ConnectionError("Invalid credentials format")
        self.connect_AP(ssid, password, timeout_s=timeout)

(connect_AP(self, ssid, password, timeout_s=10) does the heavy lifting of converting parameter formats, calling lower-level functions, and enforcing any timeout)

It handles all three cases:

• separate ssid and password strings passed in
• secrets dict passed in
• ssid string only passed in (open network)

[dhalbert]

I was also thinking whether we might just start a new library that uses the low-level SPI code here but has much more of a wifi.radio API. That would avoid issues about compatibility and incompatibility. It could be shipped as the frozen library in 9.2 or 10.0, say. It might be called Adafruit_CircuitPython_AirLift_WiFi or something. There is already a small Adafruit_CircuitPython_AirLift that just manages setup (choosing BLE or WiFi).

[anecdata]

Should I close this in favor of one for the alternatives for some future import radio-like_esp32spi?

[justmobilize]

I personally would love to see this go in as is, and then build a better one...

Happy to go update docs

[dhalbert]

Should I close this in favor of one for the alternatives for some future import radio-like_esp32spi?

No, I think we can do this as is. I will look tomorrow about how many guides need to be edited, and plan time tomorrow or soon after to merge this, push the release and then edit the guides immediately.

[justmobilize]

@dhalbert let me know how I can help.

@anecdata I will test tomorrow

[dhalbert]

OK, this is a problem: ESP32SPI is frozen into several boards: PyPortal, Titano, and MatrixPortal M4, and the i.MX 101x boards. Those boards need a frozen version because they don't have enough RAM to support a non-frozen version for other than small programs. If we change the API now, and change the Guide code, then the examples will no longer work on those boards with 9.0.x stable builds.

So I propose:

1. Include the backwards compatible API (with appropriate documentation), so that old code will work. Merge and release this any time between now and just before 9.1.0 final.
2. Just before releasing 9.1.0 final, update frozen libraries for 9.1.0 final with the upward compatible release. 9.1.0 will then work with the secrets dict and with the new API.
3. After 9.1.0 final is released, we can change the Guide code to use the separate SSID and password arguments.
4. Sometime after that, maybe 9.2 or 10.0, remove the backward compatibility.

[justmobilize]

So like this:

    def connect(self, ssid, password, timeout=10):
        """Connect to an access point with given name and password."""
        # previous versions of connect were passed in a secrets dict.
        # temporarily continue supporting this
        if isinstance(ssid, dict):
            ssid = ssid["ssid"]
            password = ssid["password"]

        self.connect_AP(ssid, password, timeout_s=timeout)

[dhalbert]

So like this:

@justmobilize I think your version would require a second argument in the case when just a dict was passed in.

See https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/pull/199#issuecomment-2088759068. I think that could also be done with ssid, password=None instead of *args

[justmobilize]

Also, finished testing. The only thing I might add is updating mac address:

>>> r1.mac_address
b'|\xdf\xa1\xf8\xfa\xf8'
>>> r2.MAC_address
bytearray(b'\\\xc1V\x12\xcf\xa4')
>>> r2.MAC_address_actual
<reversed>
>>> bytes(r2.MAC_address_actual)
b'\xa4\xcf\x12V\xc1\\'

and so would do:

    @property
    def mac_address(self):
        return bytes(reversed(self.MAC_address))

[justmobilize]

So like this:

@justmobilize I think your version would require a second argument in the case when just a dict was passed in.

See #199 (comment). I think that could also be done with ssid, password=None instead of *args

True, I like password=None, this allows new code to specify the args and pass them in named.

[anecdata]

OK, showing my ignorance, but if password is a kwarg in the function def, will it work if passed as a non-keyword arg?

edit:: n/m, TIL

>>> def myfunc(a, b=None, t=10):
...     print(a, b, t)
>>> myfunc("ssid", "password", 10)
ssid password 10

[dhalbert]

OK, showing my ignorance, but if password is a kwarg in the function def, will it work if passed as a non-keyword arg?

Is this what you mean? (EDIT: fixed example)

>>> def connect(ssid, password=None, timeout=10):
...     print(ssid, password, timeout)
... 
>>> connect("myssid")
myssid None 10
>>> connect("myssid", "mypassword")
myssid mypassword 10
>>> connect()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: connect() missing 1 required positional argument: 'ssid'

[anecdata]

Yes, forgot about the whole * and kw-only thing, thanks.

[dhalbert]

@anecdata, I fixed my example above. timeout should not be keyword only in that signature, but yes, it's still fine.

[anecdata]

Shouldn't it be keyword-only? https://docs.circuitpython.org/en/latest/shared-bindings/wifi/index.html#wifi.Radio.connect

[anecdata]

Oh, it may break backwards compatibility if it fully matches native.

[dhalbert]

Oh, it may break backwards compatibility if it fully matches native.

We can fix that when we remove the dict capability and rewrite the examples and Guide code.

[anecdata]

OK, this is odd, I'm almost certain it used to work:

print(f'MAC (actual)    {radio.MAC_address_actual=}')

prints:

MAC (actual)    radio.MAC_address_actual=<reversed>

edit: probably nobody noticed since it's almost always iterated and converted to hex, I'll fix that while I'm in there

[anecdata]

(I fixed radio.MAC_address_actual to return a bytearray instead of a reversed iterator. Shouldn't affect anything since a bytearray can be iterated just like the reversed iterator.)

[anecdata]

@justmobilize would you mind re-testing in case I missed something?

[justmobilize]

Yup. On it

[justmobilize]

@anecdata looks good. Added this test to what you have above:

radio.connect({"ssid": os.getenv("WIFI_SSID"), "password": os.getenv("WIFI_PASSWORD")})

[anecdata]

Yes, good point, I didn't update the original posted test code when we changed the direction. Also tested a local open network: radio.connect("open") radio.connect("open", timeout=10)