From 233bc1a1080149b9abd09e663293d63146b14025 Mon Sep 17 00:00:00 2001 From: Phil Bruckner Date: Fri, 7 Jun 2019 23:46:49 -0500 Subject: [PATCH] Improve amcrest error handling and bump amcrest package to 1.5.3 (#24262) * Improve amcrest error handling and bump amcrest package to 1.5.3 amcrest package update fixes command retry, especially with Digest Authentication, and allows sending snapshot command without channel parameter. Get rid of persistent_notification. Errors at startup, other than login errors, are no longer fatal. Display debug messages about how many times an error has occurred in a row. Remove initial communications test. If camera is off line at startup this just delays the component setup. Handle urllib3 errors when getting data from commands that were sent with stream=True. If errors occur during camera update, try repeating until it works or the camera is determined to be off line. Drop channel parameter in snapshot command which allows camera to use its default channel, which is different in different camera models and firmware versions. Make entities unavailable if too many errors occur in a row. Add new configuration variables to control how many errors in a row should be interpreted as camera being offline, and how frequently to "ping" camera to see when it becomes available again. Add online binary_sensor option to indicate if camera is available (i.e., responding to commands.) * Update per review comments Remove max_errors and recheck_interval configuration variables and used fixed values instead. Move definition of AmcrestChecker class to module level. Change should_poll in camera.py to return a fixed value of True and move logic to update method. --- homeassistant/components/amcrest/__init__.py | 118 ++++++++++---- .../components/amcrest/binary_sensor.py | 61 +++++-- homeassistant/components/amcrest/camera.py | 151 +++++++++++------- homeassistant/components/amcrest/const.py | 4 + homeassistant/components/amcrest/helpers.py | 15 +- .../components/amcrest/manifest.json | 2 +- homeassistant/components/amcrest/sensor.py | 84 +++++++--- homeassistant/components/amcrest/switch.py | 79 ++++++--- requirements_all.txt | 2 +- 9 files changed, 363 insertions(+), 153 deletions(-) diff --git a/homeassistant/components/amcrest/__init__.py b/homeassistant/components/amcrest/__init__.py index 58df1d8e504..1c9303b2c52 100644 --- a/homeassistant/components/amcrest/__init__.py +++ b/homeassistant/components/amcrest/__init__.py @@ -1,9 +1,10 @@ """Support for Amcrest IP cameras.""" import logging from datetime import timedelta +import threading import aiohttp -from amcrest import AmcrestCamera, AmcrestError +from amcrest import AmcrestError, Http, LoginError import voluptuous as vol from homeassistant.auth.permissions.const import POLICY_CONTROL @@ -18,12 +19,14 @@ from homeassistant.const import ( from homeassistant.exceptions import Unauthorized, UnknownUser from homeassistant.helpers import discovery import homeassistant.helpers.config_validation as cv -from homeassistant.helpers.dispatcher import async_dispatcher_send +from homeassistant.helpers.dispatcher import ( + async_dispatcher_send, dispatcher_send) +from homeassistant.helpers.event import track_time_interval from homeassistant.helpers.service import async_extract_entity_ids -from .binary_sensor import BINARY_SENSORS +from .binary_sensor import BINARY_SENSOR_MOTION_DETECTED, BINARY_SENSORS from .camera import CAMERA_SERVICES, STREAM_SOURCE_LIST -from .const import DOMAIN, DATA_AMCREST +from .const import CAMERAS, DOMAIN, DATA_AMCREST, DEVICES, SERVICE_UPDATE from .helpers import service_signal from .sensor import SENSOR_MOTION_DETECTOR, SENSORS from .switch import SWITCHES @@ -39,6 +42,8 @@ DEFAULT_NAME = 'Amcrest Camera' DEFAULT_PORT = 80 DEFAULT_RESOLUTION = 'high' DEFAULT_ARGUMENTS = '-pred 1' +MAX_ERRORS = 5 +RECHECK_INTERVAL = timedelta(minutes=1) NOTIFICATION_ID = 'amcrest_notification' NOTIFICATION_TITLE = 'Amcrest Camera Setup' @@ -58,20 +63,21 @@ AUTHENTICATION_LIST = { def _deprecated_sensor_values(sensors): if SENSOR_MOTION_DETECTOR in sensors: _LOGGER.warning( - "The 'sensors' option value '%s' is deprecated, " + "The '%s' option value '%s' is deprecated, " "please remove it from your configuration and use " - "the 'binary_sensors' option with value 'motion_detected' " - "instead.", SENSOR_MOTION_DETECTOR) + "the '%s' option with value '%s' instead", + CONF_SENSORS, SENSOR_MOTION_DETECTOR, CONF_BINARY_SENSORS, + BINARY_SENSOR_MOTION_DETECTED) return sensors def _deprecated_switches(config): if CONF_SWITCHES in config: _LOGGER.warning( - "The 'switches' option (with value %s) is deprecated, " + "The '%s' option (with value %s) is deprecated, " "please remove it from your configuration and use " - "camera services and attributes instead.", - config[CONF_SWITCHES]) + "services and attributes instead", + CONF_SWITCHES, config[CONF_SWITCHES]) return config @@ -115,33 +121,81 @@ CONFIG_SCHEMA = vol.Schema({ }, extra=vol.ALLOW_EXTRA) +# pylint: disable=too-many-ancestors +class AmcrestChecker(Http): + """amcrest.Http wrapper for catching errors.""" + + def __init__(self, hass, name, host, port, user, password): + """Initialize.""" + self._hass = hass + self._wrap_name = name + self._wrap_errors = 0 + self._wrap_lock = threading.Lock() + self._unsub_recheck = None + super().__init__(host, port, user, password, retries_connection=1, + timeout_protocol=3.05) + + @property + def available(self): + """Return if camera's API is responding.""" + return self._wrap_errors <= MAX_ERRORS + + def command(self, cmd, retries=None, timeout_cmd=None, stream=False): + """amcrest.Http.command wrapper to catch errors.""" + try: + ret = super().command(cmd, retries, timeout_cmd, stream) + except AmcrestError: + with self._wrap_lock: + was_online = self.available + self._wrap_errors += 1 + _LOGGER.debug('%s camera errs: %i', self._wrap_name, + self._wrap_errors) + offline = not self.available + if offline and was_online: + _LOGGER.error( + '%s camera offline: Too many errors', self._wrap_name) + dispatcher_send( + self._hass, + service_signal(SERVICE_UPDATE, self._wrap_name)) + self._unsub_recheck = track_time_interval( + self._hass, self._wrap_test_online, RECHECK_INTERVAL) + raise + with self._wrap_lock: + was_offline = not self.available + self._wrap_errors = 0 + if was_offline: + self._unsub_recheck() + self._unsub_recheck = None + _LOGGER.error('%s camera back online', self._wrap_name) + dispatcher_send( + self._hass, service_signal(SERVICE_UPDATE, self._wrap_name)) + return ret + + def _wrap_test_online(self, now): + """Test if camera is back online.""" + try: + self.current_time + except AmcrestError: + pass + + def setup(hass, config): """Set up the Amcrest IP Camera component.""" - hass.data.setdefault(DATA_AMCREST, {'devices': {}, 'cameras': []}) - devices = config[DOMAIN] + hass.data.setdefault(DATA_AMCREST, {DEVICES: {}, CAMERAS: []}) - for device in devices: + for device in config[DOMAIN]: name = device[CONF_NAME] username = device[CONF_USERNAME] password = device[CONF_PASSWORD] try: - api = AmcrestCamera(device[CONF_HOST], - device[CONF_PORT], - username, - password).camera - # pylint: disable=pointless-statement - # Test camera communications. - api.current_time + api = AmcrestChecker( + hass, name, + device[CONF_HOST], device[CONF_PORT], + username, password) - except AmcrestError as ex: - _LOGGER.error("Unable to connect to %s camera: %s", name, str(ex)) - hass.components.persistent_notification.create( - 'Error: {}
' - 'You will need to restart hass after fixing.' - ''.format(ex), - title=NOTIFICATION_TITLE, - notification_id=NOTIFICATION_ID) + except LoginError as ex: + _LOGGER.error("Login error for %s camera: %s", name, ex) continue ffmpeg_arguments = device[CONF_FFMPEG_ARGUMENTS] @@ -159,7 +213,7 @@ def setup(hass, config): else: authentication = None - hass.data[DATA_AMCREST]['devices'][name] = AmcrestDevice( + hass.data[DATA_AMCREST][DEVICES][name] = AmcrestDevice( api, authentication, ffmpeg_arguments, stream_source, resolution, control_light) @@ -189,7 +243,7 @@ def setup(hass, config): CONF_SWITCHES: switches }, config) - if not hass.data[DATA_AMCREST]['devices']: + if not hass.data[DATA_AMCREST][DEVICES]: return False def have_permission(user, entity_id): @@ -207,13 +261,13 @@ def setup(hass, config): if call.data.get(ATTR_ENTITY_ID) == ENTITY_MATCH_ALL: # Return all entity_ids user has permission to control. return [ - entity_id for entity_id in hass.data[DATA_AMCREST]['cameras'] + entity_id for entity_id in hass.data[DATA_AMCREST][CAMERAS] if have_permission(user, entity_id) ] call_ids = await async_extract_entity_ids(hass, call) entity_ids = [] - for entity_id in hass.data[DATA_AMCREST]['cameras']: + for entity_id in hass.data[DATA_AMCREST][CAMERAS]: if entity_id not in call_ids: continue if not have_permission(user, entity_id): diff --git a/homeassistant/components/amcrest/binary_sensor.py b/homeassistant/components/amcrest/binary_sensor.py index fe4eb25b3db..9489fc60d4d 100644 --- a/homeassistant/components/amcrest/binary_sensor.py +++ b/homeassistant/components/amcrest/binary_sensor.py @@ -5,17 +5,24 @@ import logging from amcrest import AmcrestError from homeassistant.components.binary_sensor import ( - BinarySensorDevice, DEVICE_CLASS_MOTION) + BinarySensorDevice, DEVICE_CLASS_CONNECTIVITY, DEVICE_CLASS_MOTION) from homeassistant.const import CONF_NAME, CONF_BINARY_SENSORS +from homeassistant.helpers.dispatcher import async_dispatcher_connect -from .const import BINARY_SENSOR_SCAN_INTERVAL_SECS, DATA_AMCREST +from .const import ( + BINARY_SENSOR_SCAN_INTERVAL_SECS, DATA_AMCREST, DEVICES, SERVICE_UPDATE) +from .helpers import log_update_error, service_signal _LOGGER = logging.getLogger(__name__) SCAN_INTERVAL = timedelta(seconds=BINARY_SENSOR_SCAN_INTERVAL_SECS) +BINARY_SENSOR_MOTION_DETECTED = 'motion_detected' +BINARY_SENSOR_ONLINE = 'online' +# Binary sensor types are defined like: Name, device class BINARY_SENSORS = { - 'motion_detected': 'Motion Detected' + BINARY_SENSOR_MOTION_DETECTED: ('Motion Detected', DEVICE_CLASS_MOTION), + BINARY_SENSOR_ONLINE: ('Online', DEVICE_CLASS_CONNECTIVITY), } @@ -26,7 +33,7 @@ async def async_setup_platform(hass, config, async_add_entities, return name = discovery_info[CONF_NAME] - device = hass.data[DATA_AMCREST]['devices'][name] + device = hass.data[DATA_AMCREST][DEVICES][name] async_add_entities( [AmcrestBinarySensor(name, device, sensor_type) for sensor_type in discovery_info[CONF_BINARY_SENSORS]], @@ -38,10 +45,18 @@ class AmcrestBinarySensor(BinarySensorDevice): def __init__(self, name, device, sensor_type): """Initialize entity.""" - self._name = '{} {}'.format(name, BINARY_SENSORS[sensor_type]) + self._name = '{} {}'.format(name, BINARY_SENSORS[sensor_type][0]) + self._signal_name = name self._api = device.api self._sensor_type = sensor_type self._state = None + self._device_class = BINARY_SENSORS[sensor_type][1] + self._unsub_dispatcher = None + + @property + def should_poll(self): + """Return True if entity has to be polled for state.""" + return self._sensor_type != BINARY_SENSOR_ONLINE @property def name(self): @@ -56,15 +71,39 @@ class AmcrestBinarySensor(BinarySensorDevice): @property def device_class(self): """Return device class.""" - return DEVICE_CLASS_MOTION + return self._device_class + + @property + def available(self): + """Return True if entity is available.""" + return self._sensor_type == BINARY_SENSOR_ONLINE or self._api.available def update(self): """Update entity.""" - _LOGGER.debug('Pulling data from %s binary sensor', self._name) + if not self.available: + return + _LOGGER.debug('Updating %s binary sensor', self._name) try: - self._state = self._api.is_motion_detected + if self._sensor_type == BINARY_SENSOR_MOTION_DETECTED: + self._state = self._api.is_motion_detected + + elif self._sensor_type == BINARY_SENSOR_ONLINE: + self._state = self._api.available except AmcrestError as error: - _LOGGER.error( - 'Could not update %s binary sensor due to error: %s', - self.name, error) + log_update_error( + _LOGGER, 'update', self.name, 'binary sensor', error) + + async def async_on_demand_update(self): + """Update state.""" + self.async_schedule_update_ha_state(True) + + async def async_added_to_hass(self): + """Subscribe to update signal.""" + self._unsub_dispatcher = async_dispatcher_connect( + self.hass, service_signal(SERVICE_UPDATE, self._signal_name), + self.async_on_demand_update) + + async def async_will_remove_from_hass(self): + """Disconnect from update signal.""" + self._unsub_dispatcher() diff --git a/homeassistant/components/amcrest/camera.py b/homeassistant/components/amcrest/camera.py index 3b8c8f38f8b..685d92d5ae6 100644 --- a/homeassistant/components/amcrest/camera.py +++ b/homeassistant/components/amcrest/camera.py @@ -1,6 +1,8 @@ """Support for Amcrest IP cameras.""" import asyncio +from datetime import timedelta import logging +from urllib3.exceptions import HTTPError from amcrest import AmcrestError import voluptuous as vol @@ -15,11 +17,14 @@ from homeassistant.helpers.aiohttp_client import ( async_get_clientsession) from homeassistant.helpers.dispatcher import async_dispatcher_connect -from .const import CAMERA_WEB_SESSION_TIMEOUT, DATA_AMCREST -from .helpers import service_signal +from .const import ( + CAMERA_WEB_SESSION_TIMEOUT, CAMERAS, DATA_AMCREST, DEVICES, SERVICE_UPDATE) +from .helpers import log_update_error, service_signal _LOGGER = logging.getLogger(__name__) +SCAN_INTERVAL = timedelta(seconds=15) + STREAM_SOURCE_LIST = [ 'snapshot', 'mjpeg', @@ -77,7 +82,7 @@ async def async_setup_platform(hass, config, async_add_entities, return name = discovery_info[CONF_NAME] - device = hass.data[DATA_AMCREST]['devices'][name] + device = hass.data[DATA_AMCREST][DEVICES][name] async_add_entities([ AmcrestCam(name, device, hass.data[DATA_FFMPEG])], True) @@ -106,23 +111,25 @@ class AmcrestCam(Camera): self._rtsp_url = None self._snapshot_lock = asyncio.Lock() self._unsub_dispatcher = [] + self._update_succeeded = False async def async_camera_image(self): """Return a still image response from the camera.""" - if not self.is_on: - _LOGGER.error( - 'Attempt to take snaphot when %s camera is off', self.name) + available = self.available + if not available or not self.is_on: + _LOGGER.warning( + 'Attempt to take snaphot when %s camera is %s', self.name, + 'offline' if not available else 'off') return None async with self._snapshot_lock: try: # Send the request to snap a picture and return raw jpg data response = await self.hass.async_add_executor_job( - self._api.snapshot, self._resolution) + self._api.snapshot) return response.data - except AmcrestError as error: - _LOGGER.error( - 'Could not get image from %s camera due to error: %s', - self.name, error) + except (AmcrestError, HTTPError) as error: + log_update_error( + _LOGGER, 'get image from', self.name, 'camera', error) return None async def handle_async_mjpeg_stream(self, request): @@ -131,6 +138,12 @@ class AmcrestCam(Camera): if self._stream_source == 'snapshot': return await super().handle_async_mjpeg_stream(request) + if not self.available: + _LOGGER.warning( + 'Attempt to stream %s when %s camera is offline', + self._stream_source, self.name) + return None + if self._stream_source == 'mjpeg': # stream an MJPEG image stream directly from the camera websession = async_get_clientsession(self.hass) @@ -160,6 +173,14 @@ class AmcrestCam(Camera): # Entity property overrides + @property + def should_poll(self) -> bool: + """Return True if entity has to be polled for state. + + False if entity pushes its state to HA. + """ + return True + @property def name(self): """Return the name of this camera.""" @@ -178,6 +199,11 @@ class AmcrestCam(Camera): attr[_ATTR_COLOR_BW] = self._color_bw return attr + @property + def available(self): + """Return True if entity is available.""" + return self._api.available + @property def supported_features(self): """Return supported features.""" @@ -216,6 +242,10 @@ class AmcrestCam(Camera): # Other Entity method overrides + async def async_on_demand_update(self): + """Update state.""" + self.async_schedule_update_ha_state(True) + async def async_added_to_hass(self): """Subscribe to signals and add camera to list.""" for service, params in CAMERA_SERVICES.items(): @@ -223,38 +253,37 @@ class AmcrestCam(Camera): self.hass, service_signal(service, self.entity_id), getattr(self, params[1]))) - self.hass.data[DATA_AMCREST]['cameras'].append(self.entity_id) + self._unsub_dispatcher.append(async_dispatcher_connect( + self.hass, service_signal(SERVICE_UPDATE, self._name), + self.async_on_demand_update)) + self.hass.data[DATA_AMCREST][CAMERAS].append(self.entity_id) async def async_will_remove_from_hass(self): """Remove camera from list and disconnect from signals.""" - self.hass.data[DATA_AMCREST]['cameras'].remove(self.entity_id) + self.hass.data[DATA_AMCREST][CAMERAS].remove(self.entity_id) for unsub_dispatcher in self._unsub_dispatcher: unsub_dispatcher() def update(self): """Update entity status.""" - _LOGGER.debug('Pulling data from %s camera', self.name) - if self._brand is None: - try: + if not self.available or self._update_succeeded: + if not self.available: + self._update_succeeded = False + return + _LOGGER.debug('Updating %s camera', self.name) + try: + if self._brand is None: resp = self._api.vendor_information.strip() if resp.startswith('vendor='): self._brand = resp.split('=')[-1] else: self._brand = 'unknown' - except AmcrestError as error: - _LOGGER.error( - 'Could not get %s camera brand due to error: %s', - self.name, error) - self._brand = 'unknwown' - if self._model is None: - try: - self._model = self._api.device_type.split('=')[-1].strip() - except AmcrestError as error: - _LOGGER.error( - 'Could not get %s camera model due to error: %s', - self.name, error) - self._model = 'unknown' - try: + if self._model is None: + resp = self._api.device_type.strip() + if resp.startswith('type='): + self._model = resp.split('=')[-1] + else: + self._model = 'unknown' self.is_streaming = self._api.video_enabled self._is_recording = self._api.record_mode == 'Manual' self._motion_detection_enabled = ( @@ -265,9 +294,11 @@ class AmcrestCam(Camera): self._color_bw = _CBW[self._api.day_night_color] self._rtsp_url = self._api.rtsp_url(typeno=self._resolution) except AmcrestError as error: - _LOGGER.error( - 'Could not get %s camera attributes due to error: %s', - self.name, error) + log_update_error( + _LOGGER, 'get', self.name, 'camera attributes', error) + self._update_succeeded = False + else: + self._update_succeeded = True # Other Camera method overrides @@ -343,9 +374,9 @@ class AmcrestCam(Camera): try: self._api.video_enabled = enable except AmcrestError as error: - _LOGGER.error( - 'Could not %s %s camera video stream due to error: %s', - 'enable' if enable else 'disable', self.name, error) + log_update_error( + _LOGGER, 'enable' if enable else 'disable', self.name, + 'camera video stream', error) else: self.is_streaming = enable self.schedule_update_ha_state() @@ -364,9 +395,9 @@ class AmcrestCam(Camera): self._api.record_mode = rec_mode[ 'Manual' if enable else 'Automatic'] except AmcrestError as error: - _LOGGER.error( - 'Could not %s %s camera recording due to error: %s', - 'enable' if enable else 'disable', self.name, error) + log_update_error( + _LOGGER, 'enable' if enable else 'disable', self.name, + 'camera recording', error) else: self._is_recording = enable self.schedule_update_ha_state() @@ -376,9 +407,9 @@ class AmcrestCam(Camera): try: self._api.motion_detection = str(enable).lower() except AmcrestError as error: - _LOGGER.error( - 'Could not %s %s camera motion detection due to error: %s', - 'enable' if enable else 'disable', self.name, error) + log_update_error( + _LOGGER, 'enable' if enable else 'disable', self.name, + 'camera motion detection', error) else: self._motion_detection_enabled = enable self.schedule_update_ha_state() @@ -388,9 +419,9 @@ class AmcrestCam(Camera): try: self._api.audio_enabled = enable except AmcrestError as error: - _LOGGER.error( - 'Could not %s %s camera audio stream due to error: %s', - 'enable' if enable else 'disable', self.name, error) + log_update_error( + _LOGGER, 'enable' if enable else 'disable', self.name, + 'camera audio stream', error) else: self._audio_enabled = enable self.schedule_update_ha_state() @@ -404,18 +435,18 @@ class AmcrestCam(Camera): 'configManager.cgi?action=setConfig&LightGlobal[0].Enable={}' .format(str(enable).lower())) except AmcrestError as error: - _LOGGER.error( - 'Could not %s %s camera indicator light due to error: %s', - 'enable' if enable else 'disable', self.name, error) + log_update_error( + _LOGGER, 'enable' if enable else 'disable', self.name, + 'indicator light', error) def _enable_motion_recording(self, enable): """Enable or disable motion recording.""" try: self._api.motion_recording = str(enable).lower() except AmcrestError as error: - _LOGGER.error( - 'Could not %s %s camera motion recording due to error: %s', - 'enable' if enable else 'disable', self.name, error) + log_update_error( + _LOGGER, 'enable' if enable else 'disable', self.name, + 'camera motion recording', error) else: self._motion_recording_enabled = enable self.schedule_update_ha_state() @@ -426,18 +457,18 @@ class AmcrestCam(Camera): self._api.go_to_preset( action='start', preset_point_number=preset) except AmcrestError as error: - _LOGGER.error( - 'Could not move %s camera to preset %i due to error: %s', - self.name, preset, error) + log_update_error( + _LOGGER, 'move', self.name, + 'camera to preset {}'.format(preset), error) def _set_color_bw(self, cbw): """Set camera color mode.""" try: self._api.day_night_color = _CBW.index(cbw) except AmcrestError as error: - _LOGGER.error( - 'Could not set %s camera color mode to %s due to error: %s', - self.name, cbw, error) + log_update_error( + _LOGGER, 'set', self.name, + 'camera color mode to {}'.format(cbw), error) else: self._color_bw = cbw self.schedule_update_ha_state() @@ -447,6 +478,6 @@ class AmcrestCam(Camera): try: self._api.tour(start=start) except AmcrestError as error: - _LOGGER.error( - 'Could not %s %s camera tour due to error: %s', - 'start' if start else 'stop', self.name, error) + log_update_error( + _LOGGER, 'start' if start else 'stop', self.name, + 'camera tour', error) diff --git a/homeassistant/components/amcrest/const.py b/homeassistant/components/amcrest/const.py index a0230937e95..fe07659b48a 100644 --- a/homeassistant/components/amcrest/const.py +++ b/homeassistant/components/amcrest/const.py @@ -1,7 +1,11 @@ """Constants for amcrest component.""" DOMAIN = 'amcrest' DATA_AMCREST = DOMAIN +CAMERAS = 'cameras' +DEVICES = 'devices' BINARY_SENSOR_SCAN_INTERVAL_SECS = 5 CAMERA_WEB_SESSION_TIMEOUT = 10 SENSOR_SCAN_INTERVAL_SECS = 10 + +SERVICE_UPDATE = 'update' diff --git a/homeassistant/components/amcrest/helpers.py b/homeassistant/components/amcrest/helpers.py index 270c969a6cc..69d7f5ef288 100644 --- a/homeassistant/components/amcrest/helpers.py +++ b/homeassistant/components/amcrest/helpers.py @@ -2,9 +2,16 @@ from .const import DOMAIN -def service_signal(service, entity_id=None): - """Encode service and entity_id into signal.""" +def service_signal(service, ident=None): + """Encode service and identifier into signal.""" signal = '{}_{}'.format(DOMAIN, service) - if entity_id: - signal += '_{}'.format(entity_id.replace('.', '_')) + if ident: + signal += '_{}'.format(ident.replace('.', '_')) return signal + + +def log_update_error(logger, action, name, entity_type, error): + """Log an update error.""" + logger.error( + 'Could not %s %s %s due to error: %s', + action, name, entity_type, error.__class__.__name__) diff --git a/homeassistant/components/amcrest/manifest.json b/homeassistant/components/amcrest/manifest.json index a2eb8c24e21..f79ce34897b 100644 --- a/homeassistant/components/amcrest/manifest.json +++ b/homeassistant/components/amcrest/manifest.json @@ -3,7 +3,7 @@ "name": "Amcrest", "documentation": "https://www.home-assistant.io/components/amcrest", "requirements": [ - "amcrest==1.4.0" + "amcrest==1.5.3" ], "dependencies": [ "ffmpeg" diff --git a/homeassistant/components/amcrest/sensor.py b/homeassistant/components/amcrest/sensor.py index 718d08358c4..1788b9c62b0 100644 --- a/homeassistant/components/amcrest/sensor.py +++ b/homeassistant/components/amcrest/sensor.py @@ -2,21 +2,28 @@ from datetime import timedelta import logging +from amcrest import AmcrestError + from homeassistant.const import CONF_NAME, CONF_SENSORS +from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity import Entity -from .const import DATA_AMCREST, SENSOR_SCAN_INTERVAL_SECS +from .const import ( + DATA_AMCREST, DEVICES, SENSOR_SCAN_INTERVAL_SECS, SERVICE_UPDATE) +from .helpers import log_update_error, service_signal _LOGGER = logging.getLogger(__name__) SCAN_INTERVAL = timedelta(seconds=SENSOR_SCAN_INTERVAL_SECS) -# Sensor types are defined like: Name, units, icon SENSOR_MOTION_DETECTOR = 'motion_detector' +SENSOR_PTZ_PRESET = 'ptz_preset' +SENSOR_SDCARD = 'sdcard' +# Sensor types are defined like: Name, units, icon SENSORS = { SENSOR_MOTION_DETECTOR: ['Motion Detected', None, 'mdi:run'], - 'sdcard': ['SD Used', '%', 'mdi:sd'], - 'ptz_preset': ['PTZ Preset', None, 'mdi:camera-iris'], + SENSOR_PTZ_PRESET: ['PTZ Preset', None, 'mdi:camera-iris'], + SENSOR_SDCARD: ['SD Used', '%', 'mdi:sd'], } @@ -27,7 +34,7 @@ async def async_setup_platform( return name = discovery_info[CONF_NAME] - device = hass.data[DATA_AMCREST]['devices'][name] + device = hass.data[DATA_AMCREST][DEVICES][name] async_add_entities( [AmcrestSensor(name, device, sensor_type) for sensor_type in discovery_info[CONF_SENSORS]], @@ -40,12 +47,14 @@ class AmcrestSensor(Entity): def __init__(self, name, device, sensor_type): """Initialize a sensor for Amcrest camera.""" self._name = '{} {}'.format(name, SENSORS[sensor_type][0]) + self._signal_name = name self._api = device.api self._sensor_type = sensor_type self._state = None self._attrs = {} self._unit_of_measurement = SENSORS[sensor_type][1] self._icon = SENSORS[sensor_type][2] + self._unsub_dispatcher = None @property def name(self): @@ -72,28 +81,53 @@ class AmcrestSensor(Entity): """Return the units of measurement.""" return self._unit_of_measurement + @property + def available(self): + """Return True if entity is available.""" + return self._api.available + def update(self): """Get the latest data and updates the state.""" - _LOGGER.debug("Pulling data from %s sensor.", self._name) + if not self.available: + return + _LOGGER.debug("Updating %s sensor", self._name) - if self._sensor_type == 'motion_detector': - self._state = self._api.is_motion_detected - self._attrs['Record Mode'] = self._api.record_mode + try: + if self._sensor_type == SENSOR_MOTION_DETECTOR: + self._state = self._api.is_motion_detected + self._attrs['Record Mode'] = self._api.record_mode - elif self._sensor_type == 'ptz_preset': - self._state = self._api.ptz_presets_count + elif self._sensor_type == SENSOR_PTZ_PRESET: + self._state = self._api.ptz_presets_count - elif self._sensor_type == 'sdcard': - storage = self._api.storage_all - try: - self._attrs['Total'] = '{:.2f} {}'.format(*storage['total']) - except ValueError: - self._attrs['Total'] = '{} {}'.format(*storage['total']) - try: - self._attrs['Used'] = '{:.2f} {}'.format(*storage['used']) - except ValueError: - self._attrs['Used'] = '{} {}'.format(*storage['used']) - try: - self._state = '{:.2f}'.format(storage['used_percent']) - except ValueError: - self._state = storage['used_percent'] + elif self._sensor_type == SENSOR_SDCARD: + storage = self._api.storage_all + try: + self._attrs['Total'] = '{:.2f} {}'.format( + *storage['total']) + except ValueError: + self._attrs['Total'] = '{} {}'.format(*storage['total']) + try: + self._attrs['Used'] = '{:.2f} {}'.format(*storage['used']) + except ValueError: + self._attrs['Used'] = '{} {}'.format(*storage['used']) + try: + self._state = '{:.2f}'.format(storage['used_percent']) + except ValueError: + self._state = storage['used_percent'] + except AmcrestError as error: + log_update_error(_LOGGER, 'update', self.name, 'sensor', error) + + async def async_on_demand_update(self): + """Update state.""" + self.async_schedule_update_ha_state(True) + + async def async_added_to_hass(self): + """Subscribe to update signal.""" + self._unsub_dispatcher = async_dispatcher_connect( + self.hass, service_signal(SERVICE_UPDATE, self._signal_name), + self.async_on_demand_update) + + async def async_will_remove_from_hass(self): + """Disconnect from update signal.""" + self._unsub_dispatcher() diff --git a/homeassistant/components/amcrest/switch.py b/homeassistant/components/amcrest/switch.py index 5989d4daf1e..ec286b4f404 100644 --- a/homeassistant/components/amcrest/switch.py +++ b/homeassistant/components/amcrest/switch.py @@ -1,17 +1,23 @@ """Support for toggling Amcrest IP camera settings.""" import logging +from amcrest import AmcrestError + from homeassistant.const import CONF_NAME, CONF_SWITCHES +from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity import ToggleEntity -from .const import DATA_AMCREST +from .const import DATA_AMCREST, DEVICES, SERVICE_UPDATE +from .helpers import log_update_error, service_signal _LOGGER = logging.getLogger(__name__) +MOTION_DETECTION = 'motion_detection' +MOTION_RECORDING = 'motion_recording' # Switch types are defined like: Name, icon SWITCHES = { - 'motion_detection': ['Motion Detection', 'mdi:run-fast'], - 'motion_recording': ['Motion Recording', 'mdi:record-rec'] + MOTION_DETECTION: ['Motion Detection', 'mdi:run-fast'], + MOTION_RECORDING: ['Motion Recording', 'mdi:record-rec'] } @@ -22,7 +28,7 @@ async def async_setup_platform( return name = discovery_info[CONF_NAME] - device = hass.data[DATA_AMCREST]['devices'][name] + device = hass.data[DATA_AMCREST][DEVICES][name] async_add_entities( [AmcrestSwitch(name, device, setting) for setting in discovery_info[CONF_SWITCHES]], @@ -35,10 +41,12 @@ class AmcrestSwitch(ToggleEntity): def __init__(self, name, device, setting): """Initialize the Amcrest switch.""" self._name = '{} {}'.format(name, SWITCHES[setting][0]) + self._signal_name = name self._api = device.api self._setting = setting self._state = False self._icon = SWITCHES[setting][1] + self._unsub_dispatcher = None @property def name(self): @@ -52,30 +60,63 @@ class AmcrestSwitch(ToggleEntity): def turn_on(self, **kwargs): """Turn setting on.""" - if self._setting == 'motion_detection': - self._api.motion_detection = 'true' - elif self._setting == 'motion_recording': - self._api.motion_recording = 'true' + if not self.available: + return + try: + if self._setting == MOTION_DETECTION: + self._api.motion_detection = 'true' + elif self._setting == MOTION_RECORDING: + self._api.motion_recording = 'true' + except AmcrestError as error: + log_update_error(_LOGGER, 'turn on', self.name, 'switch', error) def turn_off(self, **kwargs): """Turn setting off.""" - if self._setting == 'motion_detection': - self._api.motion_detection = 'false' - elif self._setting == 'motion_recording': - self._api.motion_recording = 'false' + if not self.available: + return + try: + if self._setting == MOTION_DETECTION: + self._api.motion_detection = 'false' + elif self._setting == MOTION_RECORDING: + self._api.motion_recording = 'false' + except AmcrestError as error: + log_update_error(_LOGGER, 'turn off', self.name, 'switch', error) + + @property + def available(self): + """Return True if entity is available.""" + return self._api.available def update(self): """Update setting state.""" - _LOGGER.debug("Polling state for setting: %s ", self._name) + if not self.available: + return + _LOGGER.debug("Updating %s switch", self._name) - if self._setting == 'motion_detection': - detection = self._api.is_motion_detector_on() - elif self._setting == 'motion_recording': - detection = self._api.is_record_on_motion_detection() - - self._state = detection + try: + if self._setting == MOTION_DETECTION: + detection = self._api.is_motion_detector_on() + elif self._setting == MOTION_RECORDING: + detection = self._api.is_record_on_motion_detection() + self._state = detection + except AmcrestError as error: + log_update_error(_LOGGER, 'update', self.name, 'switch', error) @property def icon(self): """Return the icon for the switch.""" return self._icon + + async def async_on_demand_update(self): + """Update state.""" + self.async_schedule_update_ha_state(True) + + async def async_added_to_hass(self): + """Subscribe to update signal.""" + self._unsub_dispatcher = async_dispatcher_connect( + self.hass, service_signal(SERVICE_UPDATE, self._signal_name), + self.async_on_demand_update) + + async def async_will_remove_from_hass(self): + """Disconnect from update signal.""" + self._unsub_dispatcher() diff --git a/requirements_all.txt b/requirements_all.txt index 65c92377da7..a93427b23c2 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -178,7 +178,7 @@ alpha_vantage==2.1.0 ambiclimate==0.1.2 # homeassistant.components.amcrest -amcrest==1.4.0 +amcrest==1.5.3 # homeassistant.components.androidtv androidtv==0.0.15