SleepIQ component with sensor and binary sensor platforms #2949
Conversation
| For more details about this platform, please refer to the documentation at | ||
| https://home-assistant.io/components/sensor.sleepiq/ | ||
| """ | ||
| from homeassistant.components import sleepiq |
There was a problem hiding this comment.
- 1: F401 'sleepiq' imported but unused
There was a problem hiding this comment.
This has been addressed.
|
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. |
|
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 👍 |
|
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. |
|
@pvizeli I made the changes, but now I have a test failure with voluptuous instead of using |
|
use |
|
@pvizeli added that, thanks! |
| """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) |
There was a problem hiding this comment.
You need copy config and del from copy or you del from main config and in next unit test it is also deleted
|
I've made a clean PR without merges and make some code cleanups. Can you test it? #3390 |
|
@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. |
|
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. |
|
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 |
|
I'm sorry. I've only want to help |
|
Don't worry @pvizeli, we're all learning here. It happens |
|
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. |
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):Checklist:
If user exposed functionality or configuration variables are added/changed:
If code communicates with devices, web services, or a:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.