Dependence on adafruit_esp32spi

[xorbit]

It seems this library only works with adafruit_esp32spi.
What can be done to make it compatible with other backends such as adafruit_wiznet5k?

The current implementation calls get_time on on the interface object. So is the right way to implement this to add this method to the adafruit_wiznet5k interface object? Or should this module be rewritten to use generic socket calls?

[askpatrickw]

@xorbit That change you mention is in progress see tannewt#1 It needs more love though.

[xorbit]

Great! I made a pull request to implement socket methods that were missing in adafruit_wiznet5k:
adafruit/Adafruit_CircuitPython_Wiznet5k#31

Seems to work, but I assume the implementation is intended to be used with rtc.set_time_source. When I tried it with that, the NTP call was done way too often. It looks like the NTP server returns poll at 0, so it doesn't mind being bombarded with requests. But I mind my devices doing it that often. ;)

So I made a change to be able to have a minimum poll specified on the NTP object, default is 5 hours. I don't know if you want to implement a change like that, or if you want me to send you a pull request @askpatrickw ?

[askpatrickw]

That would be up to @tannewt

[tannewt]

My intention wasn't to use it with set_time_source but rather to adjust the internal RTC once in a while based on it. The tricky bit in all of this is making sure we don't break ESP32SPI in the process.

[xorbit]

How would that look in code? Would there already be rate limiting on how often the request is made?

[anecdata]

The existing library for ESP32SPI is quite different in approach than the return-time-once UDP implementation for ESP32-S2. Some additional background here.

As I understand it:

• ESP32-S2 is a single UDP call, return the seconds, and done.
• ESP32SPI gets the time from the ESP32, which automatically gets time from the network in the background when it's connected (some issues with delays after [re-]starting a connection), and sets the MCU native RTC module with the timezone-adjusted time.

If not different libraries, at least different classes or methods would be needed, or a breaking change to unify the API. Although even in the latter case, ESP32SPI-based UDP code may not be fully compatible (might need conditionals at least), would need some research and test.

[tannewt]

I think we need to remove all uses of this library as it is now. (Those should just do what set_time does themselves.) Once we do that, then we can change this one.

In the bundle:

ag -Q set_time\(
libraries/helpers/azure/README.rst
75:        ntp.set_time()

libraries/helpers/azure/examples/azureiot_central_commands.py
55:    ntp.set_time()

libraries/helpers/azure/examples/azureiot_hub_directmethods.py
55:    ntp.set_time()

libraries/helpers/azure/examples/azureiot_hub_messages.py
57:    ntp.set_time()

libraries/helpers/azure/examples/azureiot_hub_twin_operations.py
56:    ntp.set_time()

libraries/helpers/azure/examples/azureiot_hub_simpletest.py
57:    ntp.set_time()

libraries/helpers/azure/examples/azureiot_central_simpletest.py
57:    ntp.set_time()

libraries/helpers/azure/examples/azureiot_central_properties.py
56:    ntp.set_time()

libraries/helpers/azure/examples/azureiot_central_notconnected.py
57:    ntp.set_time()

libraries/helpers/ntp/README.rst
90:    ntp.set_time()

libraries/helpers/ntp/examples/ntp_simpletest.py
38:    ntp.set_time()

libraries/helpers/ntp/adafruit_ntp.py
34:    :param bool debug: Set to True to output set_time() failures to console
46:    def set_time(self, tz_offset=0):

libraries/helpers/GC_IOT_Core/adafruit_gc_iot_core.py
347:        ntp.set_time()

Learn:

ag -Q set_time\(
PyPortal_Azure_Plant_Monitor/code.py
54:    ntp.set_time()

PyPortal_TOTP_Friend/code.py
212:    ntp.set_time()

[tannewt]

Maybe @dherrada can help us fix all these up.

[evaherrada]

@tannewt So you'd like me to remove the above files?

[tannewt]

@dherrada No, the code needs to be modified so it doesn't use the NTP library. Instead, they need to set they circuitpython time directly.

Something similar to this (from this library as-is):

        try:
            now = self._esp.get_time()
            now = time.localtime(now[0] + tz_offset)
            rtc.RTC().datetime = now
            self.valid_time = True
        except ValueError as error:
            if self.debug:
                print(str(error))
            return

[evaherrada]

@tannewt Ah, makes sense. Added to my list

[anecdata]

Just a note that esp.get_time:

• will not return a valid value until some interval has passed since connecting to the AP / internet (about 15 seconds?)
• sometimes returns the value 0
• sometimes raises RunTimeError or other esp exceptions

[tannewt]

@brentru Are you aware of this issue? I think you are the original author of this library.

[brentru]

@tannewt I am now, wasn't getting email notifications. I do not know what I need to do with this library.

Do we still need to remove all uses + archive this repository?

Has the above task been done already @evaherrada? Would you like me to do it?

[tannewt]

I think we need to remove or change all uses and then switch this library over to doing real NTP like I've done in https://github.com/tannewt/Adafruit_CircuitPython_NTP/tree/raw_ntp . This current code shouldn't be called NTP though because that's not what it does at all.

We could move this code to a new library or to ESP32SPI directly and switch the uses to it instead. Then, we can change this to the actual UDP NTP code.

[evaherrada]

@brentru Yeah sorry that slipped my mind. That'd be great