Skip to content

Documentation of the Xiaomi Philips Light component#3246

Merged
balloob merged 5 commits into
home-assistant:nextfrom
syssi:feature/xiaomi_philipslight
Sep 9, 2017
Merged

Documentation of the Xiaomi Philips Light component#3246
balloob merged 5 commits into
home-assistant:nextfrom
syssi:feature/xiaomi_philipslight

Conversation

@syssi
Copy link
Copy Markdown
Member

@syssi syssi commented Aug 26, 2017

Description:

Documentation of the Xiaomi Philips Light component.

Pull request in home-assistant: home-assistant/core#9087

@syssi
Copy link
Copy Markdown
Member Author

syssi commented Aug 26, 2017

+New-component/platform

@balloob
Copy link
Copy Markdown
Member

balloob commented Aug 26, 2017

Please add ha_version: 0.53 to your page metadata.

Did you base your page on another page? (and thus we have another page missing a ha_version tag)

@syssi
Copy link
Copy Markdown
Member Author

syssi commented Aug 26, 2017

I just removed the ha_version because I was unsure. The page is based on vacuum.xiaomi.markdown.


Current supported features are `on`, `off`, `set_cct` (colortemp) , `set_bright` (brightness).

## {% linkable_title Getting started %}
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.

I moved the instructions from the vacuum platform to the component. So a link to https://home-assistant.io/components/xiaomi/#retrieving-the-access-token would be sufficient.

Copy link
Copy Markdown
Member Author

@syssi syssi Aug 26, 2017

Choose a reason for hiding this comment

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

The new location of the token instructions isn't appropriate. The page "https://home-assistant.io/components/xiaomi/" belongs to the xiaomi gateway. A device with another protocol and another security mechanism (key != token). We mixed up two independent platforms now.

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.

Same app, same name... Sure, we can improve the location but I don't see an issue here as the intro make it clear that it's for platforms.

Maintaining three times the same instructions which will be outdated probably soon and are extensive doesn't make sense and will lead to frustrated users. We saw that with the Raspberry platforms in the past.

name: Xiaomi Philips Smart LED Ceiling Lamp
host: 192.168.130.68
token: your-ceiling-lamp-token-here
```
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.

We try to keep all configuration samples minimal. Thus, no optional requirement in the default sample. This helps user to get started quickly by copy-&-paste the sample without worrying about optional parameters which they most likely not need. If required, insert a full configuration sample later that covers special setups or alike.

@fabaff fabaff added the new-integration This PR adds documentation for a new Home Assistant integration label Aug 26, 2017
@syssi
Copy link
Copy Markdown
Member Author

syssi commented Aug 26, 2017

Can you move section "Retrieving the Access Token" to a better place or make a suggestion?

@syssi
Copy link
Copy Markdown
Member Author

syssi commented Aug 28, 2017

The new place is okay for me. I will suggest a change again if the community users will mix up the xiaomi zigbee gateway (keys) and wifi (tokens) too often which is mentioned on the same page now.

@syssi
Copy link
Copy Markdown
Member Author

syssi commented Aug 28, 2017

@balloob balloob added this to the 0.53 milestone Sep 9, 2017
@balloob balloob merged commit 045df29 into home-assistant:next Sep 9, 2017
balloob pushed a commit that referenced this pull request Sep 9, 2017
* Documentation for Xiaomi Philips Light component added.

* HA version added.

* Syntax error fixed.

* Section "Retrieving the Access Token" has a central place now.

* Spelling & grammar fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-integration This PR adds documentation for a new Home Assistant integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants