[DON'T MERGE] Xiaomi Gateway discovery using Xiaomi adhoc protocol #141

pfalcon · 2017-08-27T07:07:22Z

Uses own discovery protocol, so corresponding class is included. Likely,
the Gateway should have "development mode" activated to be discovered
(and certainly needed to be queried/controller later).

mention-bot · 2017-08-27T07:07:23Z

@pfalcon, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @magicus and @fabaff to be potential reviewers.

homeassistant · 2017-08-27T07:07:24Z

Hi @pfalcon,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

pfalcon · 2017-08-27T07:12:59Z

@Danielhiversen, @syssi : FYI.

This is actually an RFC, as I so far didn't see the documented requirements what info netdisco's "discovered" entries should contain, nor see much of the consistency in the code. My hunch there's actually none, and all is governed by how corresponding component uses it in HA (in other words, netdisco isn't really a standalone, reusable library).

I have a prototype of HA integration of this too - which happens to not care about any data returned from netdisco, only the fact of discovery is important, because HA component has own (!) discovery process.

Anyway, my first step is to wade thru that CLA process, maybe I won't get even past it ;-).

balloob · 2017-08-27T16:43:44Z

netdisco/xiaomi_gw.py

+                    entry = json.loads(data.decode("utf-8"))
+                except:  # pylint: disable=bare-except
+                    continue
+                if entry.get("model") == "gateway":


I think that we should not filter entries, instead return all discovered Xiaomi devices.

Hmm, there're few issues with that.

This implements discovery specifically for https://home-assistant.io/components/xiaomi/ , which is the Gateway device. The naming in HA is "not ideal", per p.2. So, the above code statement isn't really "filtering", but a simple check that what was discovered is actually a specific device and not something else (random UDP server running on that port).

"All Xiaomi devices" are 20-30 absolutely different devices nowadays, and might go 200-300 in the future, unless Xiaomi goes bankrupt, which doesn't seem to be a problem so far, given their marketing know-how of selling their name to random Chinese companies to help them sell their stuff to Westeners. I really wouldn't like HA try to do something about Xiaomi car simply because it can work with Xiaomi Gateway (this includes mis-discovering a car as a gateway).

I think in that case the filtering should happen in the discoverables. So we can discover a gateway as a separate model. That way if we want to discover something else, we can re-use the discovery code.

Ok, I was thinking about this too, and some clarification: discovery protocol implemented by "xiaomi_gw.py" is not a discovery protocol for "Xiaomi products", because majority of "Xiaomi products" are made by varying "subsidiaries", which have little in common on technical side (except for integration into Xiaomi's cloud). In particular, the protocol used by "xiaomi_gw.py" is currently known to be used only by Xiaomi Mi Home Gateway. That's why the protocol module is called "xiaomi_gw.py" (discoverable module is called "xiaomi.py", because it should match HA's component (or service?) name, and unfortunately, HA uses "xiaomi" for the Gateway name, which will lead to confusion as more wide-varying "Xiaomi" devices are added to HA).

It's possible that this protocol will be used in other "Xiaomi" products, but it will never be the only protocol to discover all "Xiaomi" devices. Hence, I err on the side of specificity, to avoid even remotely the situation that somebody's Xiaomi Water Tap (for example) will be recognized as Home Gateway.

@balloob, you suggest to err on the side of generality. I usually love that, but not this time, due to the reasons above. However, seeing your point, I may imagine following change:

xiaomi_gw.py gets renamed to xiaomi_whoami.py, with docstrings that this module implements Xiaomi UDP, JSON based "whoami" protocol, which is currently known to be used by just Gateway.

xiaomi.py discoverable filters just "gateway" entries, as this discoverable indeed represent only gateway.

Does that sound good?

Sounds good 👍

balloob · 2017-08-27T17:00:47Z

The entries are consistent in that we try to include an ATTR_HOST and ATTR_PORT in the result dictionary.

pfalcon · 2017-08-27T19:40:32Z

The entries are consistent in that we try to include an ATTR_HOST and ATTR_PORT in the result dictionary.

I must be very lucky then, because first random non-mDNS, non-UPNP module I looked at, doesn't even have a result dictionary, but a tuple instead: https://github.com/home-assistant/netdisco/blob/master/netdisco/tellstick.py

Anyway, thanks for the hint, I'll like how those attributes are used.

pfalcon · 2017-08-27T21:29:25Z

Actually, HA docs seem to be more supporting of the idea "it can be anything" aka "there's no consistency":

https://home-assistant.io/docs/configuration/events/#event-platform_discovered

Information that is discovered. Can be a dict, tuple etc. Example: (192.168.1.10, 8889).

balloob · 2017-08-27T23:05:51Z

With the release of Netdisco 1.0 we were supposed to migrate all to be dictionaries. Looks like we missed Tellstick.

I'll update the docs.

Uses own discovery protocol, so corresponding class is included. Likely, the Gateway should have "development mode" activated to be discovered (and certainly needed to be queried/controller later).

pfalcon · 2017-08-28T09:02:39Z

Updated to use ATTR_HOST and friends.

Now the result looks like:

$ python3 example_service.py 
INFO:netdisco.service:Scanning
2017-08-28 11:56:53.734944 - Found new service: xiaomi {'properties': {'ip': '192.168.0.101', 'cmd': 'iam', 'model': 'gateway', 'sid': '286c2285aaaa', 'port': '9898'}, 'host': '192.168.0.101', 'mac_address': '286c2285aaaa', 'port': 9898}
2017-08-28 11:56:53.735111 - Found new service: yeelight {'properties': {'epoch': '1', 'mac': '12b429354465'}, 'host': '192.168.0.100', 'hostname': 'yeelink-light-mono1_miio13646404.local.', 'device_type': 'white', 'port': 54321}

Comments:

I spotted ATTR_MAC_ADDRESS, and used that. However, "yeelight" discoverable doesn't use that, and passes MAC as an adhoc property. Worth a separate patch to fix that.
Original discovery response from the Gateway is passed as "properties" just in case.

rytilahti · 2017-08-29T20:15:51Z

netdisco/xiaomi_gw.py

+                    entry = json.loads(data.decode("utf-8"))
+                except:  # pylint: disable=bare-except
+                    continue
+                if entry.get("model") == "gateway":


Would it be possible to include an example of json returned by the device here?

Btw, the gateway (and other Xiaomi devices / devices accessible with the Mi Home app, like Yeelight bulbs) are also discoverable over mDNS with _miio._udp). Generally those devices will also respond on port 54321 for discovery (device type detection unknown to me).

For example

-> {"cmd":"whois"} <- {"cmd":"iam","port":"9898","sid":"286fffffffff","model":"gateway","ip":"192.168.130.29"}

Btw, the gateway (and other Xiaomi devices / devices accessible with the Mi Home app, like Yeelight bulbs) are also discoverable over mDNS with _miio._udp).

D'oh! Fairly speaking, I wanted to play with mDNS regarding that, but that slipped, partly because I'm not much familiar with mDNS, and would need to dig into it. So instead I just used info from https://github.com/louisZL/lumi-gateway-local-api/blob/master/device_discover.md#1-网关设备发现设备发现不加密 .

@rytilahti , would you submit an alternative PR, using mDNS? If not, I can look into it, but per above, it would take me some time to dig into it. But we surely better use existing generic discovery protocols rather than add support for adhoc vendor-specific ones.

I'm not currently able to do that, however, maybe it would be best to extend https://github.com/home-assistant/netdisco/blob/master/netdisco/discoverables/yeelight.py to detect the gateway too, and rename it miio.py? Here's how I have started to do similar detection in python-mirobo: https://github.com/rytilahti/python-mirobo/blob/master/mirobo/discovery.py#L48

Thanks for all the hints! I prepared mDNS-based patch at #144 . Let's move further discussion there.

homeassistant added the cla-needed label Aug 27, 2017

homeassistant added cla-signed and removed cla-needed labels Aug 27, 2017

pfalcon mentioned this pull request Aug 27, 2017

Xiaomi discovery stopped working since 0.50 home-assistant/core#8994

Closed

homeassistant added the cla-signed label Aug 27, 2017

pfalcon mentioned this pull request Aug 27, 2017

Discovery cycle is too long #142

Closed

balloob reviewed Aug 27, 2017

View reviewed changes

Add Xiaomi Gateway discovery.

7f16b13

Uses own discovery protocol, so corresponding class is included. Likely, the Gateway should have "development mode" activated to be discovered (and certainly needed to be queried/controller later).

homeassistant added the cla-signed label Aug 28, 2017

pfalcon mentioned this pull request Aug 28, 2017

MDNSDiscoverable: Set ATTR_MAC_ADDRESS if available. #143

Merged

rytilahti reviewed Aug 29, 2017

View reviewed changes

pfalcon changed the title ~~Add Xiaomi Gateway discovery.~~ [DON'T MERGE] Xiaomi Gateway discovery using Xiaomi adhoc protocol Sep 1, 2017

pfalcon mentioned this pull request Sep 1, 2017

Add Xiaomi Mi Home Gateway discovery. #144

Merged

balloob closed this Dec 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DON'T MERGE] Xiaomi Gateway discovery using Xiaomi adhoc protocol #141

[DON'T MERGE] Xiaomi Gateway discovery using Xiaomi adhoc protocol #141

pfalcon commented Aug 27, 2017

mention-bot commented Aug 27, 2017

homeassistant commented Aug 27, 2017

pfalcon commented Aug 27, 2017

balloob Aug 27, 2017

pfalcon Aug 27, 2017

balloob Aug 29, 2017

pfalcon Aug 29, 2017

balloob Aug 29, 2017

balloob commented Aug 27, 2017

pfalcon commented Aug 27, 2017

pfalcon commented Aug 27, 2017

balloob commented Aug 27, 2017

pfalcon commented Aug 28, 2017 •

edited

rytilahti Aug 29, 2017 •

edited

syssi Aug 29, 2017

pfalcon Aug 30, 2017

rytilahti Aug 30, 2017 •

edited

pfalcon Sep 1, 2017

[DON'T MERGE] Xiaomi Gateway discovery using Xiaomi adhoc protocol #141

[DON'T MERGE] Xiaomi Gateway discovery using Xiaomi adhoc protocol #141

Conversation

pfalcon commented Aug 27, 2017

mention-bot commented Aug 27, 2017

homeassistant commented Aug 27, 2017

pfalcon commented Aug 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balloob commented Aug 27, 2017

pfalcon commented Aug 27, 2017

pfalcon commented Aug 27, 2017

balloob commented Aug 27, 2017

pfalcon commented Aug 28, 2017 • edited

rytilahti Aug 29, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rytilahti Aug 30, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pfalcon commented Aug 28, 2017 •

edited

rytilahti Aug 29, 2017 •

edited

rytilahti Aug 30, 2017 •

edited