Refactored Arlo component and enhanced Arlo API queries and times by tchellomello · Pull Request #14823 · home-assistant/core

tchellomello · 2018-06-05T04:42:38Z

Description:

Arlo does not offer an official API so all this work is based on reverse engineering.
Refactored Arlo component and enhanced Arlo API queries and times.
This PR also added a service call method to force the entities to be updated.

Related issue (if applicable): fixes #13176

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#5508

Example entry for `configuration.yaml` (if applicable):

arlo:
  username: user@email.domain
  password: secret
  scan_interval: 120

ffmpeg:

camera: 
  - platform: arlo

alarm_control_panel:
  - platform: arlo

sensor:
  - platform: arlo

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in home-assistant.github.io

If the code communicates with devices, web services, or third-party tools:

New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
New dependencies are only imported inside functions that use them ([example][ex-import]).
New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

If the code does not interact with devices:

Tests have been added to verify that the new code works.

MartinHjelmare · 2018-06-05T07:16:17Z

+
+    def __init__(self, hub):
+        """Initialize the entity."""
+        self.hub = hub


Why store the hub on another instance if it's just that attribute and no methods needed?

MartinHjelmare · 2018-06-05T07:19:45Z

+        async_dispatcher_connect(
+            self.hass, SIGNAL_UPDATE_ARLO, self._update_callback)
+
+    def _update_callback(self):


Decorate this with @callback imported from core.py.

MartinHjelmare · 2018-06-05T07:20:38Z

        """Return icon."""
        return ICON

+    @asyncio.coroutine


We're now using Python 3.5 async syntax, ie async def.

MartinHjelmare · 2018-06-05T07:21:17Z

+
+    def _update_callback(self):
+        """Call update method."""
+        self.schedule_update_ha_state(True)


Use self.async_schedule_update_ha_state.

MartinHjelmare · 2018-06-05T07:21:37Z

-                return
-            i += 1
-        self._state = None
+        _LOGGER.info("Updating Arlo Alarm Control Panel %s", self.name)


MartinHjelmare · 2018-06-05T07:28:35Z

+        # assign refresh period to base station thread
+        try:
+            if arlo.base_stations:
+                arlo_base_station = arlo.base_stations[0]


This is better:

arlo_base_station = next(( station for station in arlo.base_stations), None) if arlo_base_station is None: return False

MartinHjelmare · 2018-06-05T07:39:08Z

+            if arlo.base_stations:
+                arlo_base_station = arlo.base_stations[0]
+                arlo_base_station.refresh_rate = scan_interval.total_seconds()
+        except (AttributeError, IndexError):


Why would there be an attribute error?

The index error is solved by using my suggestion above.

Yes, that is not needed anymore. Good refactoring!! Thanks

MartinHjelmare · 2018-06-05T07:41:28Z

        """Set the mode in the base station."""
        # Get the list of base stations identified by library
-        base_stations = self.hass.data[DATA_ARLO].base_stations
+        base_stations = self.hass.data.get(DATA_ARLO).hub.base_stations


Don't change to dict.get. We know that the key will be in the dict.

MartinHjelmare · 2018-06-05T07:43:10Z

    def should_poll(self):
        """Camera should poll periodically."""
-        return True
+        return False


This is the default for cameras.

tchellomello · 2018-06-06T14:18:26Z

Thanks for the review @MartinHjelmare Good tips indeed!! Please let me know what you think now. I hope to update the documentation tonight so then we can get this merged.

MartinHjelmare · 2018-06-06T14:28:40Z

    def _update_callback(self):
        """Call update method."""
-        self.schedule_update_ha_state(True)
+        self.async_schedule_update_ha_state()


Pass True as argument to call the update method before updating state.

self.async_schedule_update_ha_state(True)

MartinHjelmare · 2018-06-06T14:30:32Z

    """Set up an Arlo IP Camera."""
-    arlo = hass.data.get(DATA_ARLO).hub
+    arlo = hass.data[DATA_ARLO]
    if not arlo:


This can't happen.

MartinHjelmare · 2018-06-06T14:30:45Z

    """Set up an Arlo IP sensor."""
-    arlo = hass.data.get(DATA_ARLO).hub
+    arlo = hass.data.get(DATA_ARLO)
    if not arlo:


Same as above.

houndci-bot · 2018-06-06T14:51:26Z

+        """Call ArloHub to refresh information."""
+        _LOGGER.info("Updating Arlo Hub component")
+        hass.data[DATA_ARLO].update(update_cameras=True,
+                                        update_base_station=True)


continuation line over-indented for visual indent

MartinHjelmare · 2018-06-08T06:01:33Z

+    @callback
+    def _update_callback(self):
+        """Call update method."""
+        self.async_schedule_update_ha_state()


Add True as argument.

MartinHjelmare · 2018-06-08T06:03:26Z

-        return self._camera.last_image
+        return self._camera.last_image_from_cache
+
+    @asyncio.coroutine


Update all coroutines to use new syntax.

MartinHjelmare · 2018-06-08T06:04:59Z

+        async_dispatcher_connect(
+            self.hass, SIGNAL_UPDATE_ARLO, self._update_callback)
+
+    def _update_callback(self):


Update this like my previous comment. I haven't commented on all the places that needs the same changes.

Yes, my bad.

MartinHjelmare · 2018-06-08T06:05:14Z

+
+    def _update_callback(self):
+        """Call update method."""
+        self.schedule_update_ha_state(True)


tchellomello · 2018-06-08T14:48:15Z

@MartinHjelmare thanks again for the code review. I've updated the documentation and code. 🏆

arsaboo · 2018-06-08T18:05:05Z

Seeing a lot of errors (started appearing after about 14 minutes of restarting) with the latest code https://hastebin.com/evogidegih.sql

tchellomello · 2018-06-08T18:07:09Z

@arsaboo I got the same here. Thanks for the heads up. I'm working on this now

tchellomello · 2018-06-11T04:03:32Z

@arsaboo it seems we got it now. Please let me know if works for you 👍

arsaboo · 2018-06-11T15:46:20Z

Looks like everything (except the alarm_control_panel) is fixed. The camera and sensors are flawless and each entity is not making its own separate API call. Also, there are no errors/warnings in the logs....so that is already a major improvement and will make lot of people very happy.

For some reason, alarm_control_panel goes to Unknown and stays there. Not sure what is causing this.

tchellomello · 2018-06-11T15:54:28Z

@arsaboo thanks for your update. I'm glad this worked and fixed the huge amount of calls made to the Arlo API.
In regards the alarm_control_panel I'm working on a different PR that will include a big refactor in some areas of the pyarlo code. The alarm_control_panel starts correctly but after some time, it goes to unknown after some time and I think I know why. We should get a new PR for that soon.

@MartinHjelmare does that sound a good plan? Could we get this merged so then our users can get to use Arlo again while I'm working on the alarm_control_panel isolated issue?

Thanks to everyone who helped on this! 👍

arsaboo · 2018-06-11T15:56:40Z

I will let @MartinHjelmare decide, but given that Arlo is currently broken, it will be great if we can get this merged soon.

Thanks @tchellomello for all your efforts.

MartinHjelmare · 2018-06-11T19:36:34Z

Sounds good!

MartinHjelmare

Some small fixes left. You can also remove the second argument True in the camera platform call to add_devices.

MartinHjelmare · 2018-06-11T19:47:22Z

-    arlo = hass.data.get(DATA_ARLO)
+    arlo = hass.data[DATA_ARLO]
    if not arlo:
        return False


Only return.

MartinHjelmare · 2018-06-11T19:48:32Z

+    @callback
+    def _update_callback(self):
+        """Call update method."""
+        self.async_schedule_update_ha_state(True)


There's no update method for this entity anymore so we don't need the argument True here. Sorry if I said so earlier.

MartinHjelmare · 2018-06-11T19:49:27Z


-    async_add_devices(sensors, True)
+    add_devices(sensors, True)
+    return True


Remove this.

MartinHjelmare · 2018-06-11T19:53:48Z

    """Set up an Arlo IP Camera."""
-    arlo = hass.data.get(DATA_ARLO)
+    arlo = hass.data[DATA_ARLO]
    if not arlo:


I don't think this can happen?

tchellomello · 2018-06-12T02:35:02Z

Thanks @MartinHjelmare PR updated. 👍

MartinHjelmare

Great!

greybags · 2018-08-11T09:53:38Z

Hi Guys, fantastic work on this, I love that it has my Arlo cameras integrated and I can use automations to control them using presence detection but has the alarm_control_panel issue been addressed? I still see unknown after a while and it then stays there?

Cheers

tchellomello added 10 commits June 4, 2018 23:44

start arlo refactoring

d6ac61a

Refactored Arlo Hub to avoid uncessary and duplicated GETs to Arlo API

ec1442f

Refactored Arlo camera component to avoid duplicate queries

3f3387a

Added debug and error messages when video is not found

e0a252f

Transformed Arlo Control Panel to Sync

0cc5079

Makes linter happy

e437a0c

Uses total_seconds() for scan_interval

1cbd2f8

Added callback and fixed scan_interval issue

63ae201

Disable multiple tries and supported custom modes set in Arlo

66a791d

Bump PyArlo version to 0.1.4

cae9616

homeassistant added integration: arlo platform: alarm_control_panel.arlo cla-signed labels Jun 5, 2018

tchellomello mentioned this pull request Jun 5, 2018

Miscellaneous Arlo warnings and errors #13176

Closed

tchellomello changed the title Refactored Arlo component and enhanced Arlo API queries and times **WIP** Refactored Arlo component and enhanced Arlo API queries and times WIP Jun 5, 2018

Makes lint happy

da74c93

homeassistant added the cla-signed label Jun 5, 2018

MartinHjelmare reviewed Jun 5, 2018

View reviewed changes

Removed ArloHub object and added some tweaks

377a9b7

MartinHjelmare reviewed Jun 6, 2018

View reviewed changes

Fixed hub_refresh method

b0c3d03

homeassistant added the cla-signed label Jun 6, 2018

houndci-bot reviewed Jun 6, 2018

View reviewed changes

Makes lint happy

a83658c

MartinHjelmare reviewed Jun 8, 2018

View reviewed changes

tchellomello mentioned this pull request Jun 8, 2018

Updated Arlo documentation home-assistant/home-assistant.io#5508

Merged

2 tasks

tchellomello changed the title ~~Refactored Arlo component and enhanced Arlo API queries and times WIP~~ Refactored Arlo component and enhanced Arlo API queries and times Jun 8, 2018

Ajusted async syntax and added callbacks decorators

6a0a9c8

homeassistant added the cla-signed label Jun 8, 2018

ghost added the in progress label Jun 8, 2018

Bump PyArlo version to 0.1.6 to include some enhacements

db2d2ee

homeassistant added the cla-signed label Jun 11, 2018

MartinHjelmare reviewed Jun 11, 2018

View reviewed changes

Refined code

1f2f03b

homeassistant added the cla-signed label Jun 12, 2018

MartinHjelmare approved these changes Jun 12, 2018

View reviewed changes

MartinHjelmare merged commit cdc5388 into home-assistant:dev Jun 12, 2018

ghost removed the in progress label Jun 12, 2018

tchellomello deleted the arlo_fix branch June 12, 2018 13:37

This was referenced Jun 22, 2018

0.72 #15088

Merged

Arlo doing I/O in event loop #15159

Closed

home-assistant locked as resolved and limited conversation to collaborators Aug 11, 2018

ghost removed platform: alarm_control_panel.arlo labels Mar 21, 2019

Uh oh!

Conversation

tchellomello commented Jun 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MartinHjelmare Jun 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tchellomello commented Jun 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tchellomello commented Jun 8, 2018

Uh oh!

arsaboo commented Jun 8, 2018

Uh oh!

tchellomello commented Jun 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tchellomello commented Jun 11, 2018

Uh oh!

arsaboo commented Jun 11, 2018

Uh oh!

tchellomello commented Jun 11, 2018

Uh oh!

arsaboo commented Jun 11, 2018

Uh oh!

MartinHjelmare commented Jun 11, 2018

Uh oh!

MartinHjelmare left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tchellomello commented Jun 12, 2018

Uh oh!

tchellomello commented Jun 5, 2018 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):

MartinHjelmare Jun 5, 2018 •

edited

Loading

tchellomello commented Jun 8, 2018 •

edited

Loading