Skip to content

Greenwave Reality (TCP Connected) Lighting Component#11282

Merged
pvizeli merged 9 commits into
home-assistant:devfrom
dfiel:dev
Dec 24, 2017
Merged

Greenwave Reality (TCP Connected) Lighting Component#11282
pvizeli merged 9 commits into
home-assistant:devfrom
dfiel:dev

Conversation

@dfiel
Copy link
Copy Markdown
Contributor

@dfiel dfiel commented Dec 23, 2017

Created component to add support for End of Life Greenwave Reality (TCP Connected) gateway and lightbulbs

Description:

Related issue (if applicable): Added support for new platform

Example entry for configuration.yaml (if applicable):

light:
  - platform: greenwave
    host: 192.168.1.97
    version: 3

Checklist:

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

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.

"""Instruct the light to turn on."""
import greenwavereality as greenwave
temp_brightness = int((kwargs.get(ATTR_BRIGHTNESS, 255) / 255) * 100)
greenwave.set_brightness(self._host, self._did, temp_brightness, self.token)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (84 > 79 characters)

Line too long
/ 255) * 100)
greenwave.set_brightness(self._host, self._did,
temp_brightness, self.token)
greenwave.turn_on(self._host, self._did, self.token)
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.

You are sure that is also need to send a brightness and after that also a on?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, both are required.

Copy link
Copy Markdown
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

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

Looks good

add_devices(TcpLights(device, host, token) for device in doc)


class TcpLights(Light):
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.

GreenwaveLight or like this. Other suggest that the class represent more than one light and run into name conflicts

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That was left over from when this was named TCP Lighting, changing it now.

import greenwavereality as greenwave
import os
host = config.get(CONF_HOST)
tokenfile = hass.config.path('greenwave.token')
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.

use .greenwave so that should be not editable for users with a .

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changing that now

else:
token = None
doc = greenwave.grab_xml(host, token)
add_devices(TcpLights(device, host, token) for device in doc)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

undefined name 'TcpLights'

@pvizeli pvizeli merged commit 8683d75 into home-assistant:dev Dec 24, 2017
@balloob balloob mentioned this pull request Jan 11, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Mar 30, 2018
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.

5 participants