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

Add IBM Watson IoT Platform component #13664

Merged
merged 1 commit into from Jun 7, 2018

Conversation

Projects
None yet
10 participants
@mtreinish
Contributor

mtreinish commented Apr 3, 2018

Description:

This commit adds a new history component for the IBM Watson IoT
Platform. The IBM Watson IoT Platform allows for tracking of devices
and analytics on top of the device data. This new component allows
users to have home assistant automatically populate a watson iot
platform board with device data from devices managed by home assistant.

Related issue (if applicable): fixes #

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

Example entry for configuration.yaml (if applicable):

watson_iot:
  organization: 'organization_id'
  type: 'device_type'
  id: 'device_id'
  token: 'auth_token'

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

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

If the code communicates with devices, web services, or third-party tools:

  • 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/components/watson_iot.py Outdated
if item is None:
self.shutdown = True
else:
event_json = self.event_to_json(event)

This comment has been minimized.

@houndci-bot

houndci-bot Apr 3, 2018

undefined name 'event'

@stevemart

i came here out of interest to check out @mtreinish's PR, stumbled upon a typo or two. Neat stuff.

homeassistant/components/watson_iot.py Outdated
@@ -0,0 +1,215 @@
"""
A component which allows you to send data to the IBM Watson IOT Platforme.

This comment has been minimized.

@stevemart

stevemart Apr 4, 2018

typos: IoT* Platform*

This comment has been minimized.

@mtreinish

mtreinish Apr 4, 2018

Contributor

Fixed

@ezar

This comment has been minimized.

ezar commented Apr 4, 2018

Like it!

.coveragerc Outdated
@@ -712,6 +712,8 @@ omit =
homeassistant/components/tts/picotts.py
homeassistant/components/vacuum/mqtt.py
homeassistant/components/vacuum/roomba.py
homeassistant/components/vacuum/xiaomi_miio.py

This comment has been minimized.

@dgomes

dgomes May 5, 2018

Member

Why are you touching xiaomi_miio.py ?

This comment has been minimized.

@mtreinish

mtreinish May 11, 2018

Contributor

This was a rebase mistake, fixed in the next rev.

homeassistant/components/watson_iot.py Outdated
conf = config[DOMAIN]
include = conf.get(CONF_INCLUDE, {})
exclude = conf.get(CONF_EXCLUDE, {})

This comment has been minimized.

@dgomes

dgomes May 5, 2018

Member

You already defined defaults, no need to use get with default just use []

This comment has been minimized.

@mtreinish

mtreinish May 11, 2018

Contributor

Done

homeassistant/components/watson_iot.py Outdated
blacklist_d = set(exclude.get(CONF_DOMAINS, []))
client_args = {
'org': conf.get(CONF_ORG),

This comment has been minimized.

@dgomes

dgomes May 5, 2018

Member

CONF_ORG is required, use conf[CONF_ORG]

This comment has been minimized.

@mtreinish

mtreinish May 11, 2018

Contributor

Done

homeassistant/components/watson_iot.py Outdated
_state_as_value = float(state_helper.state_as_number(state))
_include_state = _include_value = True
except ValueError:
_include_state = True

This comment has been minimized.

@dgomes

dgomes May 5, 2018

Member

False ?

This comment has been minimized.

@mtreinish

mtreinish May 11, 2018

Contributor

You know I'm looking at this again, and I'm not sure what my intent was when I wrote this a month ago. False wouldn't make much sense because if we get a ValueError there _include_state will already be false. I can only assume I meant for it to be true, but I can't remember why. I'm just going to leave it as is, and if I remember what my intent was I'll add a comment here.

This comment has been minimized.

@dgomes

dgomes May 11, 2018

Member

Well I think that using exception handling to implement logic is not the best approach... maybe you can rework this part of the code into something more readable and less prone to confusion.

homeassistant/components/watson_iot.py Outdated
self.gateway = gateway
self.gateway.connect()
self.event_to_json = event_to_json
self.max_tries = 3

This comment has been minimized.

@dgomes

dgomes May 5, 2018

Member

Make 3 a constant declared in the beginning of the file

This comment has been minimized.

@mtreinish

mtreinish May 11, 2018

Contributor

Done

@balloob

This comment has been minimized.

Member

balloob commented May 17, 2018

May I ask why you closed this? This was so close to being done 🏁

@mtreinish

This comment has been minimized.

Contributor

mtreinish commented Jun 1, 2018

The latest revision reworks the logic around setting the device state in the output json to be a bit more clear to address @dgomes comments.

include = conf[CONF_INCLUDE]
exclude = conf[CONF_EXCLUDE]
whitelist_e = set(include.get(CONF_ENTITIES, []))

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 2, 2018

Member

The empty list default value is already set in the config schema. Don't use dict.get if there are defaults in place. Just use dict[key].

This comment has been minimized.

@mtreinish

mtreinish Jun 2, 2018

Contributor

Done

This comment has been minimized.

@mtreinish

mtreinish Jun 2, 2018

Contributor

Done

client_args = {
'org': conf[CONF_ORG],
'type': conf.get(CONF_TYPE),

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 2, 2018

Member

Don't use dict.get for required keys.

This comment has been minimized.

@mtreinish

mtreinish Jun 2, 2018

Contributor

Done

This comment has been minimized.

@mtreinish

mtreinish Jun 2, 2018

Contributor

Done

self.gateway = gateway
self.gateway.connect()
self.event_to_json = event_to_json
self.max_tries = MAX_TRIES

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 2, 2018

Member

You can use the constant directly. You don't need to store it in an instance attribute.

This comment has been minimized.

@mtreinish

mtreinish Jun 2, 2018

Contributor

Done

This comment has been minimized.

@mtreinish

mtreinish Jun 2, 2018

Contributor

Done

Add IBM Watson IoT Platform component
This commit adds a new history component for the IBM Watson IoT
Platform. The IBM Watson IoT Platform allows for tracking of devices
and analytics on top of the device data. This new component allows
users to have home assistant automatically populate a watson iot
platform board with device data from devices managed by home assistant.
@MartinHjelmare

Looks good!

@dgomes

dgomes approved these changes Jun 2, 2018

@dgomes

This comment has been minimized.

Member

dgomes commented Jun 2, 2018

Thanks for coming back and finishing :)

@balloob balloob merged commit d14d2fe into home-assistant:dev Jun 7, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 93.973%
Details

@wafflebot wafflebot bot removed the Ready for review label Jun 7, 2018

@mtreinish mtreinish deleted the mtreinish:watson-iot branch Jun 7, 2018

balloob added a commit to home-assistant/home-assistant.io that referenced this pull request Jun 21, 2018

Add documentation for the new watson_iot component (#5100)
* Add documentation for the new watson_iot component

This commit adds documention for the new watson_iot component which was
added in home-assistant/home-assistant#13664
It links to the official documentation for setting up the cloud service
and generating the required auth information.

* ✏️ Missing comma

* Use configuration tags

This commit updates the formatting of the configuration section of the
documentation. Previously it manually formatted a list instead of
leveraging the templating format. This corrects that oversight and uses
the standard format.

* 🎨 Adds logo

* ⬆️ ha_release -> 0.68

* ✏️ Processed RFC's

balloob added a commit to home-assistant/home-assistant.io that referenced this pull request Jun 21, 2018

Add documentation for the new watson_iot component (#5100)
* Add documentation for the new watson_iot component

This commit adds documention for the new watson_iot component which was
added in home-assistant/home-assistant#13664
It links to the official documentation for setting up the cloud service
and generating the required auth information.

* ✏️ Missing comma

* Use configuration tags

This commit updates the formatting of the configuration section of the
documentation. Previously it manually formatted a list instead of
leveraging the templating format. This corrects that oversight and uses
the standard format.

* 🎨 Adds logo

* ⬆️ ha_release -> 0.68

* ✏️ Processed RFC's

@balloob balloob referenced this pull request Jun 22, 2018

Merged

0.72 #15088

@gnkarn

This comment has been minimized.

gnkarn commented Jun 28, 2018

it would be good to add more detail on a config example , as following the docs , the config fails in loading the component :
in my case the config is simple but couldnt make it to work

watson_iot:
  organization: 'xxxxx'
  type: 'gateway'
  id: 'watson-hass-xxxx'
  token: 'xxxxxxxx'
  include:
    entities:
      - sensor.home_energy_sensor_current

--
tks

@mtreinish

This comment has been minimized.

Contributor

mtreinish commented Jun 28, 2018

@gnkarn hmm, that config looks valid can you please open an issue with the details including the error in the logs when the component fails to load. (there likely is a stack trace in the logs if it's erroring on load) I'll take a look at it as soon as I can once you open it, there might be a bug in the include/exclude configuration I didn't test that bit as thoroughly as the rest of the component. But from a quick glance nothing looks wrong with that.

@robmarkcole

This comment has been minimized.

Contributor

robmarkcole commented Jun 28, 2018

@mtreinish have you published anywhere the analysis you have performed on the Watson platform?

@gnkarn

This comment has been minimized.

gnkarn commented Jun 28, 2018

@mtreinish there is nothing of help on the logs , as the configuration of the component : could not be loaded .
i have not deleting the include as i only want to publish a few sensors , starting with one .
tks

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Jun 28, 2018

@balloob

This comment has been minimized.

Member

balloob commented Jun 28, 2018

No support in pull requests. Open an issue as requested.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.