Skip to content

Add info about new yeelight start_flow service#8166

Merged
rytilahti merged 1 commit into
home-assistant:nextfrom
zewelor:add_info_about_new_start_flow_service_and_config_options_to_yeelight
Jan 24, 2019
Merged

Add info about new yeelight start_flow service#8166
rytilahti merged 1 commit into
home-assistant:nextfrom
zewelor:add_info_about_new_start_flow_service_and_config_options_to_yeelight

Conversation

@zewelor
Copy link
Copy Markdown
Contributor

@zewelor zewelor commented Jan 14, 2019

Description:

Pull request in home-assistant (if applicable): home-assistant/core#20107

Checklist:

  • Branch: next is for changes and new documentation that will go public with the next home-assistant release. Fixes, changes and adjustments for the current release should be created against current.
  • The documentation follows the standards.

@ghost ghost added the to-do label Jan 14, 2019
@frenck frenck added new-feature This PR adds documentation for a new Home Assistant feature to an existing integration ready-for-review This PR needs to be reviewed next This PR goes into the next branch has-parent This PR has a parent PR in another repo and removed to-do labels Jan 14, 2019
Possible transitions are `RGBTransition`, `HSVTransition`, `TemperatureTransition`, `SleepTransition`


More info about values, transitions: https://gitlab.com/stavros/python-yeelight/blob/master/yeelight/flow.py
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 think it would be nicer to link to the documentation here: https://yeelight.readthedocs.io/en/stable/flow.html . Also, considering there are not so many options, maybe simply writing them out here?

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.

I think that would require to copy this whole section: https://yeelight.readthedocs.io/en/stable/yeelight.html#flow-objects

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.

Fair enough! I think linking the documentation instead of the source code is enough (the examples will make it clear how to use them) 👍

required: false
type: string
custom_effects:
description: "List of custom effects to add. Check examples below"
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.

No need for quotes.


| Service data attribute | Optional | Description |
|---------------------------|----------|---------------------------------------------------------------------------------------------|
| `entity_id` | yes | Only act on a specific yeelight. Else targets all. |
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 think this should be mandatory, see home-assistant/core#19006

@zewelor
Copy link
Copy Markdown
Contributor Author

zewelor commented Jan 17, 2019

Thanks for Review. Updated with comments addressed.

@rytilahti rytilahti changed the title Add info about new yeelight star flow service Add info about new yeelight start_flow service Jan 19, 2019
@zewelor
Copy link
Copy Markdown
Contributor Author

zewelor commented Jan 24, 2019

@rytilahti Do I need to change anything more here ?

Copy link
Copy Markdown
Member

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

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

I added a couple of things which need to be changed, then it's all good, thanks again for the effort :-)


This example shows how you can add your custom effects in your configuration.

Possible transitions are `RGBTransition`, `HSVTransition`, `TemperatureTransition`, `SleepTransition`
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.

Add dot at the end.

required: false
type: string
custom_effects:
description: List of custom effects to add. Check examples below
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.

Add dots at the end of each description, then it'd be consistent with other docs.

| Service data attribute | Optional | Description |
|---------------------------|----------|---------------------------------------------------------------------------------------------|
| `entity_id` | yes | Only act on a specific yeelight. Else targets all. |
| `entity_id` | no | Only act on a specific yeelights. |
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.

'specific lights.'


| Service data attribute | Optional | Description |
|---------------------------|----------|---------------------------------------------------------------------------------------------|
| `entity_id` | no | Only act on a specific yeelights. |
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.

'specific lights.'

|---------------------------|----------|---------------------------------------------------------------------------------------------|
| `entity_id` | no | Only act on a specific yeelights. |
| `count` | yes | The number of times to run this flow (0 to run forever). |
| `transitions` | no | Array of transitions, for desired effect. For more info check [example](#custom-effects) |
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.

remove ', for desired effect', I think it's superfluous.

Copy link
Copy Markdown
Member

@rytilahti rytilahti Jan 24, 2019

Choose a reason for hiding this comment

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

Also, 'See [examples below] ..'

Possible transitions are `RGBTransition`, `HSVTransition`, `TemperatureTransition`, `SleepTransition`


More info about values, transitions in [library docs](https://yeelight.readthedocs.io/en/stable/flow.html)
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.

More info about transitions and their expected parameters can be found in [python-yeelight documentation].

@zewelor
Copy link
Copy Markdown
Contributor Author

zewelor commented Jan 24, 2019

Thanks, fixed everything from comments.

Copy link
Copy Markdown
Member

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

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

Thanks, let's merge this! 🥇

@rytilahti rytilahti merged commit dedd3ca into home-assistant:next Jan 24, 2019
@ghost ghost removed the ready-for-review This PR needs to be reviewed label Jan 24, 2019
@zewelor zewelor deleted the add_info_about_new_start_flow_service_and_config_options_to_yeelight branch February 1, 2019 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

has-parent This PR has a parent PR in another repo new-feature This PR adds documentation for a new Home Assistant feature to an existing integration next This PR goes into the next branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants