-
-
Notifications
You must be signed in to change notification settings - Fork 497
Added an address field to the returned location packet. #2142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…geocoded address, if available, or the lat/lon if not.
|
I realize that there are situations where the phone geocode will not resolve the address, but this reduces the expense of having to do that same work server side now that Google has reduced the free geocode tokens per month.. |
|
I think this approach was discussed and rejected a while ago: owntracks/ios#769 |
|
How rate limited is iOS? Android will also rate limit, but you need to be sending a lot of geocodes in succession. Issue I'm trying to address is Google has reduced their free credit tier for geocodes. If you have 4-users on OwnTracks, and I configured the recorder to use Google as a geocode service, I'd end up with a $100 Google bill every month for this. Others have seen much worse depending on update frequency. I was coupling this into a different server side app, using the phone addresses when available (which is most of the time) and then back filling with server side geocode to address the over charge issue. |
|
The recorder cashes Geo look ups so if those four users are frequently together, and the geohash width isn't too high then caching should reduce the look up frequency. As an aside: have you compared limits to those in Opencage's free tier? |
|
What I was seeing is all users are semi-independent, which was resulting in a bill from Google for exceeding the free credits on geocoding. Opencage changed their tiers when Google restricted theirs. Opencage gives 2500/day, but it you are caught using it more than short term testing, you will be get blocked. |
|
~~I seem to remember there's a ToS constraints on both the Google and opencage geocoders that their returned results are for display only and not transmission/storage. Might be hallucinations, will have a look tomorrow~~. Edit I'm wrong about opencage. I'm not against this in principle, but I think it should be a user preference (default off). (On my phone, not done a review yet, so you may have already added this in). We'd also presumably need a tweak to the recorder to detect the incoming address on the wire and add that to the db, rather than doing a lookup on the recorder side @jpmens ? |
|
Let me know if it is something you'd accept, and I can rework with a user preference. |
|
OpenCage explicitly permits storage.
|
https://opencagedata.com/guides/how-to-compare-and-test-geocoding-services |
Are you thinking a new slider in the "Preferences -> Reporting" section, similar to "Extended Data"? Or just couple this with the "Extended Data"? |
|
(TIL github notifications don't trigger when added during an edit.)
doable if we know which JSON element to detect on |
This feels "extended data" to me. I think we put stuff in there that's platform-specific, and we're not (I think) going to be doing this on iOS any time soon? Good place to start and see how it fits. |
|
The recorder includes address data from its geohash database as follows including country, locality, tzname and local time. When sending any of these value to the recorder, we may want to use different key e.g. add a prefix: We will have to decide whether recorder will update its Geohash database from the client provided data and how to deal with situations where database address and client provided address data are both available |
I pushed a patch moving it to the extended data field. |
|
Another question, is there a way to trigger the jobs on the patch push? It seems the last didn't trigger anything. |
|
/run-tests |
|
Build & Test complete. ✨ |
|
/run-tests |
1 similar comment
|
/run-tests |
The above doesn't seem to work for me. Github isn't picking up the action? |
Contains the geocoded address, if available, or the lat/lon if not.