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 homematicip cloud climate platform #14388

Merged
merged 9 commits into from May 26, 2018

Conversation

Projects
None yet
4 participants
@mxworm
Contributor

mxworm commented May 11, 2018

Description:

  • Add support for HomematicIP climate devices.
  • Update to latest homematicip-rest-api lib

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

Checklist:

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

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

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

  • 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.

@mxworm mxworm changed the title from [WIP] Add homematicip cloud climate platform to Add homematicip cloud climate platform May 11, 2018

@mxworm

This comment has been minimized.

Contributor

mxworm commented May 14, 2018

@sander76 now I have implemented the climate group as well. Have a look....

@mxworm mxworm referenced this pull request May 18, 2018

Merged

Add HomematicIP Cloud climate device #5396

2 of 2 tasks complete
@MartinHjelmare

Only climate base component states are allowed as operation modes.

Use two dicts that map home assistant climate states and homematicip device modes. Then use those dicts to move between the states and modes in the methods that need the states or modes.

class HomematicipHeatingGroup(HomematicipGenericDevice, ClimateDevice):
"""Representation of a MomematicIP heating group."""
def __init__(self, hass, home, device):

This comment has been minimized.

@MartinHjelmare

MartinHjelmare May 19, 2018

Member

hass isn't used.

@property
def icon(self):
"""Return the icon."""
return 'mdi:nest-thermostat'

This comment has been minimized.

@MartinHjelmare

MartinHjelmare May 19, 2018

Member

The nest icon is deprecated. Use mdi:thermostat.

def current_operation(self):
"""Return current operation (auto, manual, boost, vacation)."""
if self._device.boostMode:
return STATE_BOOST

This comment has been minimized.

@MartinHjelmare

MartinHjelmare May 19, 2018

Member

Only states present in the climate base component are allowed. If you want to add a new state, it needs to pass consensus in an issue in the architecture repo first.

This comment has been minimized.

@mxworm

mxworm May 24, 2018

Contributor

@MartinHjelmare STATE_BOOST is also used in homematic climate platform. This is quite useful for users as this turns the heating on for a few minutes and changes then back to automatic mode. The feature is named in all Homematic(IP) boost mode.

This comment has been minimized.

@MartinHjelmare

MartinHjelmare May 24, 2018

Member

That doesn't change how we should do things. Changes to entity model needs to go through review in the architecture repo:
https://developers.home-assistant.io/docs/en/entity_index.html#changing-the-entity-model

There's already an issue about climate model:
home-assistant/architecture#22

modes = []
for profile in self._device.profiles:
if profile.name != '' and profile.enabled and profile.visible:
modes.append(profile.name)

This comment has been minimized.

@MartinHjelmare

MartinHjelmare May 19, 2018

Member

See above.

This comment has been minimized.

@mxworm

mxworm May 24, 2018

Contributor

@MartinHjelmare Then I need your advise: The Homematic(IP) devices have in automatic mode dedicated time and temperature profiles. Users would definitely like to switch between them. With this implementation I add the profile names (the user can name them as the like in the app) and use them like automatic states. How should I implement this feature?
Thanks Mattias

This comment has been minimized.

@MartinHjelmare

MartinHjelmare May 24, 2018

Member

I'd wait with implementing any operation modes that are not already listed in the climate component, for this PR. When it's been decided how the future of the climate component will be, it'll be easier to know how to handle these things.

else:
await self._device.set_boost(False)
index = 0
for profile in self._device.profiles:

This comment has been minimized.

@MartinHjelmare

MartinHjelmare May 19, 2018

Member

Use enumerate to get the index of the profile.

async def async_set_operation_mode(self, operation_mode):
"""Set operation mode."""
if operation_mode == 'Boost':

This comment has been minimized.

@MartinHjelmare

MartinHjelmare May 19, 2018

Member

See above.

@mxworm mxworm changed the title from Add homematicip cloud climate platform to [WIP] Add homematicip cloud climate platform May 24, 2018

def current_operation(self):
"""Return current operation ie. automatic or manual."""
return HMIP_STATE_TO_HA.get(self._device.controlMode)

This comment has been minimized.

@houndci-bot

houndci-bot May 24, 2018

blank line contains whitespace

mxworm added some commits May 24, 2018

@mxworm mxworm changed the title from [WIP] Add homematicip cloud climate platform to Add homematicip cloud climate platform May 24, 2018

@home-assistant home-assistant deleted a comment from houndci-bot May 25, 2018

@home-assistant home-assistant deleted a comment from houndci-bot May 25, 2018

@MartinHjelmare MartinHjelmare merged commit 7ea25cd into home-assistant:dev May 26, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.5%) to 94.462%
Details

iMicknl added a commit to iMicknl/home-assistant that referenced this pull request May 31, 2018

Add homematicip cloud climate platform (home-assistant#14388)
* Add support for climatic devices

* Update requirements_all

* Change icon to mdi:thermostat

* Update of homematicip-rest-api lib version

* Remove all mode or state relevant things - will come later

* Add current_operation again to see proper status

* Remove STATE_PERFORMANCE import

* Remove trailing whitespace

* Update requirements file

@balloob balloob referenced this pull request Jun 8, 2018

Merged

0.71.0 #14876

cyberjacob pushed a commit to cyberjacob/home-assistant that referenced this pull request Sep 4, 2018

Add homematicip cloud climate platform (home-assistant#14388)
* Add support for climatic devices

* Update requirements_all

* Change icon to mdi:thermostat

* Update of homematicip-rest-api lib version

* Remove all mode or state relevant things - will come later

* Add current_operation again to see proper status

* Remove STATE_PERFORMANCE import

* Remove trailing whitespace

* Update requirements file

@home-assistant home-assistant locked and limited conversation to collaborators Sep 5, 2018

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