fix downloading event snapshots; clean up events and logs; README

README.md

@@ -44,17 +44,24 @@ It exposes through device discovery a `service` and a `device` with components f
 -   `homeassistant/device/amcrest-service` - service config
 
 -   `homeassistant/device/amcrest-[SERIAL_NUMBER]` per camera, with components:
--    `event`    - all events
+-    `event`        - most all "other" events, not exposed below
--    `camera`   - a snapshot is saved every SNAPSHOT_UPDATE_INTERVAL (also based on how often camera saves snapshot image)
+-    `camera`       - a snapshot is saved every SNAPSHOT_UPDATE_INTERVAL (also based on how often camera saves snapshot image), also an "eventshot" is stored at the time an "event" is triggered in the camera. This is collected by filename, when the Amcrest camera logs a snapshot was saved because of an event (rather than just a routine timed snapshot)
--    `doorbell` - doorbell status (if AD110 or AD410)
+-    `doorbell`     - doorbell status (if AD110 or AD410)
--    `human`    - human detection (if AD410)
+-    `human`        - human detection (if AD410)
--    `motion`   - motion events (if supported)
+-    `motion`       - motion events (if supported)
--    `config`   - device configuration information
+-    `config`       - device configuration information
--    `storage`  - storage stats
+-    `storage`      - storage stats
+-    `privacy_mode` - see (and set) the privacy mode of the camere
 
-## Snapshots, Area Cards, and WebRTC
+## Snapshots/Eventshots plus Home Assistant Area Cards
 
-The `camera` snapshots work really well for the HomeAssistant `Area` cards on a dashboard - just make this MQTT camera device the only camera for an area and place an `Area` card for that location. The WebRTC option works very well with the <a href="https://github.com/AlexxIT/go2rtc">go2rtc</a> package which is a streaming server that works very well for (my) Amcrest cameras. If you setup the WebRTC config here, the `configuration_url` for the MQTT device will be the streaming RTC link instead of just a link to the hostname (which is arguably more correctly a "configuration" url, but I'd rather have a simple link from the device page to a live stream)
+The `camera` snapshots work really well for the HomeAssistant `Area` cards on a dashboard - just make this MQTT camera device the only camera for an area and place an `Area` card for that location.
+
+An "event snapshot" (`eventshot`) is separately (and specifically, by filename) collected when the camera automatically records a snapshot because of an event. Note, that if the Amcrest camera is configured to record 3 or 5 snapshots on an event - each of those will be updated by `amcrest2mqtt` and you will very quickly end up with (only) the last snapshot stored. This might alter you decision on how to configure your camera for this setting. (Or perhaps I can turn the snapshots-for-an-event into an animated image on the HA-side, thought that seems like overkill.)
+
+## WebRTC
+
+The WebRTC option works very well with the <a href="https://github.com/AlexxIT/go2rtc">go2rtc</a> package which is a streaming server that works very well for (my) Amcrest cameras. If you setup the WebRTC config here, the `configuration_url` for the MQTT device will be the streaming RTC link instead of just a link to the hostname (which is arguably a more correct "configuration" url, but I'd rather have a simple link from the device page to jump to a live stream).
 
 ## Device Support
 

VERSION

@@ -1 +1 @@
-0.99.22
+0.99.23

amcrest_api.py

@@ -119,10 +119,9 @@ class AmcrestAPI(object):
         try:
             storage = self.devices[device_id]["camera"].storage_all
         except CommError as err:
-            self.logger.error(f'Failed to communicate with device ({device_id}): No SD card?')
+            self.logger.error(f'Failed to communicate with device ({device_id}) for storage stats')
 
         return {
-            'last_update': str(datetime.now(ZoneInfo(self.timezone))),
             'used_percent': str(storage['used_percent']),
             'used': to_gb(storage['used']),
             'total': to_gb(storage['total']),
@@ -140,7 +139,7 @@ class AmcrestAPI(object):
 
             return privacy_mode
         except CommError as err:
-            self.logger.error(f'Failed to communicate with device ({device_id}): {err}')
+            self.logger.error(f'Failed to communicate with device ({device_id}) for privacy mode')
 
     def set_privacy_mode(self, device_id, switch):
         device = self.devices[device_id]
@@ -167,7 +166,7 @@ class AmcrestAPI(object):
             else:
                 self.logger.info(f'Skipped snapshot from ({device_id}) because "privacy mode" is ON')
         except CommError as err:
-            self.logger.error(f'Failed to communicate with device ({device_id}), maybe "Privacy Mode" is on? {err}')
+            self.logger.error(f'Failed to communicate with device ({device_id})')
 
     def get_snapshot(self, device_id):
         return self.devices[device_id]['snapshot'] if 'snapshot' in self.devices[device_id] else None
@@ -176,17 +175,21 @@ class AmcrestAPI(object):
     def get_recorded_file(self, device_id, file):
         device = self.devices[device_id]
 
-        data_raw = device["camera"].download_file(file)
+        try:
-        if data_raw:
+            data_raw = device["camera"].download_file(file)
-            data_base64 = base64.b64encode(data_raw)
+            if data_raw:
-            self.logger.debug(f'Processed recording from ({device_id}) {len(data_raw)} bytes raw, and {len(data_base64)} bytes base64')
+                data_base64 = base64.b64encode(data_raw)
-            if data_base64 < 100 * 1024 * 1024 * 1024:
+                self.logger.info(f'Processed recording from ({device_id}) {len(data_raw)} bytes raw, and {len(data_base64)} bytes base64')
-                return data_base64
+                if len(data_base64) < 100 * 1024 * 1024 * 1024:
-            else:
+                    return data_base64
-                self.logger.error(f'Processed recording is too large')
+                else:
-                return None
+                    self.logger.error(f'Processed recording is too large')
-
+                    return None
-        return None
+
+            return None
+        except CommError as err:
+            self.logger.error(f'Failed to download recorded file for device ({device_id}): {err}')
+            return None
 
     # Events --------------------------------------------------------------------------------------
 
@@ -204,7 +207,7 @@ class AmcrestAPI(object):
             async for code, payload in device["camera"].async_event_actions("All"):
                 await self.process_device_event(device_id, code, payload)
         except CommError as err:
-            self.logger.error(f'Failed to communicate with device ({device_id}): {err}')
+            self.logger.error(f'Failed to cmmunicate with device ({device_id})')
         except Exception as err:
             self.logger.error(f'generic Failed to get events from device({device_id}: {err}', exc_info=True)
 
@@ -213,7 +216,7 @@ class AmcrestAPI(object):
             device = self.devices[device_id]
             config = device['config']
 
-            # self.logger.debug(f'Event on {device_id} - {code}: {payload}')
+            # self.logger.info(f'Event on {device_id} - {code}: {payload}')
 
             # VideoMotion: motion detection event
             # VideoLoss: video loss detection event
@@ -237,9 +240,9 @@ class AmcrestAPI(object):
                 doorbell_payload = 'on' if payload['data']['Action'] == 'Invite' else 'off'
                 self.events.append({ 'device_id': device_id, 'event': 'doorbell', 'payload': doorbell_payload })
             elif code == 'NewFile':
-                # we don't care about recording events for general (non-saved) snapshots
+                if ('File' in payload['data'] and '[R]' not in payload['data']['File']
-                if not payload['data']['StoragePoint'] == 'NULL':
+                and ('StoragePoint' not in payload['data'] or payload['data']['StoragePoint'] != 'Temporary')):
-                    file_payload = { 'file': payload['data']['File'], 'size': payload['data']['Size'], 'event': payload['data']['Event'] }
+                    file_payload = { 'file': payload['data']['File'], 'size': payload['data']['Size'] }
                     self.events.append({ 'device_id': device_id, 'event': 'recording', 'payload': file_payload })
             elif code == 'LensMaskOpen':
                 device['privacy_mode'] = True
@@ -247,6 +250,13 @@ class AmcrestAPI(object):
             elif code == 'LensMaskClose':
                 device['privacy_mode'] = False
                 self.events.append({ 'device_id': device_id, 'event': 'privacy_mode', 'payload': 'off' })
+            # lets send these but not bother logging them here
+            elif code == 'TimeChange':
+                self.events.append({ 'device_id': device_id, 'event': code , 'payload': payload['action'] })
+            elif code == 'NTPAdjustTime':
+                self.events.append({ 'device_id': device_id, 'event': code , 'payload': payload['action'] })
+            elif code == 'RtspSessionDisconnect':
+                self.events.append({ 'device_id': device_id, 'event': code , 'payload': payload['action'] })
             # lets just ignore these
             elif code == 'InterVideoAccess': # I think this is US, accessing the API of the camera, lets not inception!
                 pass
@@ -254,6 +264,7 @@ class AmcrestAPI(object):
                 pass
             # save everything else as a 'generic' event
             else:
+                self.logger.info(f'Event on {device_id} - {code}: {payload}')
                 self.events.append({ 'device_id': device_id, 'event': code , 'payload': payload })
 
         except Exception as err:

amcrest_mqtt.py

@@ -384,7 +384,7 @@ class AmcrestMqtt(object):
                 # setup initial satte
                 self.states[device_id]['state'] = {
                     'state': 'ON',
-                    'last_update': None,
+                    'last_update': str(datetime.now(ZoneInfo(self.timezone))),
                     'host': config['host'],
                     'serial_number': config['serial_number'],
                     'sw_version': config['software_version'],
@@ -574,6 +574,7 @@ class AmcrestMqtt(object):
 
     def publish_device_state(self, device_id):
         device_states = self.states[device_id]
+        device_states['state']['last_update'] = str(datetime.now(ZoneInfo(self.timezone)))
 
         for topic in ['state','storage','motion','human','doorbell','event','recording','privacy_mode']:
             if topic in device_states:
@@ -606,10 +607,8 @@ class AmcrestMqtt(object):
             if privacy_mode is not None:
                 device_states['privacy_mode'] = 'on' if privacy_mode == True else 'off'
 
-            # get the storage info, pull out last_update and save that to the device state
             storage = self.amcrestc.get_storage_stats(device_id)
             if storage is not None:
-                device_states['state']['last_update'] = storage.pop('last_update', None)
                 device_states['storage'] = storage
 
                 self.publish_service_state()
@@ -640,13 +639,15 @@ class AmcrestMqtt(object):
     def get_recorded_file(self, device_id, file, type):
         device_states = self.states[device_id]
 
-        self.logger.info(f'Getting recorded file {file}')
+        self.logger.info(f'Getting recorded file "{file}"')
         image = self.amcrestc.get_recorded_file(device_id, file)
-        self.logger.info(f'Got back base64 image of {len(image)} bytes')
 
         if image is None:
+            self.logger.info(f'Failed to get recorded file from {self.get_device_name(device_id)}')
             return
 
+        self.logger.info(f'Got back base64 image of {len(image)} bytes')
+
         # only store and send to MQTT if the image has changed
         if device_states['camera'][type] is None or device_states['camera'][type] != image:
             device_states['camera'][type] = image
@@ -697,41 +698,40 @@ class AmcrestMqtt(object):
         await self.amcrestc.collect_all_device_events()
 
     def check_for_events(self):
-        while device_event := self.amcrestc.get_next_event():
+        try:
-            if device_event is None:
+            while device_event := self.amcrestc.get_next_event():
-                break
+                if device_event is None:
-            if 'device_id' not in device_event:
+                    break
-                self.logger(f'Got event, but missing device_id: {device_event}')
+                if 'device_id' not in device_event:
-                continue
+                    self.logger(f'Got event, but missing device_id: {device_event}')
-
+                    continue
-            device_id = device_event['device_id']
+
-            event = device_event['event']
+                device_id = device_event['device_id']
-            payload = device_event['payload']
+                event = device_event['event']
-
+                payload = device_event['payload']
-            device_states = self.states[device_id]
+
-
+                device_states = self.states[device_id]
-            # if one of our known sensors
+
-            if event in ['motion','human','doorbell','recording','privacy_mode']:
+                # if one of our known sensors
-                if event == 'recording' and payload['file'].endswith('.jpg'):
+                if event in ['motion','human','doorbell','recording','privacy_mode']:
-                    self.logger.info(f'{event} - {payload}')
+                    if event == 'recording' and payload['file'].endswith('.jpg'):
-                    self.get_recorded_file(device_id, payload['file'], 'eventshot')
+                        self.get_recorded_file(device_id, payload['file'], 'eventshot')
+                    else:
+                        # only log details if not a recording
+                        if event != 'recording':
+                            self.logger.info(f'Got event for {self.get_device_name(device_id)}: {event} - {payload}')
+                        device_states[event] = payload
+
+                    # other ways to infer "privacy mode" is off and needs updating
+                    if event in ['motion','human','doorbell'] and device_states['privacy_mode'] == 'on':
+                        device_states['privacy_mode'] = 'off'
                 else:
-                    # only log details if not a recording
+                    self.logger.info(f'Got "other" event for "{self.get_device_name(device_id)}": {event} - {payload}')
-                    if event != 'recording':
+                    device_states['event'] = f'{event} - {payload}'
-                        self.logger.info(f'Got event for {self.get_device_name(device_id)}: {event} - {payload}')
-                    device_states[event] = payload
-
-                # other ways to infer "privacy mode" is off and needs updating
-                if event in ['motion','human','doorbell'] and device_states['privacy_mode'] == 'on':
-                    device_states['privacy_mode'] = 'off'
-            # these, we don't are to log
-            elif event in ['TimeChange','NTPAdjustTime','RtspSessionDisconnect']:
-                pass
-            else:
-                self.logger.info(f'Got "other" event for "{self.get_device_name(device_id)}": {event} - {payload}')
-                device_states['event'] = event
 
-            self.publish_device_state(device_id)
+                self.publish_device_state(device_id)
+        except Exception as err:
+            self.logger.error(err, exc_info=True)
 
     # async loops and main loop -------------------------------------------------------------------
 
@@ -787,4 +787,4 @@ class AmcrestMqtt(object):
             exit(1)
         except Exception as err:
             self.running = False
-            self.log.error(f'Caught exception: {err}')
+            self.logger.error(f'Caught exception: {err}')