Review Request: Add city and country resolution to GPS geolocation data using geonames.

Aaron Seigo aseigo at kde.org
Wed Apr 15 03:59:54 CEST 2009


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/592/#review923
-----------------------------------------------------------


other than a couple of code style comments, and that i can't test it with a gps for you either :/, my only question/concern is that it's doing look ups on the internet without checking to see if we're connected. it could be in a local cache, but i'm going to guess that in a fairly typical "i'm using gps" scenario one won't have an internet connection as well. should we query solid for network status?

rambling off-topic here: i also wonder if we aren't going to want some "can use the internet for ..." settings somewhere for the case where we have a system with gps, cheap wifi and expensive g3 connectivity .. somethings, like looking up the place name on the internet, may not be acceptible even if there is g3 service? not a use case we actually have to deal with now, but it's something that pops into my head every once in a while .. .. like when i see a patch like this :)


/trunk/kdereview/plasma/dataengines/geolocation/location_gps.cpp
<http://reviewboard.kde.org/r/592/#comment615>

    opening {s of methods go on their own line.



/trunk/kdereview/plasma/dataengines/geolocation/location_gps.cpp
<http://reviewboard.kde.org/r/592/#comment616>

    single line if/while/for/etc get {}s too


- Aaron


On 2009-04-14 16:46:26, Andrew Coles wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/592/
> -----------------------------------------------------------
> 
> (Updated 2009-04-14 16:46:26)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Yesterday, I proposed a patch for using an IP geolocation service that returns longitude and latitude, as the GPS backend would give that data but the IP one would not.  Today, it's the other way around: a patch to add place name and country information to the GPS geolocation data, as the IP geolocation gives this but GPS geolocation does not.
> 
> The only caveat is that I'm programming blind - I don't have a GPS receiver, and 'it compiles' is far from good enough.  Hence, I need a volunteer to test it - anyone?
> 
> Assuming it works, the geolocation data engine will then have reached the point where the fields returned are identical, /irregardless of whether IP or GPS data is used/.  Specifically, the user gets:
> 
> - Latitude
> - Longitude
> - Accuracy
> - Country Name
> - Country Code
> - City Name
> 
> 
> Diffs
> -----
> 
>   /trunk/kdereview/plasma/dataengines/geolocation/location_gps.h 954031 
>   /trunk/kdereview/plasma/dataengines/geolocation/location_gps.cpp 954031 
> 
> Diff: http://reviewboard.kde.org/r/592/diff
> 
> 
> Testing
> -------
> 
> It compiles, and it looks alright.  How pitiful is that?  Given it's based on the IP geolocation XML code, but using a reverse geocoding rather than IP geolocation service, it should in theory work, but it really does need testing by someone with a GPS unit.
> 
> 
> Thanks,
> 
> Andrew
> 
>



More information about the Plasma-devel mailing list