ratgdo / homekit-ratgdo

A native HomeKit implementation of a Security+ 2.0 garage door controller based on ratgdo hardware
https://ratgdo.github.io/homekit-ratgdo/
GNU General Public License v3.0
214 stars 21 forks source link

In preparation for 1.8.1 release... #244

Closed dkerr64 closed 3 weeks ago

dkerr64 commented 3 weeks ago

v1.8.1 (2024-11-xx)

What's Changed

Known Issues

jgstroud commented 3 weeks ago

@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 commented 3 weeks ago

@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 commented 3 weeks ago

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 commented 3 weeks ago

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

dkerr64 commented 3 weeks ago

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 commented 3 weeks ago

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

jgstroud commented 3 weeks ago

@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

dkerr64 commented 3 weeks ago

@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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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.

jgstroud commented 3 weeks ago

@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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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.