Allow resetting time in google_travel_time

[eifinger]

Proposed change

Allow resetting already defined departure or arrival times.
Equivalent to #88253

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• The code has been formatted using Black (black --fast homeassistant tests)
• Tests have been added to verify that the new code works.

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

• Documentation added/updated for www.home-assistant.io

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

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[balloob]

Marked as draft until frontend is done.

[eifinger]

@epenet you reviewed the PR for waze. Could you also have a look on this one since it is basically the same

[epenet]

I'm not sure your PR works as intended. I think you may still need a default=None in some places.

Basically, when a blank string is selected in the frontend, the data passed back will not contain the key at all, and therefore will not update the options.
I think you may need to set default=None or default="" where you want the key to be present in the resulting options.

Please adjust the test to ensure other options can also still be cleared.

[eifinger]

I'm not sure your PR works as intended. I think you may still need a default=None in some places.

Basically, when a blank string is selected in the frontend, the data passed back will not contain the key at all, and therefore will not update the options. I think you may need to set default=None or default="" where you want the key to be present in the resulting options.

Please adjust the test to ensure other options can also still be cleared.

The only thing which can be cleared is CONF_TIME and that works. I checked that manually and the tests do reflect that as well:

This line defines the test:
Given the time type is CONF_ARRIVAL_TIME and is set to test
this line:
and the user_input does not contain CONF_TIME (because it was emptied)
this line:
then neither CONF_ARRIVAL_TIME nor CONF_DEPARTURE_TIME is configured

[eifinger]

The other options can be emptied by selecting the value without any description. This looks horrible and needs to be addressed but not in this PR

[epenet]

But what about CONF_LANGUAGE, CONF_AVOID, CONF_TRAFFIC_MODEL, CONF_TRANSIT_MODE, CONF_TRANSIT_ROUTING_PREFERENCE?

Since you dropped the default, I think they can't be reset.

[eifinger]

You are right. Its no problem for entries which define less than 7 possible values. Because then radio buttons are used and I can select the one without any description aka None value.
As soon as there are 7 or more possible values like in CONF_LANGUAGE CONF_TRANSIT_MODE a combobox/dropdown(?) is used and reset does not work.

[epenet]

You are right. Its no problem for entries which define less than 7 possible values. Because then radio buttons are used and I can select the one without any description aka None value. As soon as there are 7 or more possible values like in CONF_LANGUAGE CONF_TRANSIT_MODE a combobox/dropdown(?) is used and reset does not work.

I suggest that you take a look at the discussion on #90412
An "easy" way to solve that is to set default="" or default=None in the schema, so that if frontend doesn't pass the key it gets set to the default (None or "").
That's what I started with there - but then it was improved with selectors, translations and a sentinel.

I suggest that you do something similar here.

[epenet]

#90412 has been accepted and merged.
I suggest that you review it and adjust this one accordinly, with selectors, translations and a sentinel.
Maybe you should use selectors also for waze.

[home-assistant]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

[edenhaus]

Short update: Frontend will add support for not selecting any option, and therefore, we can remove all NONE_SENTINEL.
We should wait for the frontend before merging this PR

[edenhaus]

Thanks @eifinger 👍

Changes addressed