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

Merged
merged 9 commits into from May 26, 2018
Merged

Add homematicip cloud climate platform #14388

merged 9 commits into from May 26, 2018

Conversation

worm-ee
Copy link
Contributor

@worm-ee worm-ee 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.

@worm-ee worm-ee changed the title [WIP] Add homematicip cloud climate platform Add homematicip cloud climate platform May 11, 2018
@worm-ee
Copy link
Contributor Author

worm-ee commented May 14, 2018

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

Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

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):
Copy link
Member

Choose a reason for hiding this comment

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

hass isn't used.

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

Choose a reason for hiding this comment

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

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
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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)
Copy link
Member

Choose a reason for hiding this comment

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

See above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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

Copy link
Member

Choose a reason for hiding this comment

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

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:
Copy link
Member

Choose a reason for hiding this comment

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

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':
Copy link
Member

Choose a reason for hiding this comment

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

See above.

@worm-ee worm-ee changed the title Add homematicip cloud climate platform [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)

Choose a reason for hiding this comment

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

blank line contains whitespace

@worm-ee worm-ee changed the title [WIP] Add homematicip cloud climate platform 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
@balloob balloob mentioned this pull request Jun 8, 2018
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
* 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants