-
-
Notifications
You must be signed in to change notification settings - Fork 32k
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 support for ZhongHong HVAC Controllers #14552
Changes from 19 commits
75248c4
9de5a40
ac03c04
09ab2c5
94bf7cc
af34d34
55ac431
1c739d8
9b27983
fcfbfa1
27cfc66
4804146
7e45892
cee04b9
1d7fa50
6f1f994
760ef67
09229fa
fc8d8fa
41bcba4
183f503
5cab3f6
c2ac55c
1fa2693
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,227 @@ | ||
""" | ||
Support for ZhongHong HVAC Controller. | ||
|
||
For more details about this platform, please refer to the documentation at | ||
https://home-assistant.io/components/climate.zhong_hong/ | ||
""" | ||
import logging | ||
|
||
import voluptuous as vol | ||
|
||
from homeassistant.components.climate import ( | ||
ATTR_OPERATION_MODE, PLATFORM_SCHEMA, STATE_COOL, STATE_DRY, | ||
STATE_FAN_ONLY, STATE_HEAT, SUPPORT_FAN_MODE, SUPPORT_ON_OFF, | ||
SUPPORT_OPERATION_MODE, SUPPORT_TARGET_TEMPERATURE, ClimateDevice) | ||
from homeassistant.const import (ATTR_TEMPERATURE, CONF_HOST, CONF_PORT, | ||
EVENT_HOMEASSISTANT_STOP, TEMP_CELSIUS) | ||
from homeassistant.exceptions import PlatformNotReady | ||
import homeassistant.helpers.config_validation as cv | ||
from homeassistant.helpers.dispatcher import (async_dispatcher_connect, | ||
async_dispatcher_send) | ||
|
||
_LOGGER = logging.getLogger(__name__) | ||
|
||
CONF_GATEWAY_ADDRRESS = 'gateway_address' | ||
|
||
REQUIREMENTS = ['zhong_hong_hvac==1.0.9'] | ||
SIGNAL_DEVICE_ADDED = 'zhong_hong_device_added' | ||
SIGNAL_ZHONG_HONG_HUB_START = 'zhong_hong_hub_start' | ||
|
||
PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ | ||
vol.Required(CONF_HOST): | ||
cv.string, | ||
vol.Optional(CONF_PORT, default=9999): | ||
vol.Coerce(int), | ||
vol.Optional(CONF_GATEWAY_ADDRRESS, default=1): | ||
vol.Coerce(int), | ||
}) | ||
|
||
|
||
def setup_platform(hass, config, add_devices, discovery_info=None): | ||
"""Set up the ZhongHong HVAC platform.""" | ||
from zhong_hong_hvac.hub import ZhongHongGateway | ||
host = config.get(CONF_HOST) | ||
port = config.get(CONF_PORT) | ||
gw_addr = config.get(CONF_GATEWAY_ADDRRESS) | ||
hub = ZhongHongGateway(host, port, gw_addr) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess that there will be a traceback if the controller is not available. You need to decide what to do. Make the setup of the platform fails or raise |
||
try: | ||
devices = [ | ||
ZhongHongClimate(hub, addr_out, addr_in) | ||
for (addr_out, addr_in) in hub.discovery_ac() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is doing I/O via socket and is not async safe. I would change back to the sync api and use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can I still use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but I would instead change to |
||
] | ||
|
||
except Exception as exc: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reading the library, it looks like you already catch There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe I will let the exception through out in the future. Must I remove the stupid There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should never catch a bare exception. You should always only catch the exceptions that you expect. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So if you're using a Socket, see what the base class is and catch that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has to be updated to be the exception that is raised. You are not allowed to catch a bare exception There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I missed this comment. It is really not necessary to catch this exception. Because I catch the exception in the device library. |
||
_LOGGER.error("ZhongHong controller is not ready", exc_info=exc) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't print the full stack trace of the exception, that will confuse users. Stringify the exception instead |
||
raise PlatformNotReady | ||
|
||
_LOGGER.debug("We got %s zhong_hong climate devices", len(devices)) | ||
|
||
hub_is_initialized = False | ||
|
||
async def startup(): | ||
"""Start hub socket after all climate entity is setted up.""" | ||
nonlocal hub_is_initialized | ||
if not all([device.is_initialized for device in devices]): | ||
return | ||
|
||
if hub_is_initialized: | ||
return | ||
|
||
_LOGGER.debug("zhong_hong hub start listen event") | ||
await hass.async_add_job(hub.start_listen) | ||
await hass.async_add_job(hub.query_all_status) | ||
hub_is_initialized = True | ||
|
||
async_dispatcher_connect(hass, SIGNAL_DEVICE_ADDED, startup) | ||
|
||
# add devices after SIGNAL_DEVICE_SETTED_UP event is listend | ||
add_devices(devices) | ||
|
||
def stop_listen(event): | ||
"""Stop ZhongHongHub socket.""" | ||
hub.stop_listen() | ||
|
||
hass.bus.listen_once(EVENT_HOMEASSISTANT_STOP, stop_listen) | ||
|
||
|
||
class ZhongHongClimate(ClimateDevice): | ||
"""Representation of a ZhongHong controller support HVAC.""" | ||
|
||
def __init__(self, hub, addr_out, addr_in): | ||
"""Set up the ZhongHong climate devices.""" | ||
from zhong_hong_hvac.hvac import HVAC | ||
self._device = HVAC(hub, addr_out, addr_in) | ||
self._hub = hub | ||
self._current_operation = None | ||
self._current_temperature = None | ||
self._target_temperature = None | ||
self._current_fan_mode = None | ||
self._is_on = None | ||
self.is_initialized = False | ||
|
||
async def async_added_to_hass(self): | ||
"""Register callbacks.""" | ||
self._device.register_update_callback(self._after_update) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be moved to the entity coroutine method There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you! Finally found the root cause of starting problem |
||
self.is_initialized = True | ||
async_dispatcher_send(self.hass, SIGNAL_DEVICE_ADDED) | ||
|
||
def update(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is never called? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose home assistant core logic will invoke this method in some situation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only if |
||
"""Update device status from hub.""" | ||
self._device.update() | ||
|
||
def _after_update(self, climate): | ||
"""Callback to update state.""" | ||
_LOGGER.debug("async update ha state") | ||
if self._device.current_operation: | ||
self._current_operation = self._device.current_operation.lower() | ||
if self._device.current_temperature: | ||
self._current_temperature = self._device.current_temperature | ||
if self._device.current_fan_mode: | ||
self._current_fan_mode = self._device.current_fan_mode | ||
if self._device.target_temperature: | ||
self._target_temperature = self._device.target_temperature | ||
self._is_on = self._device.is_on | ||
self.schedule_update_ha_state() | ||
|
||
@property | ||
def should_poll(self): | ||
"""Return the polling state.""" | ||
return False | ||
|
||
@property | ||
def name(self): | ||
"""Return the name of the thermostat, if any.""" | ||
return self.unique_id | ||
|
||
@property | ||
def unique_id(self): | ||
"""Return the unique ID of the HVAC.""" | ||
return "zhong_hong_hvac_{}_{}".format(self._device.addr_out, | ||
self._device.addr_in) | ||
|
||
@property | ||
def supported_features(self): | ||
"""Return the list of supported features.""" | ||
return (SUPPORT_TARGET_TEMPERATURE | SUPPORT_FAN_MODE | ||
| SUPPORT_OPERATION_MODE | SUPPORT_ON_OFF) | ||
|
||
@property | ||
def temperature_unit(self): | ||
"""Return the unit of measurement used by the platform.""" | ||
return TEMP_CELSIUS | ||
|
||
@property | ||
def current_operation(self): | ||
"""Return current operation ie. heat, cool, idle.""" | ||
return self._current_operation | ||
|
||
@property | ||
def operation_list(self): | ||
"""Return the list of available operation modes.""" | ||
return [STATE_COOL, STATE_HEAT, STATE_DRY, STATE_FAN_ONLY] | ||
|
||
@property | ||
def current_temperature(self): | ||
"""Return the current temperature.""" | ||
return self._current_temperature | ||
|
||
@property | ||
def target_temperature(self): | ||
"""Return the temperature we try to reach.""" | ||
return self._target_temperature | ||
|
||
@property | ||
def target_temperature_step(self): | ||
"""Return the supported step of target temperature.""" | ||
return 1 | ||
|
||
@property | ||
def is_on(self): | ||
"""Return true if on.""" | ||
return self._device.is_on | ||
|
||
@property | ||
def current_fan_mode(self): | ||
"""Return the fan setting.""" | ||
return self._current_fan_mode | ||
|
||
@property | ||
def fan_list(self): | ||
"""Return the list of available fan modes.""" | ||
return self._device.fan_list | ||
|
||
@property | ||
def min_temp(self): | ||
"""Return the minimum temperature.""" | ||
return self._device.min_temp | ||
|
||
@property | ||
def max_temp(self): | ||
"""Return the maximum temperature.""" | ||
return self._device.max_temp | ||
|
||
def turn_on(self): | ||
"""Turn on ac.""" | ||
return self._device.turn_on() | ||
|
||
def turn_off(self): | ||
"""Turn off ac.""" | ||
return self._device.turn_off() | ||
|
||
def set_temperature(self, **kwargs): | ||
"""Set new target temperature.""" | ||
temperature = kwargs.get(ATTR_TEMPERATURE) | ||
if temperature is not None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this cannot happen There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just make the code looks like the same. Is that really matter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The set temperature service schema allows two ways of providing temperature, either with one temperature or with high and low temperatures. So the case on this line here can happen and is a good check. |
||
self._device.set_temperature(temperature) | ||
|
||
operation_mode = kwargs.get(ATTR_OPERATION_MODE) | ||
if operation_mode is not None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mode is not part of setting a temperature? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. operation_mode is passed in at here https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/climate/__init__.py#L190 |
||
self._device.set_operation_mode(operation_mode) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here you don't cast the operation mode to upper case like below. Is that not needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope, you are right. I think I should use |
||
|
||
def set_operation_mode(self, operation_mode): | ||
"""Set new target operation mode.""" | ||
self._device.set_operation_mode(operation_mode.upper()) | ||
|
||
def set_fan_mode(self, fan_mode): | ||
"""Set new target fan mode.""" | ||
self._device.set_fan_mode(fan_mode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'homeassistant.exceptions.PlatformNotReady' imported but unused