Closed xorbit closed 2 years ago
@xorbit That change you mention is in progress see https://github.com/tannewt/Adafruit_CircuitPython_NTP/pull/1 It needs more love though.
Great! I made a pull request to implement socket methods that were missing in adafruit_wiznet5k: https://github.com/adafruit/Adafruit_CircuitPython_Wiznet5k/pull/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 ?
That would be up to @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.
How would that look in code? Would there already be rate limiting on how often the request is made?
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:
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.
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()
Maybe @dherrada can help us fix all these up.
@tannewt So you'd like me to remove the above files?
@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
@tannewt Ah, makes sense. Added to my list
Just a note that esp.get_time
:
0
esp
exceptions@brentru Are you aware of this issue? I think you are the original author of this library.
@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?
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.
@brentru Yeah sorry that slipped my mind. That'd be great
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?