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

Fix homekit_controller on climate-1.0 #24948

Merged
merged 3 commits into from
Jul 5, 2019

Conversation

Jc2k
Copy link
Member

@Jc2k Jc2k commented Jul 4, 2019

Description:

This fixes the tests for homekit_controller climate on the climate-1.0 branch. It also fixes a typo in climate (humitidy vs humidity)

Related issue (if applicable): #23899

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the [development checklist][dev-checklist]

Copy link
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

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

   @property
    def hvac_mode(self):
        """Return current operation ie. heat, cool, idle."""
        return self._current_mode

That is wrong. If you have a current action they match with CURRENT_* use hvac_action property.

@Jc2k
Copy link
Member Author

Jc2k commented Jul 5, 2019

Re: hvac_mode - the docstring in homekit_controller was wrong, and the variable names were confusing but the implementation was doing the right thing i think so i have:

  • Fixed the docstrings based on what is in climate now
  • Renamed _current_mode to _target_mode and documented what Apple think it means at the point where it is populated. It should now contain the mode the user has selected.
  • _current_mode is now actually set to the current operating mode (it was previously getting put in self._state) with the CURRENT_ stuff. I.e. it contains what the device reports it is currently doing.
  • Have added support for hvac_action
  • Added a test to show hvac_mode and hvac_action are independent and not getting conflated in homekit_controller.

The homekit tests are still passing with these changes, and flake8 and pylint is passing for the homekit bits too.

@pvizeli pvizeli merged commit f8ca4cc into home-assistant:climate-1.0 Jul 5, 2019
@lock lock bot locked and limited conversation to collaborators Jul 6, 2019
@Jc2k Jc2k deleted the climate-1.0 branch July 18, 2019 10:42
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.

3 participants