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

In preparation for 1.8.1 release... #244

Merged
merged 13 commits into from
Oct 29, 2024
Merged

Conversation

dkerr64
Copy link
Collaborator

@dkerr64 dkerr64 commented Oct 27, 2024

v1.8.1 (2024-11-xx)

What's Changed

  • New feature... Allow selection of time zone when NTP server enabled.
  • Change... We use built in Arduino core NTP client for time instead of separate module
  • Change... Replace DNS lookup with a ping to gateway to test for network connectivity
  • Bugfix... Don't display that an update is available if running newer pre-release
  • Bugfix... When changing SSID in soft access point, make sure the WiFi settings are reset to DHCP
  • Bugfix... wifiSettingsChanged setting had been removed by mistake.

Known Issues

dkerr64 and others added 6 commits October 27, 2024 10:31
Don't use DNS lookup as a network test as that might require internet
access.  Use a simple ping check.  LwIP has ping support, but not
exposed in an easy to use manner.  Pull in ESP8266Ping library to make
ping easy.

Log once a minute the gateway ping stats
Fix bug in web.cpp that wasn't saving the wifi change setting anymore
@jgstroud
Copy link
Collaborator

@dkerr64 Ok, so I changed the DNS lookup to a gateway ping check. The old way was fine, but I wanted to eliminate the need to do an internet lookup. With the DNS method, mine would lookup google.com because my DNS server is on a different subnet. Also fixed a bug that was introduced when you refactored the settings.

@dkerr64
Copy link
Collaborator Author

dkerr64 commented Oct 28, 2024

@dkerr64 Ok, so I changed the DNS lookup to a gateway ping check. The old way was fine, but I wanted to eliminate the need to do an internet lookup. With the DNS method, mine would lookup google.com because my DNS server is on a different subnet. Also fixed a bug that was introduced when you refactored the settings.

Thanks for finding the wifi settings change bug. Yes that was a mistake on my part.

@dkerr64
Copy link
Collaborator Author

dkerr64 commented Oct 28, 2024

Do you think we should log every minute that we pinged the gateway ok? Feels excessive. Maybe only log if it fails and/or latency is excessive?

my latency logs at 3-4ms. Excessive might be >50ms ?

@jgstroud
Copy link
Collaborator

yeah, I was wondering if that was going to be too noisy. ill rework it

@dkerr64
Copy link
Collaborator Author

dkerr64 commented Oct 28, 2024

Now that I have timezone implemented, I'm thinking maybe should use ratgdo time (if available) for syslog.

Also... for timezones, I have provided selection for usual USA timezones. But I could load the full list from the CSV file here... https://github.com/nayarsystems/posix_tz_db ... it is all client-size in the browser javascript, so no impact on memory/performance at the ratgdo.

@jgstroud
Copy link
Collaborator

I think you can auto discover you timezone based on your public IP address using https://ip-api.com

@jgstroud
Copy link
Collaborator

@dkerr64 I pushed a function that will lookup the timezone offset from that url I sent earlier. You can use it if you want. If you dont want to use it, we can drop the commit

@jgstroud jgstroud force-pushed the for-1-8-1-release branch 2 times, most recently from 8dceabd to 2488343 Compare October 28, 2024 04:51
@dkerr64
Copy link
Collaborator Author

dkerr64 commented Oct 28, 2024

@jgstroud Thanks for the timezone lookup. We also need to convert from e.g. Americas/New_York into the posix encoding, in this example "EST5EDT,M3.2.0,M11.1.0". I'm doing this now on the browser side with a hardcoded list, but the full list is available online (link above).

@jgstroud
Copy link
Collaborator

Yeah I saw that. I just used UTC. That sets the time correctly, but doesn't display a friendly timezone when you print the time. I think this works if you just remove the time zone when you print the time.

Otherwise we can just use your browser selector.

@jgstroud
Copy link
Collaborator

Maybe give my commit a try for auto setting the timezone. If you don't like it we can remove the commit. My testing, it worked fine. But, to handle daylight savings time, we would probably need to check the offset daily just after midnight.

@dkerr64
Copy link
Collaborator Author

dkerr64 commented Oct 28, 2024

Maybe give my commit a try for auto setting the timezone. If you don't like it we can remove the commit. My testing, it worked fine. But, to handle daylight savings time, we would probably need to check the offset daily just after midnight.

Your lookup is useful. I think we can solve for the 2nd part... worst case in in the browser javascript side if not on the ratgdo itself. I'll look into it this evening.

@dkerr64
Copy link
Collaborator Author

dkerr64 commented Oct 28, 2024

Also, on the gateway pings, I see you do a millis() % 60000 == 0 test. I don't think this is reliable, it requires that the loop is executed exactly on a multiple of 60 seconds. If it executes just one millisecond later then the test fails. I think we need to set a static timeout = mills() + 60000 and then test for millis() > timeout

Thanks.

@jgstroud
Copy link
Collaborator

Yeah, you are right. There is no way that would be reliable. I've been writing Verilog recently where this wouldn't be an issue. My context switching in my head just isn't what it used to be. I'll make the change. Thanks for catching that.

autoset the time based on local timezone
@jgstroud
Copy link
Collaborator

@dkerr64 I fixed the mistake with the timers. also, for the Timezone, I was just setting it to UTC, so I removed the timezone code from the timeString. But I don't want to overstep. If you want to change it, go right ahead. I just got the itch to make the auto-timezone thing work. My feelings won't be hurt if you completely redo that.

@dkerr64
Copy link
Collaborator Author

dkerr64 commented Oct 28, 2024

Here is what I am working on for time zone...

  1. Default value for timezone is blank.
  2. If timezone blank, ratgdo requests timezone from external IP address, as you have coded. If not blank then it is assumed that user has already set desired timezone so no need to request.
  3. Maybe... if SSID is changed, reset timezone to blank?
  4. If the browser receives a timezone that is region/city only and no POSIX (simple test for a separating semicolon) then the browser will lookup the posix code and send it to the server.
  5. If cannot fine a POSIX encoding, then sent UTC to the server.
  6. From that point on we're all good.

On the browser side... we will asynchronously load the timezone CSV from https://github.com/nayarsystems/posix_tz_db and use that to do the lookup. We will also use it to populate a selection field in settings page. If loading from that URL fails, then use built-in list of half-dozen USA timezones.

@jgstroud
Copy link
Collaborator

That all sounds good. The only suggestion might be in step 1, instead of having a blank selection, have a default selection of "Auto".

@dkerr64
Copy link
Collaborator Author

dkerr64 commented Oct 29, 2024

Have implemented time zone. If you switch SSID then timezone is reset and discovered from external IP address. I'm not sure if this is good idea or not, but it provides us a way to test it.

@dkerr64
Copy link
Collaborator Author

dkerr64 commented Oct 29, 2024

I'm inclined to finish this up with update to readme for timezone, and then publish this.

Then maybe we look harder at dry contact support as several people are requesting it... before we get tied up with ESP32.

@jgstroud
Copy link
Collaborator

I agree. Let's not add any more to this merge. Yeah, there have been quite a few dry contact requests recently. My plan was to just lift the dry contact code verbatim from https://github.com/ratgdo/mqtt-ratgdo/blob/2.5/src/ratgdo.cpp

The biggest reason I haven't touched it yet is because I have no way to test it.

@dkerr64
Copy link
Collaborator Author

dkerr64 commented Oct 29, 2024

I agree. Let's not add any more to this merge. Yeah, there have been quite a few dry contact requests recently. My plan was to just lift the dry contact code verbatim from https://github.com/ratgdo/mqtt-ratgdo/blob/2.5/src/ratgdo.cpp

The biggest reason I haven't touched it yet is because I have no way to test it.

One of my doors has a 20-year old opener. I have not connected a ratgdo to it, but it may well require dry contact.

@dkerr64 dkerr64 merged commit 8aa795a into ratgdo:main Oct 29, 2024
@dkerr64 dkerr64 deleted the for-1-8-1-release branch October 29, 2024 22:41
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.

2 participants