Added support for TekSavvy bandwidth sensor by jpjodoin · Pull Request #11186 · home-assistant/core

jpjodoin · 2017-12-17T18:22:15Z

Description:

TekSavvy is a an internet provider in Canada. This sensors allows to monitor bandwidth usage.

Related issue (if applicable): fixes #

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

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

sensor:
  - platform: teksavvy
    api_key: API_KEY
    total_bandwidth: 400
    monitored_variables:
     - usage
     - usage_gb
     - limit
     - onpeak_download
     - onpeak_upload
     - onpeak_total
     - offpeak_download
     - offpeak_upload
     - offpeak_total
     - onpeak_remaining

Checklist:

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:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
New dependencies have been added to the REQUIREMENTS variable (example).
New dependencies are only imported inside functions that use them (example).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

homeassistant · 2017-12-17T18:22:16Z

Hi @jpjodoin,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

fabaff · 2017-12-20T09:56:25Z

+        self.data = dict()
+        self.data["limit"] = self.bandwidth_cap
+
+    def _fetch_data(self):


Docstrings are required everywhere.

Done. Would be nice if we could add that to lint.

fabaff · 2017-12-20T10:03:09Z

+        conn.request('GET', req, '', headers)
+        response = conn.getresponse()
+        json_data = response.read().decode("utf-8")
+        data = json.loads(json_data)


One option would be to use the existing rest sensor to retrieve the data as it's done for the pvoutput platform. A more modern approach would be to do it with helpers.aiohttp_client like here and here.

Refactored the code to use aiohttp_client.

Thanks !

houndci-bot · 2017-12-24T02:54:24Z

+        url = "https://api.teksavvy.com/"\
+              "web/Usage/UsageSummaryRecords?$filter=IsCurrent%20eq%20true"
+        with async_timeout.timeout(REQUEST_TIMEOUT, loop=self.loop):
+            req = yield from self.websession.get(url, headers = headers)


unexpected spaces around keyword / parameter equals

houndci-bot · 2017-12-24T02:54:25Z

+        if self.type in self.teksavvydata.data:
+            self._state = round(self.teksavvydata.data[self.type], 2)
+
+class TekSavvyData(object):


expected 2 blank lines, found 1

houndci-bot · 2017-12-24T02:54:25Z

+        sensors.append(TekSavvySensor(ts_data, variable, name))
+    async_add_devices(sensors, True)
+
+class TekSavvySensor(Entity):


expected 2 blank lines, found 1

houndci-bot · 2017-12-24T02:54:25Z

+        ret = yield from ts_data.async_update()
+    except ValueError as error:
+        _LOGGER.error("Failed conversion %s %s", CONF_TOTAL_BANDWIDTH, error)
+    if ret == False:


comparison to False should be 'if cond is False:' or 'if not cond:'

houndci-bot · 2017-12-24T02:54:25Z

+    vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string,
+})
+
+@asyncio.coroutine


expected 2 blank lines, found 1

houndci-bot · 2017-12-24T02:54:25Z

+import logging
+from datetime import timedelta
+import http.client
+import json


'json' imported but unused

houndci-bot · 2017-12-24T02:54:25Z

+"""
+import logging
+from datetime import timedelta
+import http.client


'http.client' imported but unused

jpjodoin · 2018-01-07T15:43:17Z

@fabaff Ready on my side, let me know if I should change anything else. Thanks!

fabaff · 2018-01-15T21:22:40Z

+You can get your API key here:
+https://myaccount.teksavvy.com/ApiKey/ApiKeyManagement
+
+TekSavvy only counts download only as part of the bandwidth


This belongs to the documentation.

fabaff · 2018-01-15T21:23:52Z

+import homeassistant.helpers.config_validation as cv
+from homeassistant.util import Throttle
+from homeassistant.const import (
+    CONF_API_KEY, CONF_NAME, CONF_MONITORED_VARIABLES)


Please keep the import groups and sorted. Use isort if you don't want to do it manually.

Done. Thanks for the isort tip.

fabaff · 2018-01-15T21:25:02Z

+    vol.Required(CONF_MONITORED_VARIABLES):
+        vol.All(cv.ensure_list, [vol.In(SENSOR_TYPES)]),
+    vol.Required(CONF_API_KEY): cv.string,
+    vol.Required(CONF_TOTAL_BANDWIDTH): cv.string,


Only allow integers here. This way you don't need the casting on line 73.

Done. Allows only positive integer now.

fabaff · 2018-01-15T21:28:54Z

+    def async_update(self):
+        """Get the TekSavvy bandwidth data from the web service."""
+        headers = {"TekSavvy-APIKey": self.api_key}
+        _LOGGER.info("Updaing TekSavvy data")


Make this _LOGGER.debug. Less verbose.

fabaff · 2018-01-15T21:32:59Z

+        self.websession = websession
+        self.api_key = api_key
+        self.bandwidth_cap = bandwidth_cap
+        self.data = dict()


You can use {} here.

fabaff · 2018-01-15T21:36:33Z

+    except ValueError as error:
+        _LOGGER.error("Failed conversion %s %s", CONF_TOTAL_BANDWIDTH, error)
+    if ret is False:
+        return


You should log an invalid API key. I guess that for the users it's not very helpful that a conversation failed.

Removed the conversion since we no longer have the cast. Also, added an error message if the user has an invalid API key.

jpjodoin · 2018-01-18T03:26:48Z

@fabaff : Changes are done on my side. Thanks.

jpjodoin · 2018-01-18T21:54:39Z

Thanks!

Added support for TekSavvy bandwidth sensor

71aacf3

homeassistant added new-platform platform: sensor.teksavvy cla-needed labels Dec 17, 2017

balloobbot added the platform label Dec 17, 2017

homeassistant added cla-signed and removed cla-needed labels Dec 17, 2017

Variable name fix

d39afa9

homeassistant added the cla-signed label Dec 17, 2017

jpjodoin mentioned this pull request Dec 17, 2017

Added TekSavvy data usage sensor documentation. home-assistant/home-assistant.io#4227

Merged

2 tasks

Units are Gigabytes and not Gigabits

5b7a653

fabaff reviewed Dec 20, 2017

View reviewed changes

Change REST calls to use asyncio

5c4a242

houndci-bot reviewed Dec 24, 2017

View reviewed changes

jpjodoin added 2 commits December 23, 2017 21:59

houndci violation fix

7e89944

pylint fix

036f846

fabaff requested changes Jan 15, 2018

View reviewed changes

Fix for code review

f96250a

homeassistant added the cla-signed label Jan 17, 2018

Import order asyncxxx:wq

3d7f247

Tweak docstrings and log message

3341005

homeassistant added the cla-signed label Jan 18, 2018

home-assistant deleted a comment from houndci-bot Jan 18, 2018

fabaff approved these changes Jan 18, 2018

View reviewed changes

fabaff merged commit 8ca45ac into home-assistant:dev Jan 18, 2018

balloob mentioned this pull request Jan 26, 2018

0.62 #11921

Merged

home-assistant locked and limited conversation to collaborators May 29, 2018

ghost added integration: teksavvy and removed platform: sensor.teksavvy labels Mar 21, 2019

jpjodoin deleted the teksavvy branch August 27, 2019 20:19

Uh oh!

Conversation

jpjodoin commented Dec 17, 2017 • 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!

homeassistant commented Dec 17, 2017

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpjodoin commented Jan 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

jpjodoin commented Jan 18, 2018

Uh oh!

jpjodoin commented Jan 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jpjodoin commented Dec 17, 2017 •

edited

Loading

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

jpjodoin commented Jan 7, 2018 •

edited

Loading