search

kovacsbalu / WazeRouteCalculator

Calculate actual route time and distance with Waze api
GNU General Public License v3.0
146 stars 59 forks source link

Change to allow gps coordinates as to or from address #27

Closed bjohnson8949 closed 5 years ago

bjohnson8949 commented 6 years ago

Actual code changes allow the use of gps coordinates as to and from address.

Added:

Made changes to init to decide if to \ from address is actually gps coordinates.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-5.1%) to 88.034% when pulling fc47d395f66a779e0197fd093d0d76e58fb8bf8f on bjohnson8949:master into afc25b320a55baf9657f9e01b92020a61783ad03 on kovacsbalu:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-5.1%) to 88.034% when pulling fc47d395f66a779e0197fd093d0d76e58fb8bf8f on bjohnson8949:master into afc25b320a55baf9657f9e01b92020a61783ad03 on kovacsbalu:master.

kovacsbalu commented 6 years ago

Hi @bjohnson8949 good feature, but can you please commit some test for this new functions?

SConaway commented 5 years ago

Can we please merge this as-is? The address to coords is kinda broken where I live and would like to use an official release.

kovacsbalu commented 5 years ago

Hi @SConaway, as I mentioned yes this is a good feature. I will do some refact and test before merge. Coming soon :)

SConaway commented 5 years ago

If it helps, I can help test. One note is usually when lat and long are named, it returns empty response (3/4 time)

hmmbob commented 5 years ago

Hello @kovacsbalu,

Your package is being used in Home Assistant, an open source smart home software platform. Thank you for your work on this package, as it enables a lot of users to use Waze for travel time calculations. The Home Assistant "Waze Travel Time Sensor" displays the time needed to travel from one point to another, quite often from a device trackers current location to the 'home' or 'work' zone. Home Assistant uses coordinates for "zones" (e.g. 'home','work') and for the location of device-trackers, instead of addresses.

Home Assistant currently uses version 0.6 of your package. Several users (including myself) have issues with unhandled errors with this package, which seem to have been solved by you in later versions. However, if we use the most recent version, we cannot pass coordinates to your package anymore. For whatever reason, this worked fine in 0.6, but no longer in 0.7.2. As said, Home Assistant uses coordinates for "zones" (e.g. 'home','work') and for the location of device-trackers.

Would it be possible to merge this functionality into your package, so we can update the package requirements for Home Assistant? I would gladly help, if I only knew some decent Python. I don't, hence my plea for your help.

Above all, thank you for providing this package to the community.

kovacsbalu commented 5 years ago

Hi @hmmbob, thanks for your details. I'm already a HA user :) Maybe row-SearchServer/mozi cause this problem. I will fix it and create version 0.8 with coord support.

hmmbob commented 5 years ago

Awesome! I didn't know you're using HA too - good to hear! Does that mean that whenever 0.8 gets released, you will also update the waze travel sensor in HA itself?

Thanks again - I don't want to be a guy "requiring support" or so and would gladly have done it myself, but I don't speak Python....

kovacsbalu commented 5 years ago

@hmmbob, @bjohnson8949 can you please check tag '0.8' before releasing?

hmmbob commented 5 years ago

Sure, give me a few minutes

Op za 16 feb. 2019 om 15:34 schreef Kovács Bálint <notifications@github.com

:

@hmmbob https://github.com/hmmbob, @bjohnson8949 https://github.com/bjohnson8949 can you please check tag '0.8' before releasing?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kovacsbalu/WazeRouteCalculator/pull/27#issuecomment-464351722, or mute the thread https://github.com/notifications/unsubscribe-auth/Af-eklv79sLanms1U3ERNTvWlHAeVPEZks5vOBbqgaJpZM4XHL3N .

kovacsbalu commented 5 years ago

@hmmbob maybe...but I never tried HA waze sensor :D

hmmbob commented 5 years ago

Ok, I have 3 sensors set up in Home Assistant:

1) my location -> home 2) home -> work 3) work -> home

All are coordinate based. 2 & 3 loaded fine upon home assistant start; 1) gave an error:

2019-02-16 15:42:02 ERROR (SyncWorker_3) [custom_components.sensor.waze_travel_time] Error on retrieving data: empty response

However, waiting 5 minutes (which is the default scan interval of the Home Assistant Waze sensor) did update my sensor quite fine.

So, besides raising an error because of an empty response from the server, all is working fine now!

kovacsbalu commented 5 years ago

Released... :) BTW, some times I also got empty response with caused by 429 Too Many Requests

hmmbob commented 5 years ago

Thanks, I will pull it into HA!

SConaway commented 5 years ago

What format for lat/long does it take?

hmmbob commented 5 years ago

This is a mock coordinate for one of my device trackers that works: 51.12345, 5.123456

SConaway commented 5 years ago

It works for some lat/lons, but not all.

Steven On Feb 17, 2019, 11:24 AM -0800, Hmmbob notifications@github.com, wrote:

This is a mock coordinate for one of my device trackers that works: 51.12345, 5.123456 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

kovacsbalu commented 5 years ago

@SConaway format is lat, lon = coords.split(',')