Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gearbest sensor #10556

Merged
merged 10 commits into from
Dec 5, 2017
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions .coveragerc
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ omit =
homeassistant/components/sensor/fixer.py
homeassistant/components/sensor/fritzbox_callmonitor.py
homeassistant/components/sensor/fritzbox_netmonitor.py
homeassistant/components/sensor/gearbest.py
homeassistant/components/sensor/geizhals.py
homeassistant/components/sensor/gitter.py
homeassistant/components/sensor/glances.py
Expand Down
1 change: 1 addition & 0 deletions CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ homeassistant/components/media_player/kodi.py @armills
homeassistant/components/media_player/monoprice.py @etsinko
homeassistant/components/media_player/yamaha_musiccast.py @jalmeroth
homeassistant/components/sensor/airvisual.py @bachya
homeassistant/components/sensor/gearbest.py @HerrHofrat
homeassistant/components/sensor/irish_rail_transport.py @ttroy50
homeassistant/components/sensor/miflora.py @danielhiversen
homeassistant/components/sensor/sytadin.py @gautric
Expand Down
119 changes: 119 additions & 0 deletions homeassistant/components/sensor/gearbest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
"""
Parse prices of a item from gearbest.
For more details about this platform, please refer to the documentation at
https://home-assistant.io/components/sensor.gearbest/
"""
import logging
import asyncio

Choose a reason for hiding this comment

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

'asyncio' imported but unused

from datetime import timedelta

import voluptuous as vol

from homeassistant.components.sensor import PLATFORM_SCHEMA
import homeassistant.helpers.config_validation as cv
from homeassistant.util import Throttle
from homeassistant.helpers.entity import Entity

REQUIREMENTS = ['gearbest_parser==1.0.4']
_LOGGER = logging.getLogger(__name__)

CONF_ITEMS = 'items'
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea to factor these out. However, at least CONF_NAME, CONF_URL, CONF_ID and CONF_CURRENCY should be imported from homeassistant/const.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing this out! What do you think about using CONF_ENTITIES instead of items?

Copy link
Contributor

Choose a reason for hiding this comment

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

In the context of home assistant, entity is a very fixed term for an object representing a physical (or virtual) device, something that has a state, belongs to a component, offers services, can be represented in the front end, etc. So, no, don't use that term for Gearbest items. ;-)

CONF_CURRENCY = 'currency'

ICON = 'mdi:coin'
MIN_TIME_BETWEEN_UPDATES = timedelta(seconds=2*60*60) #2h * 60min * 60sec

Choose a reason for hiding this comment

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

at least two spaces before inline comment
inline comment should start with '# '


_ITEM_SCHEMA = vol.Schema({
vol.Optional("url"): cv.string,
vol.Optional("id"): cv.string,
Copy link
Member

Choose a reason for hiding this comment

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

Not all items can be optional, at least something should be required or else how can one represent an item?

vol.Optional("name"): cv.string,
vol.Optional("currency"): cv.string
})

_ITEMS_SCHEMA = vol.Schema([_ITEM_SCHEMA])

PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
vol.Required(CONF_ITEMS): _ITEMS_SCHEMA,
vol.Required(CONF_CURRENCY): cv.string,
})


@asyncio.coroutine
def async_setup_platform(hass, config, async_add_devices, discovery_info=None):
"""Set up the Gearbest sensor."""

del discovery_info #unused

Choose a reason for hiding this comment

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

at least two spaces before inline comment
inline comment should start with '# '


hass.loop.run_in_executor(None, _add_items, hass, config, async_add_devices)

Choose a reason for hiding this comment

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

line too long (80 > 79 characters)


def _add_items(hass, config, async_add_devices):

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

Copy link
Member

Choose a reason for hiding this comment

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

This method can just be your setup_platform

currency = config.get(CONF_CURRENCY)

sensors = []
items = config.get(CONF_ITEMS)
for item in items:
try:
sensor = GearbestSensor(hass, item, currency)
Copy link
Member

Choose a reason for hiding this comment

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

sensor = GearbestSensor(converter, item, currency)

if sensor is not None:
Copy link
Member

Choose a reason for hiding this comment

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

This can't happen.

sensors.append(sensor)
except AttributeError as exc:
_LOGGER.error(exc)

async_add_devices(sensors)
Copy link
Member

Choose a reason for hiding this comment

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

Don't call async methods from sync context



class GearbestSensor(Entity):
"""Implementation of the sensor."""

def __init__(self, hass, item, currency):
Copy link
Member

Choose a reason for hiding this comment

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

Don't pass in hass. It will be set in the entity when it has been added to home assistant.

def __init__(self, converter, item, currency):

"""Initialize the sensor."""

from gearbest_parser import GearbestParser

self._hass = hass
Copy link
Member

Choose a reason for hiding this comment

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

Remove this.

self._name = item.get("name", None)
self._parser = GearbestParser()
self._parser.update_conversion_list()
Copy link
Member

Choose a reason for hiding this comment

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

The parser is updating this list once per entity. Instead can it be shared among entities?

self._item = self._parser.load(item.get("id", None),
item.get("url", None),
item.get("currency", currency))
if self._item is None:
raise AttributeError("id and url could not be resolved")
Copy link
Member

Choose a reason for hiding this comment

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

AttributeError? A ValueError maybe.

self._item.update()
Copy link
Member

Choose a reason for hiding this comment

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

Remove this, and instead call add_devices in setup_platform like this:

add_devices(sensors, True)


@property
def name(self):
"""Return the name of the item."""
return self._name if self._name is not None else self._item.name

@property
def icon(self):
"""Return the icon for the frontend."""
return ICON

@property
def state(self):
"""Return the price of the selected product."""
return self._item.price

@property
def unit_of_measurement(self):
"""Return the currency """
return self._item.currency

@property
def device_state_attributes(self):
"""Return the state attributes."""
attrs = {'name': self._item.name,
'description': self._item.description,
'image': self._item.image,
Copy link
Member

Choose a reason for hiding this comment

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

Don't make this an attribute, instead make it the entity_picture property.

'price': self._item.price,
Copy link
Member

Choose a reason for hiding this comment

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

Price is the state, so you don't need that here.

'currency': self._item.currency,
'url': self._item.url}
return attrs

@Throttle(MIN_TIME_BETWEEN_UPDATES)
def update(self):
"""Get the latest price from gearbest and updates the state."""
self._hass.loop.run_in_executor(None, self._parser.update_conversion_list)

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._hass.loop.run_in_executor(None, self._item.update)
3 changes: 3 additions & 0 deletions requirements_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,9 @@ gTTS-token==1.1.1
# homeassistant.components.device_tracker.bluetooth_le_tracker
# gattlib==0.20150805

# homeassistant.components.sensor.gearbest
gearbest_parser==1.0.4

# homeassistant.components.sensor.gitter
gitterpy==0.1.6

Expand Down