Add ClickSend notify service. by oozman · Pull Request #8135 · home-assistant/core

oozman · 2017-06-21T07:11:59Z

The clicksend platform uses ClickSend to deliver notifications from Home Assistant.

Get your ClickSend API Credentials

Go to your ClickSend Dashboard section and create your new project. After creating your project, you should now be able to obtain your username and api_key.

Checklist:

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

Documentation added/updated in home-assistant.github.io. See: PR #2868

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 files were added to .coveragerc.

homeassistant · 2017-06-21T07:12:01Z

Hi @OmarUsman,

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!

houndci-bot · 2017-06-21T07:12:03Z

+        # Display error when failed.
+        if resp.status_code != 200:
+            _LOGGER.error("Error %s : %s (Code %s)", resp.status_code,
+                          response_msg, response_code)


no newline at end of file

houndci-bot · 2017-06-21T07:12:03Z

+        data = {'messages': [{'source': 'hass.notify', 'from': self.to_no, 'to': self.to_no, 'body': message}]}
+        headers = {'Content-type': 'application/json', 'Authorization': auth}
+
+        resp = requests.post(CONF_API_URL, data=json.dumps(data), headers=headers)


line too long (82 > 79 characters)

Add a timeout to the request.

houndci-bot · 2017-06-21T07:12:03Z

+        auth = base64.b64encode(bytes(auth, 'utf-8'))
+        auth = 'Basic ' + auth.decode('utf-8')
+
+        data = {'messages': [{'source': 'hass.notify', 'from': self.to_no, 'to': self.to_no, 'body': message}]}


line too long (111 > 79 characters)

houndci-bot · 2017-06-21T07:12:03Z

+    return ClicksendNotificationService(config[CONF_USERNAME], config[CONF_API_KEY], config[CONF_TO_NO])
+
+# Implement the notification service.
+class ClicksendNotificationService(BaseNotificationService):


expected 2 blank lines, found 1

houndci-bot · 2017-06-21T07:12:04Z

+def get_service(hass, config, discovery_info=None):
+
+    # Set notification service instance.
+    return ClicksendNotificationService(config[CONF_USERNAME], config[CONF_API_KEY], config[CONF_TO_NO])


line too long (104 > 79 characters)

Would be nice if you could check if the credentials are valid and make setup fails if not. Otherwise the users end-up with a non-working platform in their setup.

houndci-bot · 2017-06-21T07:12:04Z

+})
+
+# Define service instance.
+def get_service(hass, config, discovery_info=None):


expected 2 blank lines, found 1

fabaff · 2017-06-21T07:28:06Z

+"""
+Clicksend platform for notify component.
+For more details about this platform, please refer to the documentation at
+https://clicksend.com/help


Please point to the documentation at home-assistant.io.

Is this value fine: https://home-assistant.io/docs/
Because I haven't made a specific link for the documentation yet?

The link will be https://home-assistant.io/components/notify.clicksend/ for the platform documentation.

If the configuration is not passing the validation then a log entry is created which is composed on-the-fly and takes the platform name into account.

Thanks just made the changes.

fabaff · 2017-06-21T07:28:10Z

+# Set platform  parameters.
+CONF_API_URL = 'https://rest.clicksend.com/v3/sms/send'
+CONF_USERNAME = 'username'
+CONF_API_KEY = 'api_key'


Please use the constants defined in const.py.

fabaff · 2017-06-21T07:28:38Z

+def get_service(hass, config, discovery_info=None):
+
+    # Set notification service instance.
+    return ClicksendNotificationService(config[CONF_USERNAME], config[CONF_API_KEY], config[CONF_TO_NO])


Would be nice if you could check if the credentials are valid and make setup fails if not. Otherwise the users end-up with a non-working platform in their setup.

fabaff · 2017-06-21T07:29:41Z

+    """Implementation of a notification service for the Twitter service."""
+
+    def __init__(self, username, api_key, to_no):
+


Docstring is missing.

fabaff · 2017-06-21T07:29:44Z

+        self.to_no = to_no
+
+    def send_message(self, message="", **kwargs):
+


fabaff · 2017-06-21T07:29:47Z

+    def send_message(self, message="", **kwargs):
+
+        # Send request.
+        auth = self.username + ':' + self.api_key


Please use string formatting.

fabaff · 2017-06-21T07:29:57Z

+        auth = 'Basic ' + auth.decode('utf-8')
+
+        data = {'messages': [{'source': 'hass.notify', 'from': self.to_no, 'to': self.to_no, 'body': message}]}
+        headers = {'Content-type': 'application/json', 'Authorization': auth}


As far as I remember are there constants available for Content-type and application/json.

fabaff · 2017-06-21T07:29:59Z

+        data = {'messages': [{'source': 'hass.notify', 'from': self.to_no, 'to': self.to_no, 'body': message}]}
+        headers = {'Content-type': 'application/json', 'Authorization': auth}
+
+        resp = requests.post(CONF_API_URL, data=json.dumps(data), headers=headers)


Add a timeout to the request.

- Some code spacing fixes. - Add timeout to requests. - Change doc url. - Use const.py as much as possible. - Check credentials to determine if setup fails or not. - Add docstrings. - Use string formatting.

houndci-bot · 2017-06-22T04:23:52Z

+def _authenticate(config):
+    """Authenticate with ClickSend."""
+    api_url = '{}/account'.format(BASE_API_URL)
+    headers = {HTTP_HEADER_CONTENT_TYPE: CONTENT_TYPE_JSON}


local variable 'headers' is assigned to but never used

houndci-bot · 2017-06-22T04:23:52Z

+        api_url = "{}/sms/send".format(BASE_API_URL)
+
+        resp = requests.post(api_url, data=json.dumps(data), headers=headers,
+                            auth=(self.username, self.api_key), timeout=5)


continuation line under-indented for visual indent

houndci-bot · 2017-06-22T04:23:52Z

+import voluptuous as vol
+
+from homeassistant.const import (CONF_USERNAME, CONF_API_KEY, CONF_RECIPIENT,
+    HTTP_HEADER_CONTENT_TYPE, CONTENT_TYPE_JSON, HTTP_BASIC_AUTHENTICATION)


continuation line under-indented for visual indent

houndci-bot · 2017-06-22T04:23:52Z

+
+import voluptuous as vol
+
+from homeassistant.const import (CONF_USERNAME, CONF_API_KEY, CONF_RECIPIENT,


'homeassistant.const.HTTP_BASIC_AUTHENTICATION' imported but unused

houndci-bot · 2017-06-22T04:23:52Z

+import logging
+import requests
+import json
+import base64


'base64' imported but unused

- Remove unused variables. - Continuation line under-indented for visual indent.

houndci-bot · 2017-06-22T04:38:15Z

+
+    resp = requests.get(api_url, headers=headers,
+                        auth=(config.get(CONF_USERNAME),
+                        config.get(CONF_API_KEY)), timeout=5)


continuation line under-indented for visual indent

houndci-bot · 2017-06-22T04:38:15Z

+            _LOGGER.error("Error %s : %s (Code %s)", resp.status_code,
+                          response_msg, response_code)
+
+


blank line contains whitespace

houndci-bot · 2017-06-22T04:38:15Z

+import voluptuous as vol
+
+from homeassistant.const import (CONF_USERNAME, CONF_API_KEY,
+                                 CONF_RECIPIENT,HTTP_HEADER_CONTENT_TYPE,


missing whitespace after ','

- Format code based on PEP8.

- Remove unused base64 dependency.

- Fix: D205: 1 blank line required between summary line and description (found 0) - Fix: standard import "import json" comes before "import requests"

- Add files to .coveragerc

* Add ClickSend notify service. * PR home-assistant#8135 changes. - Some code spacing fixes. - Add timeout to requests. - Change doc url. - Use const.py as much as possible. - Check credentials to determine if setup fails or not. - Add docstrings. - Use string formatting. * PR home-assistant#8135 changes. - Remove unused variables. - Continuation line under-indented for visual indent. * PR home-assistant#8135 changes. - Format code based on PEP8. * PR home-assistant#8135 changes. - Remove unused base64 dependency. * PR home-assistant#8135 changes. - Fix: D205: 1 blank line required between summary line and description (found 0) - Fix: standard import "import json" comes before "import requests" * PR home-assistant#8135 changes. - Add files to .coveragerc * Remove obvious comments and set constant

Add ClickSend notify service.

9a07577

homeassistant added new-platform platform: notify.clicksend cla-needed labels Jun 21, 2017

houndci-bot reviewed Jun 21, 2017

View reviewed changes

homeassistant added cla-signed and removed cla-needed labels Jun 21, 2017

fabaff requested changes Jun 21, 2017

View reviewed changes

PR #8135 changes.

9f2ebb8

- Some code spacing fixes. - Add timeout to requests. - Change doc url. - Use const.py as much as possible. - Check credentials to determine if setup fails or not. - Add docstrings. - Use string formatting.