SleepIQ component with sensor and binary sensor platforms #2949

technicalpickles · 2016-08-23T21:56:21Z

Description:

My bed has an API, so obviously I want to integrate it into homeassistant :D SleepNumber beds have an addon called SleepIQ. It tracks sleep over time, and in particular, it does know if you are in bed or not, and it knows your 'SleepNumber' (firmness of the mattress). There are controls for changing the incline of the frame, and firmness, but it requires using bluetooth near the bed, and doesn't seem to be exposed by the API (I wrote the library sleepyq for interacting it, and made some notes: https://github.com/technicalpickles/sleepyq).

This is my first component, and my first real python, so definitely looking for feedback both in terms of style & implementation details. I started discussion about this over in https://community.home-assistant.io/t/feedback-on-component-for-sleepiq-beds/3132

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

Example entry for configuration.yaml (if applicable):

sleepiq:
  login: !secret sleepiq_login
  password: !secret sleepiq_password

Checklist:

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

Documentation added/updated in home-assistant.io

If code communicates with devices, web services, or a:

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.

balloobbot · 2016-08-23T21:57:23Z

homeassistant/components/binary_sensor/sleepiq.py

+For more details about this platform, please refer to the documentation at
+https://home-assistant.io/components/sensor.sleepiq/
+"""
+from homeassistant.components import sleepiq


farcy v1.1

1: F401 'sleepiq' imported but unused

This has been addressed.

technicalpickles · 2016-09-12T13:54:22Z

I'm having a hard time understanding the reason for them, but I've applied what I understand the feedback to be and confirmed the functionality is intact.

pvizeli · 2016-09-12T15:36:23Z

Perfect. On Sensor component do you have forget to delete the icon static variable or add you icon function (thats is allowed). Please use also volutuous for config validation (see on other component or homematic) And then is ready to merge. Good work 👍

pvizeli · 2016-09-12T18:06:24Z

Like that https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/homematic.py#L123-L136

pvizeli · 2016-09-13T04:53:03Z

Yeah, looks good 👍 pass all test and is done. I'll change the order of class/setup (first setup function and later the class) in your code after merge, so you don't need to do that if you don't want to do self.

technicalpickles · 2016-09-13T12:30:28Z

@pvizeli I made the changes, but now I have a test failure with voluptuous instead of using validate_config because I had test cases for the missing configuration. Is there a voluptuous way of testing config, or should I just remove those?

pvizeli · 2016-09-13T13:32:00Z

use bootstrap._setup_component See i.e. notify file test.

technicalpickles · 2016-09-13T17:41:27Z

@pvizeli added that, thanks!

pvizeli · 2016-09-13T19:33:32Z

tests/components/test_sleepiq.py

        """Test the setup when no login is configured."""
        del self.config['sleepiq']['username']
-        sleepiq.setup(self.hass, self.config)
-        self.assertFalse(sleepiq.setup(self.hass, self.config))
+        assert not bootstrap._setup_component(self.hass, sleepiq.DOMAIN, self.config)


You need copy config and del from copy or you del from main config and in next unit test it is also deleted

pvizeli · 2016-09-13T21:17:00Z

I've made a clean PR without merges and make some code cleanups. Can you test it? #3390

Original from #2949

technicalpickles · 2016-09-13T23:31:10Z

@pvizeli if you wanted a clean branch, I could have made one. You squashed the commits, so I can't easily see what changes you made or why so I can learn, and it also don't count as a contribution from me.

pvizeli · 2016-09-14T05:12:34Z

You are the mentainer and you stay in release note. On Home-Assistant developer guide is written that PR should be rebased and squash, thats all. For new PR we have upstream rights to help but that was before github have this feature. I want only help you and spend some hours for that.

balloob · 2016-09-14T05:19:00Z

Sorry @technicalpickles, we're not all GitHub guru's like you.

@pvizeli some tips for next time:

You can commit directly to his branch on his fork with the new improved collaboration features.

If you use the 'squash and merge' button on GitHub, the original author will remain intact. See example

pvizeli · 2016-09-14T08:25:46Z

I'm sorry. I've only want to help

balloob · 2016-09-14T14:34:31Z

Don't worry @pvizeli, we're all learning here. It happens

technicalpickles · 2016-09-14T21:41:11Z

I'm sorry if if my last comment came of short or antagonistic. I didn't mean it that way, it's just that this PR has been quite the voyage for me.

I'm new to Home Assistant development, and actually Python too. It wasn't easy trying to pick it all up at once, but I learned a ton along the way. I was mostly disappointed in not being able to get it merged myself. I wanted to know what changes would be needed to be merged for my own learning and also to contribute better components in the future, that is all. I will gladly trade some Git and GitHub expertise for Python and Home Assistant expertise 😀

Home Assistant is probably the best managed open source project I've watched and participated in in some time. Thanks for everyone that left feedback and thanks to @pvizeli in particular for cleaning up the PR and getting it merged for 0.29.

…tant#3390) Original from home-assistant#2949

First pass at SleepIQ component with sensor and binary sensor platforms

19a1cec

balloobbot reviewed Aug 23, 2016
View reviewed changes

Move state and icon down into sleepiqsensor base class

d52fec9

technicalpickles added 3 commits September 12, 2016 17:13

Merge branch 'dev' into sleepiq

827b4d9

Add back icon for sleepnumbersensor

277e3fc

use voluptuous

6bdb246

pvizeli self-assigned this Sep 13, 2016

pvizeli added this to the 0.29 milestone Sep 13, 2016

use _setup_component to confirm validation

97ba63a

pvizeli reviewed Sep 13, 2016
View reviewed changes

pvizeli mentioned this pull request Sep 13, 2016

SleepIQ component with sensor and binary sensor platforms #3390

Merged

pvizeli added a commit that referenced this pull request Sep 13, 2016

SleepIQ component with sensor and binary sensor platforms (#3390)

1697a8c

Original from #2949

pvizeli closed this Sep 13, 2016

technicalpickles deleted the sleepiq branch September 14, 2016 23:32

hartmms pushed a commit to hartmms/home-assistant that referenced this pull request Sep 17, 2016

SleepIQ component with sensor and binary sensor platforms (home-assis…

892d9bc

…tant#3390) Original from home-assistant#2949

home-assistant locked and limited conversation to collaborators Mar 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SleepIQ component with sensor and binary sensor platforms #2949

SleepIQ component with sensor and binary sensor platforms #2949

technicalpickles commented Aug 23, 2016 •

edited

balloobbot Aug 23, 2016

technicalpickles Aug 25, 2016

technicalpickles commented Sep 12, 2016

pvizeli commented Sep 12, 2016

pvizeli commented Sep 12, 2016

pvizeli commented Sep 13, 2016 •

edited

technicalpickles commented Sep 13, 2016

pvizeli commented Sep 13, 2016

technicalpickles commented Sep 13, 2016

pvizeli Sep 13, 2016

pvizeli commented Sep 13, 2016

technicalpickles commented Sep 13, 2016

pvizeli commented Sep 14, 2016

balloob commented Sep 14, 2016 •

edited

pvizeli commented Sep 14, 2016

balloob commented Sep 14, 2016

technicalpickles commented Sep 14, 2016

SleepIQ component with sensor and binary sensor platforms #2949

SleepIQ component with sensor and binary sensor platforms #2949

Conversation

technicalpickles commented Aug 23, 2016 • edited

balloobbot Aug 23, 2016

Choose a reason for hiding this comment

technicalpickles Aug 25, 2016

Choose a reason for hiding this comment

technicalpickles commented Sep 12, 2016

pvizeli commented Sep 12, 2016

pvizeli commented Sep 12, 2016

pvizeli commented Sep 13, 2016 • edited

technicalpickles commented Sep 13, 2016

pvizeli commented Sep 13, 2016

technicalpickles commented Sep 13, 2016

pvizeli Sep 13, 2016

Choose a reason for hiding this comment

pvizeli commented Sep 13, 2016

technicalpickles commented Sep 13, 2016

pvizeli commented Sep 14, 2016

balloob commented Sep 14, 2016 • edited

pvizeli commented Sep 14, 2016

balloob commented Sep 14, 2016

technicalpickles commented Sep 14, 2016

technicalpickles commented Aug 23, 2016 •

edited

pvizeli commented Sep 13, 2016 •

edited

balloob commented Sep 14, 2016 •

edited