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.

Sign up for GitHub

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

Jump to bottom

Add ConfigFlow for here_travel_time #22731

Merged
merged 7 commits into from May 22, 2022
Merged

Add ConfigFlow for here_travel_time #22731

merged 7 commits into from May 22, 2022

Conversation

eifinger
Copy link
Contributor

Proposed change

Adjust documentation to the new config flow. I try to align with waze and google as much as possible.

Type of change

  • Spelling, grammar or other readability improvements (current branch).
  • Adjusted missing or incorrect information in the current documentation (current branch).
  • Added documentation for a new integration I'm adding to Home Assistant (next branch).
  • Added documentation for a new feature I'm adding to Home Assistant (next branch).
  • Removed stale or deprecated documentation.

Additional information

Checklist

  • This PR uses the correct branch, based on one of the following:
    • I made a change to the existing documentation and used the current branch.
    • I made a change that is related to an upcoming version of Home Assistant and used the next branch.
  • The documentation follows the Home Assistant documentation standards.

@probot-home-assistant probot-home-assistant bot added has-parent This PR has a parent PR in a other repo next This PR goes into the next branch labels May 13, 2022
@eifinger eifinger changed the title Here travel time config flow Add ConfigFlow for here_travel_time May 13, 2022
@eifinger eifinger mentioned this pull request May 13, 2022
Merged
22 tasks
allenporter
allenporter reviewed May 21, 2022
View reviewed changes
source/_integrations/here_travel_time.markdown Outdated
- Transport
ha_iot_class: Cloud Polling
ha_release: '0.100'
ha_config_flow: true
ha_quality_scale: silver
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I don't see that the manifest currently has this set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opened a PR for it and think it even is gold: home-assistant/core#72308

allenporter
allenporter reviewed May 21, 2022
View reviewed changes
source/_integrations/here_travel_time.markdown Outdated

HERE offers a Freemium Plan which includes 250,000 free Transactions per month. For the Routing API, one transaction equals one request with one starting point (no multi stop). More information can be found [here](https://developer.here.com/faqs#payment-subscription)
HERE offers a Freemium Plan which includes 250,000 free Transactions per month. For the Routing API, one transaction equals one request with one starting point (no multi stop). More information can be found [here](https://knowledge.here.com/csm_kb?id=public_kb_csm_details&number=KB0016433)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it is worth letting the user how much of this quota would be consumed by home assistant by default? Doesn't have to be this PR since this is unrelated cleanup, but I was just thinking through what i might wonder reading this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added information on how much the free requests can be used with the standard config.

allenporter
allenporter reviewed May 21, 2022
View reviewed changes
source/_integrations/here_travel_time.markdown Outdated
### Migrate from app_code to api_key

HERE has changed its authentication mechanism. It is no longer possible to use `app_id` and `app_code`. Existing users have to follow the [migration guide](https://developer.here.com/documentation/authentication/dev_guide/topics/api-key-credentials.html) in order to retrieve the now needed `api_key`.
By default HERE will deactivate your account if you exceed the free Transaction limit for the month. You can add payment details to re-enable your account as described [here](https://knowledge.here.com/csm_kb?id=public_kb_csm_details&number=KB0016434)

## Configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

My impression is now this includes the template it gives standard instructions for how to navigate to config flow and options, which is great -- though now it's not really explaining how the integration itself works as much. Maybe before jumping into dynamic configuration it's worth explaining the concepts of setting the origin and destination, then it goes next into the more advanced dynamic config with that baseline understanding set?

It is fairly straight forward from the config flow, however, so very open to the idea this isn't needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied a part of the waze documentation to be more clear.

@frenck frenck added this to To Do in New documentation via automation May 22, 2022
@frenck frenck added the parent-merged The parent PR has been merged already label May 22, 2022
allenporter
allenporter reviewed May 22, 2022
View reviewed changes
Copy link
Contributor

@allenporter allenporter left a comment

Choose a reason for hiding this comment

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

Looks good to me for the config flow changes.

Can re-review when the quality scale change lands, or you can separate that out to a follow up.

@eifinger
Copy link
Contributor Author

@allenporter I removed the quality scale so we can finish this up quicker

New documentation automation moved this from To Do to Awaiting Parent PR May 22, 2022
@allenporter allenporter merged commit 55aa606 into home-assistant:next May 22, 2022
New documentation automation moved this from Awaiting Parent PR to Done May 22, 2022
@probot-home-assistant probot-home-assistant bot removed the parent-merged The parent PR has been merged already label May 22, 2022
@github-actions github-actions bot locked and limited conversation to collaborators May 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@allenporter allenporter allenporter approved these changes

Assignees
No one assigned
Labels
cla-signed has-parent This PR has a parent PR in a other repo next This PR goes into the next branch
Projects
No open projects
New documentation
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@eifinger @allenporter @frenck @homeassistant