Skip to content

Waze Travel Time: optional inclusive/exclusive filters#14000

Merged
fabaff merged 6 commits intohome-assistant:devfrom
mario-tux:dev
May 8, 2018
Merged

Waze Travel Time: optional inclusive/exclusive filters#14000
fabaff merged 6 commits intohome-assistant:devfrom
mario-tux:dev

Conversation

@mario-tux
Copy link
Copy Markdown
Contributor

Added optional inc_filter and `excl_filter' params that allow to refine the reported routes: the first is not always the best/desired. A simple case-insensitive filtering (no regular expression) is used.

Description:

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#5208

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: waze_travel_time
    origin: Montréal, QC
    destination: Québec, QC
    region: 'US'
    inc_filter: 'High'
    excl_filter: 'local'

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

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

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

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

Added optional `inc_filter` and `excl_filter' params that allow to refine the reported routes: the first is not always the best/desired. A simple case-insensitive filtering (no regular expression) is used.
@homeassistant

This comment has been minimized.

if self._inc_filter is not None:
results = {k: v for k, v in results.items() if self._inc_filter.lower() in k.lower()}
if self._excl_filter is not None:
results = {k: v for k, v in results.items() if self._excl_filter.lower() not in k.lower()}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (106 > 79 characters)

self._origin, self._destination, self._region, None)
results = params.calc_all_routes_info()
if self._inc_filter is not None:
results = {k: v for k, v in results.items() if self._inc_filter.lower() in k.lower()}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (101 > 79 characters)


try:
waze_data = WazeRouteData(origin, destination, region)
waze_data = WazeRouteData(origin, destination, region, inc_filter, excl_filter)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (87 > 79 characters)

self._inc_filter.lower() in k.lower()}
if self._excl_filter is not None:
results = {k: v for k, v in results.items() if
self._excl_filter.lower() not in k.lower()}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent

results = {k: v for k, v in results.items() if
self._inc_filter.lower() in k.lower()}
if self._excl_filter is not None:
results = {k: v for k, v in results.items() if
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

trailing whitespace

results = params.calc_all_routes_info()
if self._inc_filter is not None:
results = {k: v for k, v in results.items() if
self._inc_filter.lower() in k.lower()}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent

self._origin, self._destination, self._region, None)
results = params.calc_all_routes_info()
if self._inc_filter is not None:
results = {k: v for k, v in results.items() if
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

trailing whitespace

try:
waze_data = WazeRouteData(origin, destination, region)
waze_data = WazeRouteData(origin, destination, region,
inc_filter, excl_filter)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent


try:
waze_data = WazeRouteData(origin, destination, region)
waze_data = WazeRouteData(origin, destination, region,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

trailing whitespace

@fabaff
Copy link
Copy Markdown
Member

fabaff commented May 1, 2018

@Myrddyn1, can you please comment on this? Thanks

@mario-tux
Copy link
Copy Markdown
Contributor Author

Maybe the example of usage of the filters proposed in the documentation doesn't make so sense.
I use such filter in my daily use because Waze, for my daily trip for work, regularly proposes three paths. The one I prefer makes use of the high-way but it is not always on the top of the list. The other two paths make use of secondary roads. With such filters (even with just one) I can influence the selection with clear rules.

@Myrddyn1
Copy link
Copy Markdown

Myrddyn1 commented May 7, 2018

Hi,
This is a good feature :)
So, you would keep the exact same route whatever happens on the way? (accident for instance)
Would you consider a better route if the usual route is like XX min longer than the best one ?

@mario-tux
Copy link
Copy Markdown
Contributor Author

mario-tux commented May 7, 2018

@Myrddyn1
These are just filters on the synthetic names automatically reported by Waze: they look to report the main roads used in the path.
I guess, in case of accident, it would continue to report the main road name (if still used).

If I use an "inclusive filter" with the high-way name, he would select it even if longer than the others (not using the high-way). If I use an "exclusive filter" with a not-preferred road name, he would remove any path reporting such name event if it is shorter. Such filters are a simple way to apply personal preferences and, as in my case, to prevent continuing variation of the selected path (few minutes of difference among the alternatives).

IMHO such filters should not annoy anyone who don't want to use them and they could even used in combination with other future criteria. They just cut-off the reported list.

@Myrddyn1
Copy link
Copy Markdown

Myrddyn1 commented May 7, 2018

@diraimondo
I understand your feature.
I was just wondering if you would reconsider your route by setting a threshold in minutes as a parameter.
Thus, if your preferred route presents an unusual delay, and its delay is (let's say) 15 min higher than the second route option, the plugin will give you the second option.

Anyway, with or without that threshold, I agree with your feature :)

Copy link
Copy Markdown
Member

@fabaff fabaff left a comment

Choose a reason for hiding this comment

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

Thanks 🐦

@fabaff fabaff merged commit 6231394 into home-assistant:dev May 8, 2018
@balloob balloob mentioned this pull request May 28, 2018
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
…t#14000)

* Waze Travel Time: optional inclusive/exclusive filters

Added optional `inc_filter` and `excl_filter' params that allow to refine the reported routes: the first is not always the best/desired. A simple case-insensitive filtering (no regular expression) is used.

* fix line lenght

* fix spaces

* Rename var

* Fix typo

* Fix missing var
@home-assistant home-assistant locked and limited conversation to collaborators Sep 5, 2018
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.

5 participants