adafruit / Adafruit_CircuitPython_NTP

Network Time Protocol (NTP) Helper for CircuitPython
MIT License
9 stars 18 forks source link

Add ESP32SPI support #32

Closed justmobilize closed 4 months ago

justmobilize commented 4 months ago

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

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

justmobilize commented 4 months ago

@anecdata and @dhalbert - simple update

anecdata commented 4 months ago

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

jepler commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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

anecdata commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

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

justmobilize commented 4 months ago

But the esp32spi only has recv...

justmobilize commented 4 months ago

@jepler anything thoughts / recommendations?

jepler commented 4 months ago

https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/blob/main/adafruit_esp32spi/adafruit_esp32spi_socketpool.py#L137 I see "def recv_into", is it not applicable?

justmobilize commented 4 months ago

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

jepler commented 4 months ago

Maybe #201 helps with the connect() difference

justmobilize commented 4 months ago

@jepler that works

justmobilize commented 4 months ago

Once ESP32SPI is released, I'll update this PR

justmobilize commented 4 months ago

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

justmobilize commented 4 months ago

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

justmobilize commented 4 months ago

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