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

Update Xiaomi Vacuum to new StateVacuumDevice #15643

Merged
merged 6 commits into from
Aug 13, 2018

Conversation

cnrd
Copy link
Contributor

@cnrd cnrd commented Jul 23, 2018

Description:

Adds support for states to the Xiaomi Vacuum Platform, this can be merged after: #15573

It also introduce breaking changes, as it no longer uses:

  • STATE_ON
  • STATE_OFF

Also removes any functions related to ToggleEntity, which results in the following services no longer working:

  • turn_on (Use start instead)
  • turn_off (Use return_to_dock instead)
  • toggle (No replacement)

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

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

try:
return STATE_CODE_TO_STATE[int(self.vacuum_state.state_code)]
except KeyError:
_LOGGER.error("STATE not supported: %s, state_code: %s, please report",

Choose a reason for hiding this comment

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

line too long (87 > 79 characters)

@cnrd
Copy link
Contributor Author

cnrd commented Jul 25, 2018

@syssi @rytilahti Can you take a look at this, so that we may get this in the same release as #15573 ? :-)

@fabaff fabaff changed the title Xiaomi Vacuum: Add support for states Add support for states of the Xiaomi Vacuum Jul 29, 2018
@arbreng
Copy link
Contributor

arbreng commented Aug 1, 2018

Part of: home-assistant/architecture#29

@cnrd cnrd changed the title Add support for states of the Xiaomi Vacuum Update Xiaomi Vacuum to new StateVacuumDevice Aug 12, 2018
@cnrd
Copy link
Contributor Author

cnrd commented Aug 12, 2018

@rytilahti @syssi Sorry for asking again, but could one of you chime in on this, such that it can be a part of the same release as the Component changes in: #15751

Copy link
Member

@syssi syssi left a comment

Choose a reason for hiding this comment

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

LGTM. I cannot test the changes because I don't own the device.

@cnrd
Copy link
Contributor Author

cnrd commented Aug 13, 2018

I have been running it here since I pushed the changes to the PR without problems.

@fabaff fabaff merged commit 3a60c8b into home-assistant:dev Aug 13, 2018
@ghost ghost removed the in progress label Aug 13, 2018
@balloob balloob mentioned this pull request Aug 29, 2018
@marciogranzotto
Copy link
Contributor

@cnrd this broke my node-red automation. The state doesn't have enough information anymore. When calling the service xiaomi_remote_control_move_step I used to get state change from charging to manual mode and then from manual mode to idle. Now I get from idle to unknown and then unknown to idle.

@cnrd
Copy link
Contributor Author

cnrd commented Sep 4, 2018

The proposal have been open for a long time: home-assistant/architecture#29 remote was not identified as a common state and as such is not supported. The best thing we can do currently is change that unknown to cleaning.

I'm not sure how many vacuums support remote, so you may have to propose to have it added in that thread such that people who use other vacuums can chip in on wether it makes sense to add that state or not.

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Sep 4, 2018
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

8 participants