kodi-pvr / pvr.hdhomerun

Kodi's HDHomeRun client addon
GNU General Public License v2.0
20 stars 24 forks source link

Add setting to use HTTP-based tuner discovery #133

Closed djp952 closed 2 years ago

djp952 commented 2 years ago

Refers to Issue https://github.com/kodi-pvr/pvr.hdhomerun/issues/116 - "Use ip address instead of device discovery"

This Pull Request adds a new setting to the addon that allows users to opt into using the SiliconDust API "HTTP Discovery" (at least that's what I've always it) to find their tuner devices. The normal UDP broadcast discovery doesn't work when the tuner(s) are on a different subnet than the client.

The HTTP discovery data can become "stale", as any device the user had turned on and connected will stay in this list for up to 24 hours after it's been turned off or disconnected. As such, the only data points used from the API are "DeviceID", which indicates the device has tuners and "LocalIP". The LocalIP address is used to then contact the device and gather the discovery information normally via libhdhomerun. This allows stale devices to be ignored.

If no devices are found via HTTP, there is a fallback to try UDP discovery as well. I felt this important since this API might disappear in the future.

Please let me know what you think or any edits that need to be made, happy to oblige.

I only added the setting string to en_gb, this is correct and it will be translated automatically at some point, yes?

djp952 commented 2 years ago

Will address all this evening and re-push, thank you for the review!

djp952 commented 2 years ago

All requested changes made; sorry to Jenkins for the double force-push, I noted a blank line between variable declarations that shouldn't have been there but didn't see it until I looked at the green/red diff here

fuzzard commented 2 years ago

Looks ok to me. I'll have to ping @ksooo or @phunkyfish for merge rights.

One question i will ask, are you aware of whats returned in the event of an ipv6 network? Im not silly enough to run that for my internal gear but im sure someone will chime in one day with WHATABOUTMYIPV6. The query only comes about due to the use of ntohl.

Im ok to pushing it back until someone complains, so was just more a query whether in your experience with your other DVR addon if anyone had ever mentioned usage on an ipv6 network

djp952 commented 2 years ago

One question i will ask, are you aware of whats returned in the event of an ipv6 network? Im not silly enough to run that for my internal gear but im sure someone will chime in one day with WHATABOUTMYIPV6. The query only comes about due to the use of ntohl.

The current HDHomeRun ecosystem is exclusively IPv4, their APIs actually redirect to "ipv4-api.hdhomerun.com" if you dig into them. There is no support for IPv6 I can see in their libraries and I've never seen any discussion about that from SiliconDust, everything they talk about is always in IPv4 speak.

I think it's a safe bet for now, if they ever add IPv6 to their devices we'll see changes in libhdhomerun, the ntohl version of the IP is all that would be accepted today by the library. Basically, we'd have no way to add it right now even if we wanted to :)

phunkyfish commented 2 years ago

Add-on builds appear to be broken, once they fixed I'll get this released. Thanks for the contribution @djp952!

fuzzard commented 2 years ago

If you have some spare time @djp952 may be worth a backport to v19. I'm not sure how add-ons usually handle features, but I imagine it's not as strict as core Kodi.

phunkyfish commented 2 years ago

Add-on builds appear to be broken, once they fixed I'll get this released. Thanks for the contribution @djp952!

Builds working and it’s released.