Skip to content

adding device_tracker.tomato https params#4389

Merged
frenck merged 8 commits into
home-assistant:nextfrom
GregoryDosh:next
Jan 24, 2018
Merged

adding device_tracker.tomato https params#4389
frenck merged 8 commits into
home-assistant:nextfrom
GregoryDosh:next

Conversation

@GregoryDosh
Copy link
Copy Markdown
Contributor

@GregoryDosh GregoryDosh commented Jan 10, 2018

Description:
Added new configuration parameters for https support in the device_tracker.tomato component.

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

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.

@todschmidt
Copy link
Copy Markdown
Contributor

I'd suggest adding the default values for the optional parameters (port: 80, ssl: false, verify; true)

@GregoryDosh
Copy link
Copy Markdown
Contributor Author

Sounds good, I'll update the documents & code to match.

- platform: tomato
host: YOUR_ROUTER_IP_ADDRESS
host: YOUR_ROUTER_IP_ADDRESS_OR_HOSTNAME
port: YOUR_ROUTER_PORT
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.

Optional configuration variables should not be in the examples.
We are trying to keep the examples as minimal as possible.

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.

Sounds good!

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.

It might be a good thing to call out that ^^ in the standards documentation since I don't think it mentions optional variables not being part of the examples.

@frenck frenck added the new-feature This PR adds documentation for a new Home Assistant feature to an existing integration label Jan 11, 2018
frenck
frenck previously approved these changes Jan 14, 2018
@frenck
Copy link
Copy Markdown
Member

frenck commented Jan 14, 2018

Thanks for cleaning up @GregoryDosh! 🏅

Can be merged as soon as the parent PR gets merged.

description: "The port number of your router, e.g. `443`."
required: false
type: int
default: 80/443 (ssl disabled/ssl enabled)
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.

@frenck How should the default port param be handled in the case of ssl/non ssl being chosen? Not sure how this should be represented to the users.

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.

Maybe reference it as "auto detected"?

Of "80/443 (automatically detected)"

@frenck frenck self-assigned this Jan 24, 2018
@frenck
Copy link
Copy Markdown
Member

frenck commented Jan 24, 2018

Parent PR got merged, I will go ahead and merged the docs in as well.

@frenck frenck merged commit 48fbcd8 into home-assistant:next Jan 24, 2018
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.

3 participants