Skip to content
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

Add documentation for ADS component #3777

Closed
wants to merge 49 commits into from
Closed

Conversation

stlehmann
Copy link
Contributor

Description:

Add documentation for ADS component.

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

Stefan Lehmann and others added 30 commits October 18, 2017 18:07
* remove ssl and port in configuration.yaml

Make clear to remove SSL and port in configuration.yaml to avoid 502 errors

* Update nginx_proxy.markdown

* Minor change
* Update Arlo Control Panel with away mode variable

Just a simple change to add a description about adding an away mode to the Arlo Control Panel.

* Remove blank line
* Update recorder.markdown

* It's a platform and not a component
Got distracted and typed before/after instead of below/above
Updating the navigation links for Z-Wave
* added note: type names differ from entity names

* Minor changes
Add info in connection problem section
rdbahm and others added 9 commits October 25, 2017 07:25
* Fixing Typo that was mentioned in home-assistant#3764

* Cleaning up some other things in glossary

As I was making the other commit (home-assistant#3766) I noticed some other things in the glossary that could be changed.

One of the main things is that each example read differently so there was a lot of jumping between how examples were written, I've made them a bit more unified.

I also added a bit more explanatory text and added some links as suggested in home-assistant#3764.

I made this a seperate pull because I figured it might be a bit more opinionated than the typo error.

* Missed discovery.

* Add quotes
Removed info about versions before 0.4x
Added link to the Hass.io Z-Wave docs, and a note that the pre-reqs don't apply to it.
@stlehmann stlehmann changed the title Ads Add documentation for ADS component Oct 25, 2017
@fabaff fabaff added Hacktoberfest An PR on this issue (or the PR itself) is eligible towards Hacktoberfest! new-integration This PR adds documentation for a new Home Assistant integration labels Oct 26, 2017
@fabaff fabaff removed the Hacktoberfest An PR on this issue (or the PR itself) is eligible towards Hacktoberfest! label Nov 6, 2017
Copy link
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

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

@MrLeeH I've reviewed your submission and left some suggestions to improve the documentation. Could you please take a look?

footer: true
logo: home-assistant.png
ha_category: Hub
ha_release: 0.55
Copy link
Member

Choose a reason for hiding this comment

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

You might want to update the version number

comments: false
sharing: true
footer: true
logo: home-assistant.png
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add the TwinCAT or Backhoff logo?


Configuration parameters:

- **device** (*Required*): The AMS NetId that identifies the device.
Copy link
Member

Choose a reason for hiding this comment

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

Please use the configuration tags for listing configuration variables. For more information:
https://home-assistant.io/developers/documentation/create_page/#configuration


Service parameters:

- **adsvar**: name of the variable on the ADS device. To access global variables
Copy link
Member

Choose a reason for hiding this comment

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

Please use the configuration tags for listing configuration variables. For more information:
https://home-assistant.io/developers/documentation/create_page/#configuration

```

Configuration variables:
- **adsvar** (*Required*): The name of the variable which you want to access on
Copy link
Member

Choose a reason for hiding this comment

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

Please use the configuration tags for listing configuration variables. For more information:
https://home-assistant.io/developers/documentation/create_page/#configuration


Configuration variables:

- **adsvar** (*Required*): The name of the variable which you want to access on
Copy link
Member

Choose a reason for hiding this comment

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

Please use the configuration tags for listing configuration variables. For more information:
https://home-assistant.io/developers/documentation/create_page/#configuration

- **device_class** (*Optional*): The [type/class](/components/binary_sensor/) of the sensor to set the icon in the frontend.
- **use_notify** (*Optional*): Enable device notifications. Default: yes.
- **poll_interval** (*Optional*): If device notifications are disabled polling
is used. This parameter defines the pollint interval in milliseconds. Defaults
Copy link
Member

Choose a reason for hiding this comment

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

Typo: pollint -> polling

is used. This parameter defines the pollint interval in milliseconds. Defaults
to 1000.

If *device notifications* are enabled the *ADS* device will push a change
Copy link
Member

Choose a reason for hiding this comment

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

Missing comma: are enabled, the
Missing comma: Otherwise, the value

- **factor** (*Optional*): Use a factor that divides the stored value before
displaying in Home Assistant. Default: 1.

If *device notifications* are enabled the *ADS* device will push a change
Copy link
Member

Choose a reason for hiding this comment

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

Missing comma: are enabled, the
Missing comma: Otherwise, the value

If *device notifications* are enabled the *ADS* device will push a change
directly to Home Assistant. Otherwise the value will be polled regularly.

The *factor* can be used to implement fixed decimals. E.g. set *factor* to 100
Copy link
Member

Choose a reason for hiding this comment

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

Missing comma: E.g., set

@stlehmann
Copy link
Contributor Author

@frenck Thank you for your review. I updated the version number to 0.59 and fixed the typos and commas. Also I added the components tags to all platforms. Concerning the logo I think I might need to ask the company for permission to use their logo.

@frenck
Copy link
Member

frenck commented Nov 20, 2017

@MrLeeH Not sure what you have done at this point.
But there are a lot of merge conflicts now and there are a lot of commits in this PR that should not be there...

Could you please take a look?

@stlehmann
Copy link
Contributor Author

@frenck Damn, after I made the changes you suggested I realized I was on the wrong branch. So I merged it in the ads branch. There must have something gone wrong.

@stlehmann
Copy link
Contributor Author

I don't know where those conflicts are coming from. Maybe I just start a new PR?

@stlehmann stlehmann closed this Nov 20, 2017
@stlehmann stlehmann deleted the ads branch November 20, 2017 19:12
@stlehmann
Copy link
Contributor Author

stlehmann commented Nov 20, 2017

There are still some changes in the component going on. I will wait with the PR until all details have been clarified. Thank you for the review so far. I implemented them to the best of my knowledge and certain.

@stlehmann stlehmann mentioned this pull request Dec 1, 2017
2 tasks
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.