Add juicenet platform by jesserockz · Pull Request #7668 · home-assistant/core

jesserockz · 2017-05-20T10:27:11Z

Description:

The juicenet sensor platform pulls data from a JuiceNet Charging station equipped with a wifi connection.

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

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

juicenet:
    access_token: ACCESS_TOKEN

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.

mention-bot · 2017-05-20T10:27:15Z

@jesserockz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @robbiet480 to be potential reviewers.

houndci-bot · 2017-05-20T10:27:18Z

+        attributes = super().device_state_attributes
+        if self.type == 'status':
+            if man_dev_id:
+                attributes["manufacturer_device_id"] = man_dev_id


undefined name 'man_dev_id'

houndci-bot · 2017-05-20T10:27:19Z

+        """Return the state attributes."""
+        attributes = super().device_state_attributes
+        if self.type == 'status':
+            if man_dev_id:


undefined name 'man_dev_id'

houndci-bot · 2017-05-20T10:27:19Z

+    'energy_added': ['Energy added', 'Wh']
+}
+
+def setup_platform(hass, config, add_devices, discovery_info=None):


expected 2 blank lines, found 1

houndci-bot · 2017-05-20T10:27:19Z

+
+    return True
+
+class JuicenetDevice(Entity):


expected 2 blank lines, found 1

houndci-bot · 2017-05-20T10:27:19Z

+    })
+}, extra=vol.ALLOW_EXTRA)
+
+def setup(hass, config):


expected 2 blank lines, found 1

Add missing blank lines

houndci-bot · 2017-05-20T11:07:34Z

+
+class JuicenetSensorDevice(JuicenetDevice, Entity):
+    """Implementation of a Juicenet sensor."""
+


blank line contains whitespace

Add missing docstring

balloob · 2017-05-23T21:52:26Z

+        _LOGGER.info("Refreshing Juicenet states from API")
+        for entity_list in hass[DOMAIN]['entities'].values():
+            for entity in entity_list:
+                time.sleep(1)


Please don't use sleep. Can you fetch all the data at once and then notifiy the entities with new info?

balloob · 2017-05-23T21:53:32Z

+    def pull_new_devices(call):
+        """Pull new devices added to users Juicenet account since startup."""
+        _LOGGER.info("Getting new devices from Juicenet API")
+        discovery.load_platform(hass, 'sensor', DOMAIN, {}, config)


This is a very weird way of "pulling" new devices. Why can't you talk to the API directly?

balloob · 2017-05-23T21:54:50Z

+    dev = []
+    for device in api.get_devices():
+        _id = device.id()
+        if _id not in hass.data[DOMAIN]['unique_ids']:


You don't have to do this, Home Assistant does this automatically as long as you implement the property unique_id.

Use the hass built in unique_id

houndci-bot · 2017-05-23T22:27:08Z

+    for device in api.get_devices():
+        _id = device.id()
+        for variable in SENSOR_TYPES:
+            if "{}-{}".format(_id, variable) not in hass.data[DOMAIN]['unique_ids']:


line too long (84 > 79 characters)

houndci-bot · 2017-05-23T22:27:08Z

+"""
+
+import logging
+import time


'time' imported but unused

balloob · 2017-06-05T06:23:54Z

+    @asyncio.coroutine
+    def async_added_to_hass(self):
+        """Callback when entity is added to hass."""
+        self.hass.data[DOMAIN]['entities']['sensor'].append(self)


Why are you appending it, is this still necessary?

Same as above

balloob · 2017-06-05T06:24:02Z

+    for device in api.get_devices():
+        for variable in SENSOR_TYPES:
+            _id = "{}-{}".format(device.id(), variable)
+            if _id not in hass.data[DOMAIN]['unique_ids']:


This check is no longer necessary

Thanks, I was writing this integration while looking at the wink one.

balloob · 2017-06-05T06:24:49Z

+        elif self.type == 'energy_added':
+            icon = 'mdi:flash'
+        else:
+            icon = 'mdi:eye'


You should return None. That will cause Home Assistant to use the default sensor icon.

balloob · 2017-06-05T06:25:35Z

+    def device_state_attributes(self):
+        """Return the state attributes."""
+        attributes = {}
+        if self.type == 'status':


This shouldn't be in a generic JuiceNet entity. This is part of an implementation.

balloob · 2017-06-05T06:25:57Z

+
+    hass.data[DOMAIN] = {}
+    hass.data[DOMAIN]['unique_ids'] = []
+    hass.data[DOMAIN]['entities'] = {}


You're no longer using this?

Same as above

jesserockz · 2017-06-05T07:20:34Z

Thanks for the feedback @balloob, Im guessing homeassistant has evolved a bit since the wink platform was written.

houndci-bot · 2017-06-05T08:28:23Z

+at https://home-assistant.io/components/sensor.juicenet/
+"""
+
+import asyncio


'asyncio' imported but unused

jesserockz · 2017-06-05T09:43:05Z

@balloob It looks like a previous PR merge has broken the lint test #7318
code link

balloob

Looks great 🐬 👍 Thanks!

Add juicenet platform

221c29c

homeassistant added new-platform integration: juicenet platform: sensor.juicenet cla-signed labels May 20, 2017

balloobbot added component platform requirement labels May 20, 2017

houndci-bot reviewed May 20, 2017

View reviewed changes

jesserockz mentioned this pull request May 20, 2017

Add juicenet platform home-assistant/home-assistant.io#2670

Merged

Update missing variable

fefa866

Add missing blank lines

homeassistant added the cla-signed label May 20, 2017

Jesse Hills added 2 commits May 20, 2017 22:55

Remove unnecessary override

ee234a4

Update juicenet.py

e62b243

houndci-bot reviewed May 20, 2017

View reviewed changes

Remove whitespace

884457c

Add missing docstring

balloob reviewed May 23, 2017

View reviewed changes

Remove unused services

d198aaa

Use the hass built in unique_id

houndci-bot reviewed May 23, 2017

View reviewed changes

Jesse Hills added 3 commits May 24, 2017 10:28

Fix lint issues

243eadc

Update python-juicenet library version

9448e85

Update python-juicenet library version

eceadf7

balloob reviewed Jun 5, 2017

View reviewed changes

Remove unnecessary code

34231a2

homeassistant added the cla-signed label Jun 5, 2017

houndci-bot reviewed Jun 5, 2017

View reviewed changes

Jesse Hills added 2 commits June 5, 2017 20:29

Remove unused import

1a5bb9e

Remove super call

8b023b8

balloob approved these changes Jun 5, 2017

View reviewed changes

balloob merged commit aee25a0 into home-assistant:dev Jun 5, 2017

balloob mentioned this pull request Jun 16, 2017

0.47 #8055

Merged

home-assistant locked and limited conversation to collaborators Sep 4, 2017

jesserockz deleted the juicenet-platform branch February 8, 2018 04:16

ghost removed the platform: sensor.juicenet label Mar 21, 2019


		class JuicenetSensorDevice(JuicenetDevice, Entity):
		"""Implementation of a Juicenet sensor."""

Uh oh!

Conversation

jesserockz commented May 20, 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!

mention-bot commented May 20, 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!

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!

jesserockz commented Jun 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesserockz commented Jun 5, 2017

Uh oh!

balloob left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jesserockz commented May 20, 2017 •

edited

Loading

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