home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.44k stars 30.68k forks source link

New TPLink component does not work with declarative config. Discovery not working on different subnets. #21916

Closed bgulla closed 5 years ago

bgulla commented 5 years ago

Home Assistant release with the issue:

0.89.1

Last working Home Assistant release (if known): 0.88

Operating environment (Hass.io/Docker/Windows/etc.):

Docker, Hassio Tested Component/platform:

https://www.home-assistant.io/components/tplink/

Description of problem: TPLink declarative configuration not working. Discovery is limited to the same subnet. I have created a new docker instance with a baremetal config and still see the error. No devices show up as entities.

Note: The hass instance is running on the 10.0.1.x subnet and my IOT/TP-Link devices are on 10.0.8.x

Problem-relevant configuration.yaml entries and (fill out even if it seems unimportant):

tplink:
  discovery: true
  light:
    - host: 10.0.8.252

Traceback (if applicable):

2019-03-10 21:55:27 ERROR (MainThread) [homeassistant.core] Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/usr/src/app/homeassistant/helpers/entity_platform.py", line 257, in _async_add_entity
    if entity.unique_id is not None:
  File "/usr/src/app/homeassistant/components/tplink/light.py", line 82, in unique_id
    return self._sysinfo["mac"]
TypeError: 'NoneType' object is not subscriptable

Additional information: I have also tested this with switches and neither are working as expected and throw the same error.

OttoWinter commented 5 years ago

If tplink discovery uses mDNS (which it probably is), then this is a problem with your network setup. You need to enable multicast UDP traffic over the two subnets, otherwise there's no way to get the discovery to work.

easoncareer commented 5 years ago

tplink not working with different error.


Mon Mar 11 2019 11:56:55 GMT-0700 (Pacific Daylight Time)
Error while setting up platform tplink
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/homeassistant/helpers/entity_platform.py", line 127, in _async_setup_platform
    asyncio.shield(task, loop=hass.loop),
  File "/usr/local/lib/python3.7/asyncio/tasks.py", line 769, in shield
    inner = ensure_future(arg, loop=loop)
  File "/usr/local/lib/python3.7/asyncio/tasks.py", line 592, in ensure_future
    raise TypeError('An asyncio.Future, a coroutine or an awaitable is '
TypeError: An asyncio.Future, a coroutine or an awaitable is required
MartinHjelmare commented 5 years ago

The latter error is a bug. We should change async_setup_platform to be a coroutine.

CC @rytilahti

rytilahti commented 5 years ago

tplink uses a custom protocol for discovery, but it would be possible to make the broadcast address it's using configurable to support other subnets: https://github.com/GadgetReactor/pyHS100/blob/master/pyHS100/discover.py#L34

The nonetype error is caused by the fact that it hasn't been able to fetch the system information from the device, which indicates that there is no connectivity between the networks. Can you try if pyhs100 works in console?

The third error is something, maybe related to what @MartinHjelmare said, but I'm not sure about that.

MartinHjelmare commented 5 years ago

This should be a coroutine: https://github.com/home-assistant/home-assistant/blob/5eab86986e6c56325e9e70d72a9ddbfca53b19ec/homeassistant/components/tplink/light.py#L32

This too: https://github.com/home-assistant/home-assistant/blob/5eab86986e6c56325e9e70d72a9ddbfca53b19ec/homeassistant/components/tplink/switch.py#L27

xfgavin commented 5 years ago

tplink not working with different error.

Mon Mar 11 2019 11:56:55 GMT-0700 (Pacific Daylight Time)
Error while setting up platform tplink
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/homeassistant/helpers/entity_platform.py", line 127, in _async_setup_platform
    asyncio.shield(task, loop=hass.loop),
  File "/usr/local/lib/python3.7/asyncio/tasks.py", line 769, in shield
    inner = ensure_future(arg, loop=loop)
  File "/usr/local/lib/python3.7/asyncio/tasks.py", line 592, in ensure_future
    raise TypeError('An asyncio.Future, a coroutine or an awaitable is '
TypeError: An asyncio.Future, a coroutine or an awaitable is required

Met same error here, can't populate tp-link switches and lights.

marc-gist commented 5 years ago

Discover sometimes works for me, sometimes not, so i just manually set them... However, this new "version" doesn't allow custom "names".

i.e. this should work:

tplink:
  switch:
    - host: 192.168.1.122
      name: custom name for switch

otherwise, I have to re-write ALL of my automatons 👎

rytilahti commented 5 years ago

@marc-gist you can rename your devices with the app or with the pyhs100tool, this option was removed to simplify the code. That discovery is not working every time is obviously not a good sign and should be fixed, maybe the 3 second timeout is too short

HuntingBears01 commented 5 years ago

I also have my TP-Link plugs on a different subnet and neither autodiscover or manual configuration are able to see my devices.

Here's my 0.89 configuration.yaml (not working)

tplink:
  discovery: false
  switch:
    - host: 192.168.20.200
    - host: 192.168.20.201
    - host: 192.168.20.202
    - host: 192.168.20.203

and here's my 0.88 configuration.yaml (working)

switch:
  - platform: tplink
    host: 192.168.20.200
  - platform: tplink
    host: 192.168.20.201
  - platform: tplink
    host: 192.168.20.202
  - platform: tplink
    host: 192.168.20.203

Logs:

2019-03-16 13:16:47 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry TP-Link Smart Home for tplink Traceback (most recent call last): File "/usr/local/lib/python3.7/site-packages/pyHS100/smartdevice.py", line 116, in _query_helper request=request, File "/usr/local/lib/python3.7/site-packages/pyHS100/protocol.py", line 47, in query sock = socket.create_connection((host, port), timeout) File "/usr/local/lib/python3.7/socket.py", line 727, in create_connection raise err File "/usr/local/lib/python3.7/socket.py", line 716, in create_connection sock.connect(sa) OSError: [Errno 113] No route to host The above exception was the direct cause of the following exception: Traceback (most recent call last): File "/usr/src/app/homeassistant/config_entries.py", line 283, in async_setup result = await component.async_setup_entry(hass, self) File "/usr/src/app/homeassistant/components/tplink/__init__.py", line 120, in async_setup_entry await hass.async_add_executor_job(_fill_device_lists) File "/usr/local/lib/python3.7/concurrent/futures/thread.py", line 57, in run result = self.fn(*self.args, **self.kwargs) File "/usr/src/app/homeassistant/components/tplink/__init__.py", line 110, in _fill_device_lists if dev.is_dimmable: # Dimmers act as lights File "/usr/local/lib/python3.7/site-packages/pyHS100/smartplug.py", line 131, in is_dimmable return "brightness" in self.sys_info File "/usr/local/lib/python3.7/site-packages/pyHS100/smartdevice.py", line 185, in sys_info return defaultdict(lambda: None, self.get_sysinfo()) File "/usr/local/lib/python3.7/site-packages/pyHS100/smartdevice.py", line 195, in get_sysinfo return self._query_helper("system", "get_sysinfo") File "/usr/local/lib/python3.7/site-packages/pyHS100/smartdevice.py", line 119, in _query_helper raise SmartDeviceException('Communication error') from ex pyHS100.smartdevice.SmartDeviceException: Communication error

I've reverted back to 0.88 and my plugs are working again so they are definitely reachable from Home Assistant

kneath commented 5 years ago

Should we split off the first part of this issue's topic into another issue? They don't seem very related, but I do think both are important.

I just recently upgraded and losing the ability to declare names for my switches is an order of magnitude loss of functionality for me. I set static IPs for all my switches such that I can declare their names and functionality (automations & scripts) in Home Assistant. Now that other people in my house can rename the switches and silently break functionality, it seems silly to even attempt to control them through Home Assistant.

This has changed Home Assistant's role from being able to control specific unique devices to possibly being able to control some kind of device somewhere in my house. That's a huge bummer.

marc-gist commented 5 years ago

@kneath I agree with you... however, had a discussion with Dev's on Discord and I now understand their reasons for this. I also still disagree 🙁 with them... but hopefully the implement the ability for any component to set the name/id via config. There is still so much good in HA I'm sticking with it.

Thankfully most of my TP-Link devices were named the same inside the device, so got the same entity_id on upgrade. I only had to fix a handful.

I believe they are also working on the Users feature to allow different level of people, which should address your concern over anyone being able to change the entity_id via the UI.

bgulla commented 5 years ago

Looks like other users are running into this too.

https://www.reddit.com/r/homeassistant/comments/bbtz2p/tplink_bulbs_and_naming/

rytilahti commented 5 years ago

@bgulla that reddit discussion is related to naming of devices, this issue is about the discoverability of devices on different subnets (which should be reported to upstream, the only way to fix that is to allow specifying a custom subnet for discovery and then adapt homeassistant to allow passing that via config or config flow).

@balvant813 I removed your comment as the description is already in #22313.

bgulla commented 5 years ago

Do you have more info on how to identify another subnet for discovery?

On Sat, Apr 13, 2019 at 1:55 AM Teemu R. notifications@github.com wrote:

@bgulla https://github.com/bgulla that reddit discussion is related to naming of devices, this issue is about the discoverability of devices on different subnets (which should be reported to upstream, the only way to fix that is to allow specifying a custom subnet for discovery and then adapt homeassistant to allow passing that via config or config flow).

@balvant813 https://github.com/balvant813 I removed your comment as the description is already in #22313 https://github.com/home-assistant/home-assistant/issues/22313.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/home-assistant/home-assistant/issues/21916#issuecomment-482779433, or mute the thread https://github.com/notifications/unsubscribe-auth/ACkoMwJ9ODIUj_HV8IPxQraJ_aiEKOqBks5vgXFqgaJpZM4bnjuQ .

-- Brandon

rytilahti commented 5 years ago

@bgulla that needs to be fixed first in the upstream, but the basic idea would be to allow selecting the broadcast target (see https://github.com/GadgetReactor/pyHS100/blob/master/pyHS100/discover.py#L34), then assuming you know your subnets you can simply use the broadcast address for the given network (instead of using 255.255.255.255).

kneath commented 5 years ago

@rytilahti As per my comment above, this issue used to be about two different things (since edited) — the dns discovery and the declaritive naming of devices. Unfortunately, all issues filed about declarative naming were closed as duplicates of this one. Should we create a new issue for the declarative naming? Or is this more of a “sucks for you” type of situation? It’s a huge bummer that’s really leading me to head toward downgrading several versions to this date. In actuality, I have removed all logic from HA and moved to the Kasa app because of side effects of this change, which is... not where I want to be.

marc-gist commented 5 years ago

@rytilahti As per my comment above, this issue used to be about two different things (since edited) — the dns discovery and the declarative naming of devices. Unfortunately, all issues filed about declarative naming were closed as duplicates of this one. Should we create a new issue for the declarative naming? Or is this more of a “sucks for you” type of situation? It’s a huge bummer that’s really leading me to head toward downgrading several versions to this date. In actuality, I have removed all logic from HA and moved to the Kasa app because of side effects of this change, which is... not where I want to be.

You can manually setup still via "host" (need IP address of each device, so need to reserve/static setup). as any declaration of names/entity_id is no longer supported vi yaml configuration. You can change entity_id in the UI after the initial run. There after, the entity_id's will remain the same. Are you having some other issue?

kneath commented 5 years ago

I’m not really sure what you mean by “host” — could you provide a link or more explanation? I do have all of my devices on static ips.

Changing the entity_id doesn’t really work for... I dunno, all of the reasons. The new discovery service is flakey at best (try to use a device that doesn’t connect at hass startup due to bad wifi, or try to rename a device after it’s been discovered, or delete a device after resetting it, or...). So, since I can’t get reliable names (via name => ip delcarative config), I cannot control my devices with home assistant at all. I cannot tell you the exact issue other than all of my automations with tp-link are non-functional (or at best, 10% functional) after this non-declarative update.

rytilahti commented 5 years ago

@kneath first of all, there has never been a dns-based discovery for these devices, but if you have a dns server (e.g. on your router providing dns for your plugs) using those in the config should work just fine. What exactly you mean with declarative config? Configuring the hosts manually inside your configuration file? If so, that should still work just fine.

There may be some edge cases when the devices are not available during the homeassistant's startup, though, as there is no way to signal to homeassistant to retry the initialization (how it was done before by raising a PlatformNotReady exception). This is something I think should be fixed not for each integration separately, but once and for all integrations. If someone knows how to deal with this, please let me know and/or feel free to create a PR.

Otherwise it's unclear to me what is the exact issue here. In the future it is better to avoid bundling multiple different issues into one report: currently the title speaks about declarative config, which I interpret to be static, manual configuration in the file. that is how I'm using it and it is working just fine. As I already mentioned before, discovery not working over multiple subnets is a completely different issue and that has nothing to do with the manual configuration..

balvant813 commented 5 years ago

I have no luck in discovering TPLINK bulbs via HA (since .90). Switches/Plugs are discovered by HA but I have to manage naming etc in core_entity.yaml. For the bulbs, names etc are managed via Kasa app. To much complexity in managing devices! I have decided not to install any new TP-Link devices anymore. Balvant From: Teemu R. notifications@github.com Sent: Monday, April 15, 2019 12:32 AM To: home-assistant/home-assistant home-assistant@noreply.github.com Cc: Balvant Patel softdev@balvantpatel.com; Mention mention@noreply.github.com Subject: Re: [home-assistant/home-assistant] New TPLink component does not work with declarative config. Discovery not working on different subnets. (#21916)

@kneathhttps://github.com/kneath first of all, there has never been a dns-based discovery for these devices, but if you have a dns server (e.g. on your router providing dns for your plugs) using those in the config should work just fine. What exactly you mean with declarative config? Configuring the hosts manually inside your configuration file? If so, that should still work just fine.

There may be some edge cases when the devices are not available during the homeassistant's startup, though, as there is no way to signal to homeassistant to retry the initialization (how it was done before by raising a PlatformNotReady exception). This is something I think should be fixed not for each integration separately, but once and for all integrations. If someone knows how to deal with this, please let me know and/or feel free to create a PR.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/home-assistant/home-assistant/issues/21916#issuecomment-483103860, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AhNZMzfj9ISOLFqVaBTcGPXom_WW9d3jks5vhAC_gaJpZM4bnjuQ.

kneath commented 5 years ago

@rytilahti I apologize for using incorrect terminology. As to your assumption that you can still declaratively link specific hosts (IP addresses) to names in yaml, that is no longer the case. The only option for managing names/devices is now to use the discovery tool in the web UI in connection with the Kasa app. This currently has a lot of bugs (like I listed above), but more importantly, is just fundementally against how I use home assistant.

I’m kind of bummed this got derailed with all this back and forth. I have seen lots and lots of people stumble on the removal of this declarative naming. A lot of those people got redirected to this issue, and then in this issue told it wasn’t about declarative naming. It would be really great to have a way to write (in yaml) the names of my switches so that all my automations and config are self-contained to home-assistant and not split between yaml, home assistant web ui, and Kasa, and were reliable and easy to debug. My whole reason for using home assistant was to take control of my devices, not to use more 3rd party apps like Kasa.

That’s the last I’ll say on this, for now I’m downgrading home assistant and pinning it to an old version that lets me do this. Because right now it’s just become a web UI for viewing my Kasa configuration.

rytilahti commented 5 years ago

@balvant813 so the discovery is just fine with the plugs&strips? Maybe the problem is #20994 that is already fixed in master? About renaming the devices, see below.

@kneath you can do renaming from the homeassistant's ui, or with pyhs100 console tool. If the renaming as done within the UI is not something you like for some reason, that's something that needs to be fixed homeassistant in general, not separately for every single integration -- the exact reason for having a unique ids for devices is that it will makes things like renaming possible.

IIRC not having a separate setting to overload the names was even requested by the core devs, if someone wants to go and dig around the PRs and commits to verify, feel free to do. Or if you want, you can always open a PR to add that feature back to see if it will get accepted, I don't personally care about that.

bgulla commented 5 years ago

All- This issue has gotten off topic and I have created a new ticket that focuses on the networking bug rather than the 'omg i cant rename this in homeassistant, gimmie my FOSS money back'. New Issue