Skip to content

Conversation

@eifinger
Copy link
Contributor

Proposed change

Fixes #145660

Having a comma in the address leads to vol.MultipleInvalid. Catching vol.Invalid as the base type fixes the issue.

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

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
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.

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

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.

To help with the load of incoming pull requests:

@eifinger eifinger changed the title Support addresses with comma Support addresses with comma in google_travel_time May 26, 2025
@eifinger eifinger force-pushed the google_travel_time_setup_address_fail branch from 51e8b68 to c458527 Compare May 27, 2025 15:46
Comment on lines +11 to +21
@pytest.mark.parametrize(
("location", "expected_result"),
[
(
"12.34,56.78",
Waypoint(
location=Location(
lat_lng=latlng_pb2.LatLng(
latitude=12.34,
longitude=56.78,
)
Copy link
Member

Choose a reason for hiding this comment

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

would this make sense to test at the places where we use this to avoid for example an incorrect schema for the config flow where the same can happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed on Discord I think by testing convert_to_waypoint to make sure it does what it is supposed to do, we can trust that its use in the ConfigFlow does also work. Testing the config flow with different set of data would also be a possibility albeit one which needs a bigger set of changes.

@eifinger eifinger force-pushed the google_travel_time_setup_address_fail branch from c458527 to 6558f14 Compare May 27, 2025 16:48
@joostlek joostlek added this to the 2025.6.0 milestone May 27, 2025
@joostlek joostlek merged commit 07fd1f9 into home-assistant:dev May 27, 2025
34 checks passed
bramkragten pushed a commit that referenced this pull request May 27, 2025
@github-actions github-actions bot locked and limited conversation to collaborators May 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

google_travel_time cannot be set up with an address

3 participants