valiquette / homebridge-rachio-irrigation

Provides Homebridge support for Rachio controller
MIT License
17 stars 4 forks source link

Documentation changes for using Webhook Relay instead of Port Forwarding #12

Closed jstrellner closed 2 years ago

jstrellner commented 2 years ago

I didn't want to use port forwarding, and while using Webhook Relay adds a number of steps, it has the benefit of not needing to poke a hole in my local firewall, and it works if your IP address gets changed from your ISP.

This pull request @valiquette just edits the documentation to explain how to do this.

jstrellner commented 2 years ago

@valiquette Yes, I do see that in the logs, in red no less. Suppressing that check with a flag would work, but would complicate things by having another config option that needs to manually be changed.

I see three options (third comes with a suggestion).

A final thought in the case of using a webhook relay, you can still verify that it's working by doing a GET/POST request to somethinglongandrandom.hooks.webhookrelay.com:[port]/test. In theory, you could check if it's a FQDN and then hit somethinglongandrandom.hooks.webhookrelay.com:[port]/startup-test or similar, and if you don't get that webhook within x seconds of a startup, throw an error message. Might be overkill, though.

valiquette commented 2 years ago

@jstrellner I was thinking about this earlier too, that the pattern applied in the config is for ipv4 only and would exclude an ipv6 address too. So option 3 works best, I will remove the pattern on schema and add some logic to check for valid ipv4, ipv6 and FQDN prior to setting the webhook. I will change the error to a warning if the address used does not match the address expected and ignore if FQDN. You can update your instructions to reflect not needed to manually edit the config file. Thanks

jstrellner commented 2 years ago

@valiquette I've updated the readme to remove the config file editing part, and a few minor other changes.

I did take the liberty of assuming the previous External IP Address label would be updated to External IP Address or Domain. Hopefully that's ok.

p.s. Just so you know, if it's not clear, great work on this plugin. Thanks for all your work on it.