Skip to content

Add light control to Sleepiq integration#31322

Closed
sluzynsk wants to merge 2 commits into
home-assistant:devfrom
sluzynsk:sleepiq_lights
Closed

Add light control to Sleepiq integration#31322
sluzynsk wants to merge 2 commits into
home-assistant:devfrom
sluzynsk:sleepiq_lights

Conversation

@sluzynsk
Copy link
Copy Markdown

Proposed change

Additions to the Sleepiq integration to enable control of the under bed and nightstand lights.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

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

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

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

@MartinHjelmare MartinHjelmare changed the title Added light control to Sleepiq integration Add light control to Sleepiq integration Jan 30, 2020
"""Icon to use in the frontend, if any."""
return ICON

async def async_turn_on(self):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
async def async_turn_on(self):
def turn_on(self):

This integration isn't async so you'll need to avoid doing I/O in async.

"""Instruct the light to turn on."""
self.turn_on()

async def async_turn_off(self):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
async def async_turn_off(self):
def turn_off(self):

add_entities(dev)


class SleepNumberLight(sleepiq.SleepIQLight):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The base class for this should be a Light

from homeassistant.components.light import Light


def __init__(self, sleepiq_data, bed_id, light):
"""Initialize the light."""
sleepiq.SleepIQLight.__init__(self, sleepiq_data, bed_id, light)
Copy link
Copy Markdown
Member

@bdraco bdraco Mar 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this and pass in what you need from setup_platform

if discovery_info is None:
return

data = sleepiq.DATA
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this will already be up to date in init so you shouldn't need to update again here.

I realize this is existing in init but we shouldn't use globals. You can fix this by storing the data in

hass.data[DOMAIN] = DATA

Then you can access it in setup_platform

DATA = hass.data[DOMAIN]

DATA should probably be renamed as well.

@@ -0,0 +1,127 @@
"""The tests for SleepIQ light platform."""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding tests!!!

Please remove the classes from the tests. A good example of how tests should be written is tests/components/wled

Copy link
Copy Markdown
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting this PR! Please see my comments inline above.

@stale
Copy link
Copy Markdown

stale Bot commented Apr 17, 2020

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale Bot added the stale label Apr 17, 2020
@stale stale Bot closed this Apr 25, 2020
@lock lock Bot locked and limited conversation to collaborators Apr 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants