Skip to content

Adds docs for RainMachine component/hub#5251

Merged
arsaboo merged 11 commits into
home-assistant:nextfrom
bachya:rainmachine-1
May 3, 2018
Merged

Adds docs for RainMachine component/hub#5251
arsaboo merged 11 commits into
home-assistant:nextfrom
bachya:rainmachine-1

Conversation

@bachya
Copy link
Copy Markdown
Contributor

@bachya bachya commented Apr 26, 2018

Description:
Adds docs necessary for moving RainMachine to component/hub model.

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

Checklist:

  • Branch: Fixes, changes and adjustments should be created against current. New documentation for platforms/components and features should go to next.
  • The documentation follow the standards.

@bachya bachya mentioned this pull request Apr 26, 2018
Merged
7 tasks
MartinHjelmare
MartinHjelmare requested changes Apr 28, 2018
View reviewed changes
Comment thread source/_components/rainmachine.markdown Outdated
footer: true
logo: rainmachine.png
ha_category: Hub
ha_release: 0.68
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare Apr 28, 2018

Choose a reason for hiding this comment

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

0.69

MartinHjelmare
MartinHjelmare previously approved these changes Apr 30, 2018
View reviewed changes
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.

Thanks!

@bachya bachya mentioned this pull request May 1, 2018
Merged
3 tasks
MartinHjelmare
MartinHjelmare requested changes May 2, 2018
View reviewed changes
Comment thread source/_components/switch.rainmachine.markdown Outdated
required: false
type: boolean
default: true
zone_run_time:
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare May 2, 2018

Choose a reason for hiding this comment

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

Move this to the component page.

Copy link
Copy Markdown
Contributor Author

@bachya bachya May 2, 2018

Choose a reason for hiding this comment

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

Not understanding why? This follows a model similar to the Nest docs: https://www.home-assistant.io/components/sensor.nest/ – are you saying that configuration should only be mentioned in the component docs (and not mentioned at all in the platform docs)?

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare May 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, since that's where (the component) you change the configuration. It doesn't make sense to mention it on another page.

Copy link
Copy Markdown
Contributor Author

@bachya bachya May 2, 2018

Choose a reason for hiding this comment

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

Got it!

Comment thread source/_components/switch.rainmachine.markdown Outdated
zone_run_time: 240
```

{% configuration %}
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare May 2, 2018

Choose a reason for hiding this comment

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

Remove this.

Copy link
Copy Markdown
Contributor Author

@bachya bachya May 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

See above.

Comment thread source/_components/switch.rainmachine.markdown Outdated
</p>

The platform allows for either local (i.e., directly across the LAN) or remote (i.e., through RainMachine's cloud API) access; the route you choose will dictate what your configuration should look like.
## {% linkable_title Configuring the Platform %}
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare May 2, 2018

Choose a reason for hiding this comment

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

Move this to the component page.

Copy link
Copy Markdown
Contributor Author

@bachya bachya May 2, 2018

Choose a reason for hiding this comment

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

See above.

MartinHjelmare
MartinHjelmare reviewed May 2, 2018
View reviewed changes
Comment thread source/_components/rainmachine.markdown Outdated
# switch configuration options
```

{% configuration %}
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare May 2, 2018

Choose a reason for hiding this comment

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

Merge this with the configuration tag above. We should only have one of these sections with all options.

Copy link
Copy Markdown
Contributor Author

@bachya bachya May 2, 2018

Choose a reason for hiding this comment

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

My result feels awkward; pushing for you to review.

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare May 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This should still be merged with the config tag above ending on line 49.

Copy link
Copy Markdown
Contributor Author

@bachya bachya May 3, 2018

Choose a reason for hiding this comment

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

Sorry, I must be really dense. Will push shortly.

@MartinHjelmare
Copy link
Copy Markdown
Member

MartinHjelmare commented May 2, 2018

It's ok to have multiple configuration examples and the first one should be with only required items. My comment was regarding the configuration tag which is the spec for the configuration. There shall (:wink:) only be one of these.

@arsaboo
Copy link
Copy Markdown
Contributor

arsaboo commented May 3, 2018

@bachya It should be something like (so there is only one configuration entry):

{% configuration %}
ip_address:
  description: the IP address or hostname of your RainMachine unit
  required: optional
  type: string
password:
  description: your RainMachine password.
  required: true
  type: string
port:
  description: the TCP port used by your unit for the REST API
  required: false
  type: int
  default: 8080
ssl:
  description: whether communication with the local device should occur over HTTPS
  required: false
  type: boolean
  default: true
zone_run_time:
  description: the default number of seconds that a zone should run when turned on
  required: false
  type: int
  default: 600  
{% endconfiguration %}

MartinHjelmare
MartinHjelmare reviewed May 3, 2018
View reviewed changes
Comment thread source/_components/rainmachine.markdown Outdated
required: false
type: boolean
default: true
zone_run_time:
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare May 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I think you should add a key switches here that is a map and has zone_run_time as an item nested below it.

MartinHjelmare
MartinHjelmare approved these changes May 3, 2018
View reviewed changes
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!

@bachya
Copy link
Copy Markdown
Contributor Author

bachya commented May 3, 2018

WE DID IT. Thanks for your patience, @MartinHjelmare (and thank you, @arsaboo, for kicking me into action).

@arsaboo
Copy link
Copy Markdown
Contributor

arsaboo commented May 3, 2018

Merging it as the parent PRs are already merged

@arsaboo arsaboo merged commit d7fcb15 into home-assistant:next May 3, 2018
@bachya bachya deleted the rainmachine-1 branch May 3, 2018 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MartinHjelmare MartinHjelmare MartinHjelmare approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bachya @MartinHjelmare @arsaboo