Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 63 additions & 8 deletions homeassistant/components/sensor/arlo.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@
from homeassistant.components.arlo import (
CONF_ATTRIBUTION, DEFAULT_BRAND, DATA_ARLO, SIGNAL_UPDATE_ARLO)
from homeassistant.components.sensor import PLATFORM_SCHEMA
from homeassistant.const import (ATTR_ATTRIBUTION, CONF_MONITORED_CONDITIONS)
from homeassistant.const import (
ATTR_ATTRIBUTION, CONF_MONITORED_CONDITIONS, TEMP_CELSIUS,
DEVICE_CLASS_TEMPERATURE, DEVICE_CLASS_HUMIDITY)

from homeassistant.helpers.dispatcher import async_dispatcher_connect
from homeassistant.helpers.entity import Entity
from homeassistant.helpers.icon import icon_for_battery_level
Expand All @@ -28,7 +31,10 @@
'total_cameras': ['Arlo Cameras', None, 'video'],
'captured_today': ['Captured Today', None, 'file-video'],
'battery_level': ['Battery Level', '%', 'battery-50'],
'signal_strength': ['Signal Strength', None, 'signal']
'signal_strength': ['Signal Strength', None, 'signal'],
'temperature': ['Temperature', TEMP_CELSIUS, 'thermometer'],
'humidity': ['Humidity', '%', 'water-percent'],
'air_quality': ['Air Quality', 'ppm', 'biohazard']
}

PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
Expand All @@ -50,10 +56,24 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
SENSOR_TYPES[sensor_type][0], arlo, sensor_type))
else:
for camera in arlo.cameras:
if sensor_type == 'temperature' or \
sensor_type == 'humidity' or \
sensor_type == 'air_quality':
continue

name = '{0} {1}'.format(
SENSOR_TYPES[sensor_type][0], camera.name)
sensors.append(ArloSensor(name, camera, sensor_type))

for base_station in arlo.base_stations:
if ((sensor_type == 'temperature' or
sensor_type == 'humidity' or
sensor_type == 'air_quality') and
base_station.model_id == 'ABC1000'):
name = '{0} {1}'.format(
SENSOR_TYPES[sensor_type][0], base_station.name)
sensors.append(ArloSensor(name, base_station, sensor_type))

add_devices(sensors, True)


Expand All @@ -62,6 +82,7 @@ class ArloSensor(Entity):

def __init__(self, name, device, sensor_type):
"""Initialize an Arlo sensor."""
_LOGGER.debug('ArloSensor created for %s', name)
self._name = name
self._data = device
self._sensor_type = sensor_type
Expand All @@ -81,6 +102,7 @@ async def async_added_to_hass(self):
@callback
def _update_callback(self):
"""Call update method."""
_LOGGER.debug('update')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this. The state update will be logged by the core at info level.

self.async_schedule_update_ha_state(True)

@property
Expand All @@ -101,6 +123,15 @@ def unit_of_measurement(self):
"""Return the units of measurement."""
return SENSOR_TYPES.get(self._sensor_type)[1]

@property
def device_class(self):
"""Return the device class of the sensor."""
if self._sensor_type == 'temperature':
return DEVICE_CLASS_TEMPERATURE
elif self._sensor_type == 'humidity':
return DEVICE_CLASS_HUMIDITY
return None

def update(self):
"""Get the latest data and updates the state."""
_LOGGER.debug("Updating Arlo sensor %s", self.name)
Expand All @@ -124,13 +155,31 @@ def update(self):
elif self._sensor_type == 'battery_level':
try:
self._state = self._data.battery_level
except TypeError:
except (AttributeError, TypeError):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would there be an AttributeError?

@tchellomello tchellomello Jun 24, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I don't think the attribute error should be necessary since all cameras have the battery_level

self._state = None

elif self._sensor_type == 'signal_strength':
try:
self._state = self._data.signal_strength
except TypeError:
except (AttributeError, TypeError):
self._state = None

elif self._sensor_type == 'temperature':
try:
self._state = self._data.ambient_temperature
except (AttributeError, TypeError):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would there be these errors? The library shouldn't let a type error bubble up from a simple attribute access. All attributes should be initialized after init so attribute error shouldn't be possible either. If it is, the library should be updated to handle that better.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had these in there because the library was in some cases bubbling up an AttributeError but in further testing, it appears to be the result of an issue that was fixed in pyarlo==0.1.9 where the method to get the ambient sensor data was not being called on update. I've removed them and will push.

self._state = None

elif self._sensor_type == 'humidity':
try:
self._state = self._data.ambient_humidity
except (AttributeError, TypeError):
self._state = None

elif self._sensor_type == 'air_quality':
try:
self._state = self._data.ambient_air_quality
except (AttributeError, TypeError):
self._state = None

@property
Expand All @@ -141,10 +190,16 @@ def device_state_attributes(self):
attrs[ATTR_ATTRIBUTION] = CONF_ATTRIBUTION
attrs['brand'] = DEFAULT_BRAND

if self._sensor_type == 'last_capture' or \
self._sensor_type == 'captured_today' or \
self._sensor_type == 'battery_level' or \
self._sensor_type == 'signal_strength':
known_sensor_types = [

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be moved to a constant at module level.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a lazy workaround for address a maximum conditions in an if statement on my part. There's a dictionary with keys of known types already at the module level, I'll use that.

'last_capture',
'captured_today',
'battery_level',
'signal_strength',
'temperature',
'humidity',
'air_quality']

if self._sensor_type in known_sensor_types:
attrs['model'] = self._data.model_id

return attrs
158 changes: 158 additions & 0 deletions tests/components/sensor/test_arlo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
"""The tests for the Netgear Arlo sensors."""
import asyncio
import unittest
from collections import namedtuple
from unittest import mock
from unittest.mock import patch, MagicMock
from homeassistant.const import (
DEVICE_CLASS_TEMPERATURE, DEVICE_CLASS_HUMIDITY, ATTR_ATTRIBUTION)
from homeassistant.components.sensor import arlo
from homeassistant.components.arlo import DATA_ARLO
from homeassistant.helpers import dispatcher
from tests.common import get_test_home_assistant

TEST_USERNAME = 'test@domain.com'
TEST_PASSWORD = 'password'

class TestArloSensor(unittest.TestCase):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expected 2 blank lines, found 1

"""Test Netgear Arlo sensors."""

def _setup_arlo(self):
self.hass.data = {
DATA_ARLO: 'test'
}

@staticmethod
def _get_mock_sensor(name='Last', sensor_type='last_capture', data=None):
if data is None:
data = {}
return arlo.ArloSensor(name, data, sensor_type)

@staticmethod
def _get_named_tuple(input_dict):
return namedtuple('Struct', input_dict.keys())(*input_dict.values())

def setUp(self): # pylint: disable=invalid-name
"""Setup shared dependencies for each test."""
self.hass = get_test_home_assistant()

def tearDown(self): # pylint: disable=invalid-name
"""Tear down shared dependencies for each test."""
self.hass.stop()

def test_setup_with_no_data(self):
"""Test setup_platform with no data."""
result = arlo.setup_platform(self.hass, None, None)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setup_platform shouldn't return anything. Please fix that in the module and update this test.

self.assertEqual(result, False)

def test_setup_with_valid_data(self):
"""Test setup_platform with valid data."""
add_devices = mock.MagicMock()
config = {
'monitored_conditions': [
'last_capture',
'total_cameras',
'captured_today',
'battery_level',
'signal_strength',
'temperature',
'humidity',
'air_quality'
]
}
self.hass.data[DATA_ARLO] = MagicMock()
arlo.setup_platform(self.hass, config, add_devices)
add_devices.assert_called_once()

def test_sensor_name(self):
"""Test the name property."""
sensor = TestArloSensor._get_mock_sensor()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method is weirdly named since it doesn't seem to return a mock sensor but an actual instance of an arlo sensor.

Access methods using self..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will correct the method name and change to self access.

Also, just for reference as I'm coming back to python several years away, is it not optimal to use the @staticmethod decorator and treat these as "static" as opposed to members? Or is this just convention? Or is this an artifact of having this in a class as opposed to just standalone functions? Thanks for the help.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static methods can still be accessed on the instance, ie using self. inside instance methods. It's only inside the static method that the instance isn't available.

Yeah, the static methods are part of what could be cleaned up if we would move to standalone pytest tests.

self.assertEqual(sensor.name, 'Last')

@asyncio.coroutine

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, having just come back to python (after several years and from python2), async methods in python3 have come quite a long way, and I'm still getting accustomed to the new stuff:

When I omit this, it complains that the function is never awaited. In tests for other HA components, I noticed that standalone functions omit this, but functions declared in classes have it. Is this correlation true or just coincidence? If it does in fact need to be removed, what's the appropriate way to address the no-await issue?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're already declaring the method a coroutine by saying async def. This is the new async syntax in Python 3.5. So we shouldn't add the coroutine decorator on top of that. Class based unittest tests don't support coroutines as test methods. Here we have to move to standalone pytest tests.

@patch('homeassistant.helpers.dispatcher.async_dispatcher_connect', MagicMock())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (84 > 79 characters)

async def test_async_added_to_hass(self):
"""Test dispatcher called when added."""
sensor = TestArloSensor._get_mock_sensor()
await sensor.async_added_to_hass()
dispatcher.async_dispatcher_connect.assert_called_once()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer not to use assert_called_once but instead use assert len(mock.calls) == 1. A typo on the mock method would pass silently.


def test_sensor_state_default(self):
"""Test the state property."""
sensor = TestArloSensor._get_mock_sensor()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use self._get_mock_sensor().

self.assertIsNone(sensor.state)

def test_sensor_icon_battery(self):
"""Test the battery icon."""
data = TestArloSensor._get_named_tuple({
'battery_level': 50
})

sensor = TestArloSensor._get_mock_sensor('Battery Level', 'battery_level', data)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (88 > 79 characters)

self.assertEqual(sensor.icon, 'mdi:battery-50')

def test_sensor_icon(self):
"""Test the icon property."""
sensor = TestArloSensor._get_mock_sensor('Temperature', 'temperature')
self.assertEqual(sensor.icon, 'mdi:thermometer')

def test_unit_of_measure(self):
"""Test the unit_of_measurement property."""
sensor = TestArloSensor._get_mock_sensor()
self.assertIsNone(sensor.unit_of_measurement)
sensor = TestArloSensor._get_mock_sensor('Battery Level', 'battery_level')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (82 > 79 characters)

self.assertEqual(sensor.unit_of_measurement, '%')

def test_device_class(self):
"""Test the device_class property."""
sensor = TestArloSensor._get_mock_sensor()
self.assertIsNone(sensor.device_class)
sensor = TestArloSensor._get_mock_sensor('Temperature', 'temperature')
self.assertEqual(sensor.device_class, DEVICE_CLASS_TEMPERATURE)
sensor = TestArloSensor._get_mock_sensor('Humidity', 'humidity')
self.assertEqual(sensor.device_class, DEVICE_CLASS_HUMIDITY)

def test_update_total_cameras(self):
"""Test update method for total_cameras sensor type."""
data = TestArloSensor._get_named_tuple({
'cameras': [0, 0]
})
sensor = TestArloSensor._get_mock_sensor('Arlo Cameras', 'total_cameras', data)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (87 > 79 characters)

sensor.update()
self.assertEqual(sensor.state, 2)

def test_update_caputred_today(self):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo.

"""Test update method for captured_today sensor type."""
data = TestArloSensor._get_named_tuple({
'captured_today': [0, 0, 0, 0, 0]
})
sensor = TestArloSensor._get_mock_sensor('Captured Today', 'captured_today', data)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (90 > 79 characters)

sensor.update()
self.assertEqual(sensor.state, 5)

def _test_update(self, sensor_type, key, value):
data = TestArloSensor._get_named_tuple({
key: value
})
sensor = TestArloSensor._get_mock_sensor('test', sensor_type, data)
sensor.update()
self.assertEqual(sensor.state, value)

def test_update(self):
"""Test update method for direct transcription sensor types."""
self._test_update('battery_level', 'battery_level', 100)
self._test_update('signal_strength', 'signal_strength', 100)
self._test_update('temperature', 'ambient_temperature', 21.4)
self._test_update('humidity', 'ambient_humidity', 45.1)
self._test_update('air_quality', 'ambient_air_quality', 14.2)

def test_attributes_known_sensor(self):
"""Test attributes for known sensor type."""
data = TestArloSensor._get_named_tuple({
'model_id': 'ABC1000'
})
sensor = TestArloSensor._get_mock_sensor('test', 'humidity', data)
attrs = sensor.device_state_attributes
self.assertEqual(attrs.get(ATTR_ATTRIBUTION), 'Data provided by arlo.netgear.com')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (90 > 79 characters)

self.assertEqual(attrs.get('brand'), 'Netgear Arlo')
self.assertEqual(attrs.get('model'), 'ABC1000')