Skip to content

Add auto discovery for nanoleaf aurora lights#14301

Merged
MartinHjelmare merged 5 commits intohome-assistant:devfrom
Oro:light-aurora-discovery
May 20, 2018
Merged

Add auto discovery for nanoleaf aurora lights#14301
MartinHjelmare merged 5 commits intohome-assistant:devfrom
Oro:light-aurora-discovery

Conversation

@Oro
Copy link
Copy Markdown
Contributor

@Oro Oro commented May 5, 2018

Description:

Added auto discovery to the nanoleaf aurora platform

Related issue (if applicable): none

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

Example entry for configuration.yaml (if applicable):

discovery:

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:

@fabaff fabaff changed the title auto discovery added for nanoleaf aurora lights Add auto discovery for nanoleaf aurora lights May 11, 2018
if not token:
token = nanoleaf.setup.generate_auth_token(host)
if not token:
_LOGGER.error("""Could not generate the auth token, did you press
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 one pair of double quotes around the log string. Break the line by ending with a double quote, and then continue with a new double quote on the next line.

return

add_devices([AuroraLight(aurora_light)], True)
else:
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 else is not needed since there is a return above.


add_devices([AuroraLight(aurora_light)], True)
else:
aurora_light.hass_name = name
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare May 20, 2018

Choose a reason for hiding this comment

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

Don't put the name as an instance attribute on the aurora light instance. Pass the name as an additional parameter when creating the entity instance.

if conf.get(host, {}).get('token'):
token = conf[host]['token']
elif config is not None:
host = config.get(CONF_HOST)
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.

Don't use dict.get(key) on required keys or keys that have a default value. Use dict[key].

conf = load_json(hass.config.path(CONFIG_FILE))
if conf.get(host, {}).get('token'):
token = conf[host]['token']
elif config is not None:
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.

This will never be None. Setup via discovery will pass an empty dict for config. Change the check to else:.

token = nanoleaf.setup.generate_auth_token(host)
if not token:
_LOGGER.error("Could not generate the auth token, did you press"
" and hold the power button on %s for 5-7 seconds?", name)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent

token = nanoleaf.setup.generate_auth_token(host)
if not token:
_LOGGER.error("Could not generate the auth token, did you press "
"and hold the power button on %s for 5-7 seconds?", name)
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 (83 > 79 characters)

@Oro
Copy link
Copy Markdown
Contributor Author

Oro commented May 20, 2018

(hopefully) all done, thank you for the review!

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare 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. Last comment.


DATA_NANOLEAF_AURORA = 'nanoleaf_aurora'

CONFIG_FILE = 'nanoleaf_aurora.conf'
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare May 20, 2018

Choose a reason for hiding this comment

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

If the config file is not supposed to be edited by users, we want it to be hidden, ie start with a dot.

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Great!

@MartinHjelmare MartinHjelmare merged commit c8ad9c4 into home-assistant:dev May 20, 2018
@Oro
Copy link
Copy Markdown
Contributor Author

Oro commented May 20, 2018

I definitely borked up the pull request on the doc, sorry about that. Should not do something like that late at night; I'll most likely send another pull request tomorrow after I've gotten some sleep...

@Oro Oro deleted the light-aurora-discovery branch May 21, 2018 09:30
@balloob balloob mentioned this pull request Jun 8, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Sep 5, 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.

4 participants