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

HmIP thermostat fix with operations #18068

Merged
merged 2 commits into from Nov 6, 2018

Conversation

Projects
None yet
4 participants
@pvizeli
Member

pvizeli commented Oct 31, 2018

Description:

Fix handling with HmIP thermostats like HM devices.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
op_list = []
# HMIP use set_point_mode for operation
if HMIP_CONTROL_MODE in self._data:
return [STATE_MANUAL, STATE_AUTO, STATE_BOOST]

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Oct 31, 2018

Member

STATE_BOOST isn't part of climate base component, so we're not allowed to use that for operation mode.

Also, the platform is redefining STATE_MANUAL etc, instead of importing these from the climate component.

This comment has been minimized.

@pvizeli

pvizeli Nov 1, 2018

Member

That is correct. I let it stay like it is because I refactor the hole climate on my new work next year.

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Nov 1, 2018

Member

Ok. I would like to see a bit longer description in this PR on background and scope. Why do we do these changes? We would expect that from a contributor.

This comment has been minimized.

@balloob

balloob Nov 6, 2018

Member

So right now, any platform that is using unsupported states can keep using it. We should no longer add new ones.

Pascal will soon work full time on HA via Nabu Casa, and one of his first tasks will be to tackle the climate cleanup.

@MartinHjelmare

I think it sounds good to make an intermediate fix now, and wait with a breaking change to later.

I don't know the integration or the background of these changes, so I can't really judge the changes. So merge when it's ready 👍.

@pvizeli pvizeli merged commit f4d3d59 into dev Nov 6, 2018

6 checks passed

Hound No violations found. Woof!
WIP Legacy commit status override — see details
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.005%) to 93.088%
Details

@wafflebot wafflebot bot removed the in progress label Nov 6, 2018

@pvizeli pvizeli deleted the hmip-state branch Nov 6, 2018

zxdavb added a commit to zxdavb/home-assistant that referenced this pull request Nov 13, 2018

HmIP thermostat fix with operations (home-assistant#18068)
* HmIP thermostat fix with operations

* Update homematic.py

@balloob balloob referenced this pull request Nov 29, 2018

Merged

0.83 #18776

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment