Adding climate.velbus support #18100
Adding climate.velbus support #18100fabaff merged 5 commits intohome-assistant:devfrom cereal2nd:dev
Conversation
ghost
added
the
|
|
||
| DEPENDENCIES = ['velbus'] | ||
|
|
||
| OPERATION_LIST = ['comfort', 'day', 'night', 'safe'] |
There was a problem hiding this comment.
This is wrong. Only states imported from the base climate component is allowed.
There was a problem hiding this comment.
I don't see how we can get all states to work in this case ...3
What do you suggest we use as a mapping?
There was a problem hiding this comment.
I don't know the integration or what those modes do, so I don't have a suggestion at the moment.
There was a problem hiding this comment.
basically those 4 are are all temperature presets
There was a problem hiding this comment.
The home assistant operation state explains how the thermostat tries to adjust temperature, eg heat, cool, auto, eco. It doesn't say what temperature the thermostat should set. We have target_temperature and set_temperature for that.
If I understand correctly 'comfort', 'day', 'night', 'safe' are levels of temperature the thermostat should try to keep? If so, we shouldn't use operation mode in home assistant to change those.
There was a problem hiding this comment.
so this means i will only have on operation mode in HA (heat)
and how do i handle the temperature levels?
There was a problem hiding this comment.
If we can set a specific temperature degree with set_temperature these modes are not required, right? I don't think we currently have any matching control options for these modes. But look at the base climate component features.
| @property | ||
| def current_operation(self): | ||
| """Return current operation ie. heat, cool, idle.""" | ||
| return self._module.get_climate_mode() |
There was a problem hiding this comment.
This should return one of the home assistant climate states.
|
|
||
| def set_operation_mode(self, operation_mode): | ||
| """Set new target operation mode.""" | ||
| self._module.set_mode(operation_mode) |
There was a problem hiding this comment.
We need to convert from home assistant climate state to device mode and vice versa. Define two constant dicts at the module level that do this.
| def set_operation_mode(self, operation_mode): | ||
| """Set new target operation mode.""" | ||
| self._module.set_mode(operation_mode) | ||
| self.schedule_update_ha_state() |
There was a problem hiding this comment.
Remove this. Let the state update callback update state.
| """Set new target temperatures.""" | ||
| if kwargs.get(ATTR_TEMPERATURE) is not None: | ||
| self._module.set_temp(kwargs.get(ATTR_TEMPERATURE)) | ||
| self.schedule_update_ha_state() |
|
@MartinHjelmare is right, this platform does not follow our climate entity specification. To avoid a future breaking change, I will remove this and it can be submitted again with the right operation modes. |
6 participants
Description:
Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#7323
Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
REQUIREMENTSvariable ([example][ex-requir]).requirements_all.txtby runningscript/gen_requirements_all.py.