Skip to content

Update the MQTT fan documentation #1345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Update the MQTT fan documentation #1345

wants to merge 1 commit into from

Conversation

wingrunr21
Copy link

This is a documentation update for the MQTT Fan component. When the original component was merged (via home-assistant/core#3095) incomplete/incorrect documentation was added at the same time. This brings the docs inline with the component.

@mention-bot
Copy link

@wingrunr21, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fabaff to be a potential reviewer.

@robbiet480 robbiet480 added the Hacktoberfest An PR on this issue (or the PR itself) is eligible towards Hacktoberfest! label Oct 27, 2016
@Landrash Landrash added the new-feature This PR adds documentation for a new Home Assistant feature to an existing integration label Oct 28, 2016
Copy link
Contributor

@Landrash Landrash left a comment

Choose a reason for hiding this comment

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

Good addition but one change that would need to be made.


Optimistic mode can be forced, even if state topic is available. Try to enable it, if experiencing incorrect fan operation.
Optimistic mode can be forced even if a `state_topic` is available. Try to enable it if you are experiencing incorrect fan operation.

To enable MQTT fans in your installation, add the following to your `configuration.yaml` file:

```yaml
# Example configuration.yml entry
fan:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a great addition but the example code should always try to be minimized.
Feel free to have a fully fleshed out example also but avoid having the Optional variables in the minimized one.

Copy link
Author

Choose a reason for hiding this comment

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

So happy to make the change but the way the original author defined the component would mean the minimal version would ONLY be the command topic. Would you be ok with some kind of happy medium?

@robbiet480 robbiet480 removed the Hacktoberfest An PR on this issue (or the PR itself) is eligible towards Hacktoberfest! label Nov 1, 2016
@fabaff
Copy link
Member

fabaff commented Nov 5, 2016

With home-assistant/core#4134 was the documentation updated without taking that PR into account.

Duplicated work. I will fix my mess.

@fabaff fabaff mentioned this pull request Nov 5, 2016
@fabaff fabaff closed this Nov 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature This PR adds documentation for a new Home Assistant feature to an existing integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants