Add a connect_single method to Discover to avoid the need for UDP

[bdraco]

This should equate to a significant reliability improvement for networks with poor wifi (edge of range)/udp.

needs #494

related issue home-assistant/core#99449

Can be tested with HA by doing

diff --git a/homeassistant/components/tplink/__init__.py b/homeassistant/components/tplink/__init__.py
index d8285cbed70..1c98c1e1ae1 100644
--- a/homeassistant/components/tplink/__init__.py
+++ b/homeassistant/components/tplink/__init__.py
@@ -87,7 +87,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     """Set up TPLink from a config entry."""
     host = entry.data[CONF_HOST]
     try:
-        device: SmartDevice = await Discover.discover_single(host)
+        device: SmartDevice = await Discover.connect_single(host)
     except SmartDeviceException as ex:
         raise ConfigEntryNotReady from ex
 

[rytilahti]

Sounds like a good idea to me, however I'm wondering if this will make more difficult to integrate #509 and the potential support for tapo devices, as for those we don't have the information about the protocol available w/o performing a discovery call.

[codecov]

Codecov Report

All modified lines are covered by tests ✅

Files	Coverage Δ
kasa/discover.py	85.06% <100.00%> (+0.81%)	⬆️

📢 Thoughts on this report? Let us know!.

[bdraco]

Sounds like a good idea to me, however I'm wondering if this will make more difficult to integrate #509 and the potential support for tapo devices, as for those we don't have the information about the protocol available w/o performing a discovery call.

I think we can work around that by adding params to connect_single to pass the needed protocol info from the previous discovery when the device is set-up and save it in the HA config entry. This way we avoid having to discovery it again which would also give these devices the same reliability improvement.

This assumes that the discovered info for a device doesn't need to be discovered every time.

Something like that would likely be a better design anyways since it would avoid the first query for the current protocol. I was looking for a solution that didn't involve a major change to HA to start with which could be iterated on in the future.

[rytilahti]

Sounds like a good solution to me. I was wondering if we should convert the cli tool to use connect_single too, but I think that can wait considering we may need the discovery query as we don't want to store any infos for it. Would you mind adding a couple of words into the docs (either docstrings or docs/source/discover.rst) to describe the difference between those two ways to discover single devices?

Ping @sdb9696 as this is relevant for the klap PR & for its future homeassistant support.

[bdraco]

added docstrings in 892f4c4

[rytilahti]

Thanks! 👍

[sdb9696]

@bdraco @rytilahti
Hi, I took a look at this PR but I don't think the UDP discover_single we introduced to support new protocols is the issue. Reason being that the kasa library currently in HA is 0.5.3 and that version doesn't use UDP, it used a similar direct TCP connection as per the connect_single introduced by this PR unless I am mistaken:

https://github.com/python-kasa/python-kasa/blob/0.5.3/kasa/discover.py

[bdraco]

@sdb9696 We haven't switched HA to use connect_single yet

https://github.com/home-assistant/core/blob/cef68ea33c509369a71bd1f5a775b17dc25b4f63/homeassistant/components/tplink/__init__.py#L90

The problem with the power strips needs to be fixed with it first before we can use it in home-assistant/core#103150

#538

[sdb9696]

@bdraco Yes I know but we haven't switched python-kasa to use UDP yet either (well it seems it's been bumped to 0.5.4 today but the issue being reported was well before that). If you look at the https://github.com/python-kasa/python-kasa/blob/0.5.3/kasa/discover.py you can see it used the old discover_single method which was the same as the new connect single you're adding here.

[bdraco]

@bdraco Yes I know but we haven't switched python-kasa to use UDP yet either (well it seems it's been bumped to 0.5.4 today but the issue being reported was well before that). If you look at the 0.5.3/kasa/discover.py you can see it used the old discover_single method which was the same as the new connect single you're adding here.

I follow now. So we are likely to have more problems now that its bumped 😢

[sdb9696]

Hopefully not because the UDP method only sends to the target ip, it doesn't broadcast to 255.255.255.255, so it shouldn't really slow things down.

However, that said I noticed when looking at your PR that some users are using hostnames to add to HA and the UDP discover_single does not handle this properly. I created PR #539 to fix this and I'd suggest if we're happy with it we put it in a 0.5.5 and bump to that.