Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ESP32SPI support #32

Merged
merged 4 commits into from
May 8, 2024
Merged

Conversation

justmobilize
Copy link
Contributor

@justmobilize justmobilize commented May 1, 2024

Add support for ESP32SPI, which would allow this to be used with:

  • Onboard WiFi
  • WIZNET5k
  • ESP32SPI

Merged code from: https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/blob/main/examples/esp32spi_udp_client.py

@justmobilize justmobilize marked this pull request as ready for review May 1, 2024 04:52
@justmobilize
Copy link
Contributor Author

@anecdata and @dhalbert - simple update

Copy link
Member

@anecdata anecdata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! Tested on config with ESP32-S3 and ESP32SPI. Code looks fine, and the two components were each already reviewed when they were merged into their respective repos.

test code...
# also uses esp32spi pr#199 to simplify connect and ipv4_address
import time
import os
import board
import digitalio
import wifi
import adafruit_connection_manager
from adafruit_esp32spi import adafruit_esp32spi
import adafruit_ntp

radios = []
pools = []
connection_managers = []
ntps = []

# native wifi
radios.append(wifi.radio)

# esp32spi
spi = board.SPI()
esp32_cs = digitalio.DigitalInOut(board.D13)
esp32_reset = digitalio.DigitalInOut(board.D12)
esp32_ready = digitalio.DigitalInOut(board.D11)
radios.append(adafruit_esp32spi.ESP_SPIcontrol(spi, esp32_cs, esp32_ready, esp32_reset))

for radio in range(0, len(radios)):
    pools.append(adafruit_connection_manager.get_radio_socketpool(radios[radio]))
    connection_managers.append(adafruit_connection_manager.get_connection_manager(pools[radio]))
    radios[radio].connect(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"))
    ntps.append(adafruit_ntp.NTP(pools[radio], tz_offset=0))

while True:
    for radio in range(0, len(radios)):
        radios[radio].connect(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"))
        print(f'{radios[radio].ipv4_address} {ntps[radio].datetime}')
        time.sleep(1)

@anecdata
Copy link
Member

anecdata commented May 1, 2024

We may want to add this to the list, like WIZnet, where the init wouldn't necessarily require a network connection.

adafruit_ntp.py Outdated
self._server = server
self._port = port
self._packet = bytearray(48)
self._socket_address = self._pool.getaddrinfo(server, port)[0][4]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anecdata good call out on the init. I'll move this to the datetime property:

if self._socket_address is None:
   self._socket_address = self._pool.getaddrinfo(server, port)[0][4]

Will hold on other reviews first

@anecdata anecdata requested a review from a team May 1, 2024 19:36
@jepler
Copy link
Member

jepler commented May 1, 2024

It appears that the main change is to work around missing udp "sendto" support in wiznet. Did anyone investigate whether sendto (and any other missing functionality) can be added to wiznet esp32spi instead?

@jepler
Copy link
Member

jepler commented May 1, 2024

It appears that arduino's version of nina-fw supports socket_sendto but adafruit's fork does not.

if send can substitute for sendto, is there any reason to retain the two runtime branches (sendto & connect+send) rather than just switching to connect+send with a note that it's used for compatibility with nina-fw / esp32spi?

@justmobilize
Copy link
Contributor Author

@jepler I will try that. since sendto, basically just does that - it should work

@anecdata
Copy link
Member

anecdata commented May 1, 2024

My impression has been that connect isn't typically used for UDP. In this case it's an artifact of the non-standard way Arduino (NINA) handles sockets. Virtually none of the typical socket code sequences (tcp server/client, udp server/client - especially the servers) that runs on CPython, native wifi, or WIZnet, work in NINA. Maybe we can fudge around that. But yes, sendto is a bit of a red herring, just a convenient way for now to differentiate (although... does it break CPython?).

edit: looks like connect does work in CPython, either branch would work

@justmobilize
Copy link
Contributor Author

@jepler the native socket doesn't have recv, so either way we are doing special code per chip. This seems the most straightforward... Thoughts?

@jepler
Copy link
Member

jepler commented May 2, 2024

as the return value of recvfrom_into is ignored, can't recv_into be substituted?

@justmobilize
Copy link
Contributor Author

But the esp32spi only has recv...

@justmobilize
Copy link
Contributor Author

@jepler anything thoughts / recommendations?

@jepler
Copy link
Member

jepler commented May 3, 2024

@justmobilize
Copy link
Contributor Author

@jepler okay, I got it as close as I can. The only difference now is the connect method

@jepler
Copy link
Member

jepler commented May 6, 2024

Maybe #201 helps with the connect() difference

@justmobilize
Copy link
Contributor Author

@jepler that works

@justmobilize
Copy link
Contributor Author

Once ESP32SPI is released, I'll update this PR

@justmobilize
Copy link
Contributor Author

@anecdata willing to re-test with the newest esp32spi?

Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for your patience on this. I'm much happier now that nothing's conditional based on the socket implementation; I think this is the best we can do without changes to the nina-fw running on the airlift microcontrollers.

@justmobilize
Copy link
Contributor Author

@jepler thanks for pushing. I also like this better!

Copy link
Member

@anecdata anecdata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Using adafruit_esp32spi socket.connect(): Auto-select UDP_MODE #201, this NTP PR is repeatedly successful on native wifi, esp32spi, and WIZnet, with no changes to user code or library needed. NTP sessions from each device IP address confirmed at the router.

test code...
import time
import os
import traceback
import board
import digitalio
import wifi
import adafruit_connection_manager
from adafruit_esp32spi import adafruit_esp32spi
from adafruit_wiznet5k import adafruit_wiznet5k
import adafruit_ntp
import adafruit_requests


# General

def setup(radio):
    radios.append(radio)
    pool = adafruit_connection_manager.get_radio_socketpool(radio)
    ntp = adafruit_ntp.NTP(pool, tz_offset=0)
    ntps.append(ntp)

def connect(radio):
    if radio.__class__.__name__ == "Radio":
        ipv4_str = connect_native(radio)
    elif radio.__class__.__name__ == "WIZNET5K":
        ipv4_str = connect_eth(radio)
    elif radio.__class__.__name__ == "ESP_SPIcontrol":
        ipv4_str = connect_esp(radio)
    return ipv4_str

# Native wifi

def setup_native():
    radio = wifi.radio
    setup(radio)

def connect_native(radio):
    while not radio.connected:
        try:
            radio.connect(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"))
        except Exception as ex:
            traceback.print_exception(ex, ex, ex.__traceback__)
    ipv4_str = radio.ipv4_address
    return ipv4_str

# WIZnet

def setup_eth():
    cs = digitalio.DigitalInOut(board.A3)
    mac = f'de:ad:be:ef:fe:{252:02x}'  # need this to avoid DHCP collisions
    radio = adafruit_wiznet5k.WIZNET5K(spi, cs, mac=mac, debug=False)
    setup(radio)

def connect_eth(radio):
    ipv4_str = radio.pretty_ip(radio.ip_address)
    return ipv4_str

# ESP32SPI

def setup_esp():
    esp32_cs = digitalio.DigitalInOut(board.A0)
    esp32_reset = digitalio.DigitalInOut(board.A1)
    esp32_ready = digitalio.DigitalInOut(board.A2)
    radio = adafruit_esp32spi.ESP_SPIcontrol(spi, esp32_cs, esp32_ready, esp32_reset, debug=False)
    setup(radio)

def connect_esp(radio):
    while not radio.is_connected:
        try:
            radio.connect_AP(os.getenv("WIFI_SSID"), os.getenv("WIFI_PASSWORD"))
        except Exception as ex:
            traceback.print_exception(ex, ex, ex.__traceback__)
    ipv4_str = radio.pretty_ip(radio.ip_address)
    return ipv4_str


time.sleep(3)  # wait for serial
spi = board.SPI()
radios = []
ntps = []
setup_native()
setup_eth()
setup_esp()
while True:
    print(f'{"-"*25}')
    for radio in radios:
        print(connect(radio))
    for ntp in ntps:
        try:
            print(ntp.datetime)
            time.sleep(1)
        except Exception as ex:
            traceback.print_exception(ex, ex, ex.__traceback__)

@justmobilize
Copy link
Contributor Author

@jepler (or maybe @dhalbert), can this be merged?

@jepler jepler merged commit 5cea507 into adafruit:main May 8, 2024
1 check passed
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request May 14, 2024
Updating https://github.com/adafruit/Adafruit_CircuitPython_BME680 to 3.7.5 from 3.7.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_BME680#72 from FoamyGuy/displayio_example

Updating https://github.com/adafruit/Adafruit_CircuitPython_IS31FL3731 to 3.4.0 from 3.3.11:
  > Merge pull request adafruit/Adafruit_CircuitPython_IS31FL3731#54 from DJDevon3/DJDevon3-IS31FL3731

Updating https://github.com/adafruit/Adafruit_CircuitPython_RFM69 to 2.1.21 from 2.1.20:
  > Merge pull request adafruit/Adafruit_CircuitPython_RFM69#54 from vladak/spelling_1

Updating https://github.com/adafruit/Adafruit_CircuitPython_NTP to 3.1.0 from 3.0.13:
  > Merge pull request adafruit/Adafruit_CircuitPython_NTP#32 from justmobilize/esp32spi-support

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Updated download stats for the libraries
@justmobilize justmobilize deleted the esp32spi-support branch May 21, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants