Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add component for xiaomi robot vacuum (switch.xiaomi_vacuum) #7913

Merged
merged 3 commits into from
Jul 7, 2017

Conversation

rytilahti
Copy link
Member

@rytilahti rytilahti commented Jun 5, 2017

Description:

Adds support for Xiaomi MiJia vacuum cleaner, uses python-mirobo for device communication. Only basic functionality (starting, stopping & returning to the base, and informing about its state through state attributes) is currently implemented.

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#2771

Example entry for configuration.yaml (if applicable):

switch:
  - platform: xiaomi_vacuum
    host: 192.168.123.123
    name: 'My vacuum'
    token: '123abc70343055483230644c53707aaa'

Checklist:

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

@jcastro
Copy link

jcastro commented Jun 5, 2017

I just pushed a first revision of the documentation here home-assistant/home-assistant.io#2771

@rytilahti rytilahti changed the title WIP: add component for xiaomi robot vacuum (switch.xiaomi_vacuum) Add component for xiaomi robot vacuum (switch.xiaomi_vacuum) Jun 5, 2017
@bramkragten
Copy link
Member

bramkragten commented Jun 6, 2017

Hi, thanks for the nice work!
Maybe my understanding of how home-assistant is structured is wrong, but I have a question:
Why did you make a switch for the vacuum? Don't you want to add sensors and other things to it to read the status of the brushes, set the speed, etc. Your library is capable of a lot more then just turning it on and off... Making it a switch now limits your possibilities for expansion (without breaking changes), wouldn't you rather make it a component, with a switch, sensors and services underneath?

@rytilahti
Copy link
Member Author

I made it a switch as it was the simplest way to get it usable without extra effort (and boilerplate). The information about sensors is given out in state_attributes so one can simply use templating to make it look nicer. Setting the speed would be nice, but it could also be exposed as a service of the switch, if I'm not mistaken?

@bramkragten
Copy link
Member

Yeah, that would work. Just a little more work for the end user, but it's workable.

I tried it last night by the way, but it does not work for me.
I keep getting the following in the log:

2017-06-06 23:14:41 ERROR (Thread-4) [mirobo.vacuum] got error when receiving: timed out
2017-06-06 23:14:41 ERROR (Thread-4) [homeassistant.components.switch.xiaomi_vacuum] Got exception while fetching the state:
2017-06-06 23:15:12 ERROR (Thread-10) [mirobo.vacuum] got error when receiving: timed out
2017-06-06 23:15:12 ERROR (Thread-10) [homeassistant.components.switch.xiaomi_vacuum] Got exception while fetching the state:
2017-06-06 23:15:43 ERROR (Thread-8) [mirobo.vacuum] got error when receiving: timed out
2017-06-06 23:15:43 ERROR (Thread-8) [homeassistant.components.switch.xiaomi_vacuum] Got exception while fetching the state:
2017-06-06 23:16:14 ERROR (Thread-10) [mirobo.vacuum] got error when receiving: timed out
2017-06-06 23:16:14 ERROR (Thread-10) [homeassistant.components.switch.xiaomi_vacuum] Got exception while fetching the state:

I'm sure the IP is correct, and I got the token from the CLI tool.
Any ideas what I might be doing wrong?

@rytilahti
Copy link
Member Author

rytilahti commented Jun 7, 2017

Did you update to the newest firmware already? If yes, the issue may lie in the fact that the clocks are not synchronized, see rytilahti/python-miio#14 .

@bramkragten
Copy link
Member

I'm on version 3.3.9_003073.

@rytilahti
Copy link
Member Author

rytilahti commented Jun 7, 2017

Timeout just means that the robot is not answering, which could be caused by an incorrect token, wrong IP address or different, stricter firmware as is probably the case with the newest one. Anyway, I think this inclusion can be postponed until this gets resolved & new version gets pushed.

In the meanwhile the component could indeed be splitted to multiple parts for a nicer user-experience, I will take a look into it but it may take some time until I have some spare time to do that (bumping the version & fixing the TS issue have a higher priority).

@bramkragten
Copy link
Member

I think that if you add this config to the documentation, you can stick to a switch. Add a service to the switch and an input select to the docs for the fan speed and I think it's great!

@jcastro
Copy link

jcastro commented Jun 7, 2017

Yes I was goIng to add it, now waiting on what @rytilahti wants to do!

@jcastro
Copy link

jcastro commented Jun 11, 2017

@rytilahti hey! would be great if you can let me know what are your plans for this components, keep it as a switch for now or create a platform as it was suggested so I can continue with documentation or wait! thanks :)

Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine.

@balloob
Copy link
Member

balloob commented Jun 12, 2017

@rytilahti let us know what you want to do. We can let this go in as is. If you plan on adding platforms to other components, you probably want to split it out. Have a component that handles the auth and have platforms for switch, sensors etc. For now you could just simply extract auth and move it to a component. That way at least we won't have to make breaking changes in the future.

@rytilahti
Copy link
Member Author

I think it makes sense to convert this into a component before rushing it in, especially considering the potential future support for other device types (air purifier, ..). I'm currently occupied with other responsibilities but I hope I'll find some time during the weekend to make the conversion and fix the issue with newer fw versions.

@jcastro
Copy link

jcastro commented Jun 12, 2017

@rytilahti great! please ping me when you start working on it to align it with the new documentation!

@andrey-git
Copy link
Contributor

This should either be merged or closed.

@rytilahti
Copy link
Member Author

So, the bug causing newer firmware versions not to work with the component is now fixed. I'll prepare a new release & bump the version soon enough. Other than that I think (considering that there has been no process in adding support for other types of devices), I think this can be merged now as a platform and if, in the future, support for other devices gets added we can reconsider converting this into a component.

@jcastro I've read something about token discovery not working anymore with newer firmware releases, but the token has to be extracted from a paired (apple/android) device. I think the documentation may need to be updated to handle this.

@balloob
Copy link
Member

balloob commented Jul 7, 2017

Alright @rytilahti, will merge this. You can bump the dependency when you made it work with newer firmware versions.

@balloob balloob merged commit 8a7cfce into home-assistant:dev Jul 7, 2017
@balloob
Copy link
Member

balloob commented Jul 7, 2017

@jcastro please address the comments on the documentation PR

@jcastro
Copy link

jcastro commented Jul 7, 2017

@rytilahti do you have any info on how to get the token from the mobile app? I have taken a look at my iPhone but I couldn't find it anywhere in the app

@rytilahti
Copy link
Member Author

@jcastro the information is stored in a sqlite file ac cording to the comments here, so one needs some extra tools both on iPhone and on Android: aholstenson/miio#40 (comment) onwards. I'm unsure what's the best way to describe this all in a simple manner..

@balloob balloob mentioned this pull request Jul 13, 2017
@rytilahti rytilahti deleted the xiaomi_vacuum branch July 22, 2017 16:36
dethpickle pushed a commit to dethpickle/home-assistant that referenced this pull request Aug 18, 2017
…sistant#7913)

* add component for xiaomi robot vacuum (switch.xiaomi_vacuum)

* enforce token length, update requirements_all.txt and .coveragerc

* bump version to avoid catching generic exception
@home-assistant home-assistant locked and limited conversation to collaborators Oct 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants