Add here_travel_time

[eifinger]

Description:

This PR adds here_travel_time as a new component. It is based on the code of google_travel_time but uses the here api.

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#9648

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: here_travel_time
    app_id: "YOUR_APP_ID"
    app_code: "YOUR_APP_CODE"
    origin_latitude: "51.222975"
    origin_longitude: "9.267577"
    destination_latitude: "51.257430"
    destination_longitude: "9.335892"

Checklist:

• The code change is tested and works locally.
• Local tests pass with tox. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated in home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
• Untested files have been added to .coveragerc.

[eifinger]

Hi, thanks for taking the time and for giving me feedback. I'm on vacation this week and will get back to you next Monday.

[eifinger]

I implemented the requested changes. While doing so I looked at the architecture discussion you linked and the other travel_time integrations and I too see a need/benefit for a travel time component. Lots of duplicate code and different naming conventions for the same stuff.

I tried to follow the Waze implementation in order to have a common code base which can be refactored in the future.

I would prefer to do this refactoring (creating a travel_time component) in a follow up PR as this will need more discussions and will impact more than one integration.

[ljmerza]

You'll probably need to add tests to get this merged.

[eifinger]

How would these tests look like / what should be tested?
I could not find any tests for google_travel_time or waze_travel_time as a reference.

[MartinHjelmare]

Tests are not required, although appreciated, for integrations that communicates with devices or services.

[eifinger]

I am happy to provide them. Could you point me to a component test which you think might be a good starting point on how to do it properly?

And thank you for the code, reviews and comments. I am really learning a lot here!

[MartinHjelmare]

The manual alarm control panel has test that set up the component, via setup_component or async_setup_component, which sets up the platform, adds an entity and asserts the state of the entity by getting it from the core state machine. Update is forced by mocking time passed.

https://github.com/home-assistant/home-assistant/blob/78a5dc71ac34637d8216f759be04ccc79c9f7b56/tests/components/manual/test_alarm_control_panel.py#L203-L235

I/O, api calls, should be patched as needed.

[eifinger]

I added the requested changes. Will work on tests after the weekend

[eifinger]

I released the component as a custom_component under https://github.com/eifinger/here_travel_time to gather some user feedback and did indeed find some bugs and missing features. Until I implement them and the tests I will close this PR and later reopen it again.

[ljmerza]

if you add too many features and try to merge later on you might make it harder for yourself. It's always easier to merge something simple first just to get it in. Once the integration is there, it's easier to get updates merged to it in my experience.

…

On Thu, Jul 18, 2019 at 8:50 AM Kevin Eifinger ***@***.***> wrote: I released the component as a custom_component under https://github.com/eifinger/here_travel_time to gather some user feedback and did indeed find some bugs and missing features. Until I implement them and the tests I will close this PR and later reopen it again. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#24603>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA7FQBMPSBBW22ETZEY5G4TQABRKXANCNFSM4HZCM2CQ> .

[eifinger]

I got to the tests faster than I thought. Let me know what I can improve, thank you!

[eifinger]

Reopen after following https://gist.github.com/robertpainsi/2c42c15f1ce6dab03a0675348edd4e2c

[MartinHjelmare]

Just the final comment left to resolve. Then we can merge! 🎉

[MartinHjelmare]

Awesome!

[MartinHjelmare]

Thanks for the dedicated work!

Can be merged when build passes.

[eifinger]

Thank you for all the time and explanations! I learned a lot!

[eifinger]

@MartinHjelmare I thought the frontend would somehow parse/clean up the attribution message but it does not. The frontend shows the message as is with html tags. Is there anything I can do on the component side?

[eifinger]

I found another attribution source in Ottawa, CA.

"sourceAttribution": {
            "attribution": "With the support of <span class=\"company\"><a href=\"https://transit.api.here.com/r?appId=Mt1bOYh3m9uxE7r3wuUx&u=http://ottawa.ca/en/mobile-apps-and-open-data/open-data-terms-use\">OC Transpo</a></span>. All information is provided without warranty of any kind.",
            "supplier": [
                {
                    "title": "OC Transpo",
                    "href": "https://transit.api.here.com/r?appId=Mt1bOYh3m9uxE7r3wuUx&u=http://ottawa.ca/en/mobile-apps-and-open-data/open-data-terms-use",
                    "note": [
                        {
                            "type": "disclaimer",
                            "text": "<a href=\"https://transit.api.here.com/r?appId=Mt1bOYh3m9uxE7r3wuUx&u=http://ottawa.ca/en/mobile-apps-and-open-data/open-data-terms-use\">Includes content from OC Transpo: http://ottawa.ca/en/mobile-apps-and-open-data/open-data-terms-use</a>",
                            "href": "https://transit.api.here.com/r?appId=Mt1bOYh3m9uxE7r3wuUx&u=http://ottawa.ca/en/mobile-apps-and-open-data/open-data-terms-use",
                            "hrefText": "Includes content from OC Transpo: http://ottawa.ca/en/mobile-apps-and-open-data/open-data-terms-use"
                        }
                    ]
                }
            ]
        }

Following the link provided under href leads to a 404. So I figure this information is not curated correctly.

I will add another commit which builds the attribution (if present) according to the supplier/title attribute.