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

@@ -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' "
+            "the '%s' option with value '%s' instead",
-            "instead.", SENSOR_MOTION_DETECTOR)
+            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.",
+            "services and attributes instead",
-            config[CONF_SWITCHES])
+            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': []})
+    hass.data.setdefault(DATA_AMCREST, {DEVICES: {}, CAMERAS: []})
-    devices = config[DOMAIN]
 
-    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],
+            api = AmcrestChecker(
-                                device[CONF_PORT],
+                hass, name,
-                                username,
+                device[CONF_HOST], device[CONF_PORT],
-                                password).camera
+                username, password)
-            # pylint: disable=pointless-statement
-            # Test camera communications.
-            api.current_time
 
-        except AmcrestError as ex:
+        except LoginError as ex:
-            _LOGGER.error("Unable to connect to %s camera: %s", name, str(ex))
+            _LOGGER.error("Login error for %s camera: %s", name, ex)
-            hass.components.persistent_notification.create(
-                'Error: {}<br />'
-                'You will need to restart hass after fixing.'
-                ''.format(ex),
-                title=NOTIFICATION_TITLE,
-                notification_id=NOTIFICATION_ID)
             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):

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(
+            log_update_error(
-                'Could not update %s binary sensor due to error: %s',
+                _LOGGER, 'update', self.name, 'binary sensor', error)
-                self.name, 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()

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 .const import (
-from .helpers import service_signal
+    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:
+        available = self.available
-            _LOGGER.error(
+        if not available or not self.is_on:
-                'Attempt to take snaphot when %s camera is off', self.name)
+            _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:
+            except (AmcrestError, HTTPError) as error:
-                _LOGGER.error(
+                log_update_error(
-                    'Could not get image from %s camera due to error: %s',
+                    _LOGGER, 'get image from', self.name, 'camera', error)
-                    self.name, 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 not self.available or self._update_succeeded:
-        if self._brand is None:
+            if not self.available:
-            try:
+                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:
+            if self._model is None:
-                _LOGGER.error(
+                resp = self._api.device_type.strip()
-                    'Could not get %s camera brand due to error: %s',
+                if resp.startswith('type='):
-                    self.name, error)
+                    self._model = resp.split('=')[-1]
-                self._brand = 'unknwown'
+                else:
-        if self._model is None:
+                    self._model = 'unknown'
-            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:
             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(
+            log_update_error(
-                'Could not get %s camera attributes due to error: %s',
+                _LOGGER, 'get', self.name, 'camera attributes', error)
-                self.name, 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(
+            log_update_error(
-                'Could not %s %s camera video stream due to error: %s',
+                _LOGGER, 'enable' if enable else 'disable', self.name,
-                'enable' if enable else 'disable', self.name, error)
+                '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(
+            log_update_error(
-                'Could not %s %s camera recording due to error: %s',
+                _LOGGER, 'enable' if enable else 'disable', self.name,
-                'enable' if enable else 'disable', self.name, error)
+                '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(
+            log_update_error(
-                'Could not %s %s camera motion detection due to error: %s',
+                _LOGGER, 'enable' if enable else 'disable', self.name,
-                'enable' if enable else 'disable', self.name, error)
+                '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(
+            log_update_error(
-                'Could not %s %s camera audio stream due to error: %s',
+                _LOGGER, 'enable' if enable else 'disable', self.name,
-                'enable' if enable else 'disable', self.name, error)
+                '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(
+            log_update_error(
-                'Could not %s %s camera indicator light due to error: %s',
+                _LOGGER, 'enable' if enable else 'disable', self.name,
-                'enable' if enable else 'disable', self.name, error)
+                '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(
+            log_update_error(
-                'Could not %s %s camera motion recording due to error: %s',
+                _LOGGER, 'enable' if enable else 'disable', self.name,
-                'enable' if enable else 'disable', self.name, error)
+                '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(
+            log_update_error(
-                'Could not move %s camera to preset %i due to error: %s',
+                _LOGGER, 'move', self.name,
-                self.name, preset, error)
+                '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(
+            log_update_error(
-                'Could not set %s camera color mode to %s due to error: %s',
+                _LOGGER, 'set', self.name,
-                self.name, cbw, error)
+                '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(
+            log_update_error(
-                'Could not %s %s camera tour due to error: %s',
+                _LOGGER, 'start' if start else 'stop', self.name,
-                'start' if start else 'stop', self.name, error)
+                'camera tour', error)

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'

homeassistant/components/amcrest/helpers.py

@@ -2,9 +2,16 @@
 from .const import DOMAIN
 
 
-def service_signal(service, entity_id=None):
+def service_signal(service, ident=None):
-    """Encode service and entity_id into signal."""
+    """Encode service and identifier into signal."""
     signal = '{}_{}'.format(DOMAIN, service)
-    if entity_id:
+    if ident:
-        signal += '_{}'.format(entity_id.replace('.', '_'))
+        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__)

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"

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'],
+    SENSOR_PTZ_PRESET: ['PTZ Preset', None, 'mdi:camera-iris'],
-    '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':
+        try:
-            self._state = self._api.is_motion_detected
+            if self._sensor_type == SENSOR_MOTION_DETECTOR:
-            self._attrs['Record Mode'] = self._api.record_mode
+                self._state = self._api.is_motion_detected
+                self._attrs['Record Mode'] = self._api.record_mode
 
-        elif self._sensor_type == 'ptz_preset':
+            elif self._sensor_type == SENSOR_PTZ_PRESET:
-            self._state = self._api.ptz_presets_count
+                self._state = self._api.ptz_presets_count
 
-        elif self._sensor_type == 'sdcard':
+            elif self._sensor_type == SENSOR_SDCARD:
-            storage = self._api.storage_all
+                storage = self._api.storage_all
-            try:
+                try:
-                self._attrs['Total'] = '{:.2f} {}'.format(*storage['total'])
+                    self._attrs['Total'] = '{:.2f} {}'.format(
-            except ValueError:
+                        *storage['total'])
-                self._attrs['Total'] = '{} {}'.format(*storage['total'])
+                except ValueError:
-            try:
+                    self._attrs['Total'] = '{} {}'.format(*storage['total'])
-                self._attrs['Used'] = '{:.2f} {}'.format(*storage['used'])
+                try:
-            except ValueError:
+                    self._attrs['Used'] = '{:.2f} {}'.format(*storage['used'])
-                self._attrs['Used'] = '{} {}'.format(*storage['used'])
+                except ValueError:
-            try:
+                    self._attrs['Used'] = '{} {}'.format(*storage['used'])
-                self._state = '{:.2f}'.format(storage['used_percent'])
+                try:
-            except ValueError:
+                    self._state = '{:.2f}'.format(storage['used_percent'])
-                self._state = 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()

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_DETECTION: ['Motion Detection', 'mdi:run-fast'],
-    'motion_recording': ['Motion Recording', 'mdi:record-rec']
+    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':
+        if not self.available:
-            self._api.motion_detection = 'true'
+            return
-        elif self._setting == 'motion_recording':
+        try:
-            self._api.motion_recording = 'true'
+            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':
+        if not self.available:
-            self._api.motion_detection = 'false'
+            return
-        elif self._setting == 'motion_recording':
+        try:
-            self._api.motion_recording = 'false'
+            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':
+        try:
-            detection = self._api.is_motion_detector_on()
+            if self._setting == MOTION_DETECTION:
-        elif self._setting == 'motion_recording':
+                detection = self._api.is_motion_detector_on()
-            detection = self._api.is_record_on_motion_detection()
+            elif self._setting == MOTION_RECORDING:
-
+                detection = self._api.is_record_on_motion_detection()
-        self._state = 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()

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