Merge lp:~vthompson/ubuntu-weather-app/reboot-location-qml into lp:ubuntu-weather-app

[Victor Thompson]

* Implement QtLocation reverse geocoding
* Use Open Street Map plugin
* Update apparmor

* Implement QtLocation reverse geocoding
* Use Open Street Map plugin
* Update apparmor

This simply displays the reverse geocoded city name in the LocationsPage at the first item. This uses the QtLocations QML API for Location models and utilizes the OSM plugin for reverse geocoding. The Nokia HERE plugin did not appear to support reverse geocoding and also required an API key. It may take a bit for the "Undefined" text to change to your current GPS location.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:44
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/90/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-utopic-amd64-ci/60
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-vivid-amd64-ci/90

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/90/rebuild

review: Approve (continuous-integration)

[Alan Pope 🍺🐧🐱 🦄 (popey)]

Can you explain what I should see with this branch?
Is it supposed to show when Locations page is empty?
http://people.canonical.com/~alan/screenshots/device-2015-05-19-101516.png

[Victor Thompson (vthompson)]

It should always add an item in that view. Are you on RTM? If so, maybe
there's something missing in that image. You should get prompted to share
your location. Could you attach your weather app log?
On May 19, 2015 4:15 AM, "Alan Pope " <email address hidden> wrote:

> Can you explain what I should see with this branch?
> Is it supposed to show when Locations page is empty?
> http://people.canonical.com/~alan/screenshots/device-2015-05-19-101516.png
>
> --
>
> https://code.launchpad.net/~vthompson/ubuntu-weather-app/reboot-location-qml/+merge/259462
> You are the owner of lp:~vthompson/ubuntu-weather-app/reboot-location-qml.
>

[Alan Pope 🍺🐧🐱 🦄 (popey)]

Just tested on my devel proposed phone which appears to be wily now..

phablet@ubuntu-phablet:~$ system-image-cli --info
current build number: 218
device name: krillin
channel: ubuntu-touch/devel-proposed/ubuntu
last update: 2015-05-19 08:35:36
version version: 218
version ubuntu: 20150519
version device: 20150511-3912934
version custom: 20150519
phablet@ubuntu-phablet:~$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu Wily Werewolf (development branch)
Release: 15.10
Codename: wily

I get a blank screen if I remove the last city, and the main screen shows a bouncer at the bottom with nothing else going on.
http://paste.ubuntu.com/11224712/ is the log

[Victor Thompson (vthompson)]

@popey, this is similar to what you should see initially [1], then once you get a lock on your location you should see something like this [2].

If you remove the only real city in the list, then yes, right now you get the bouncer on the main screen--we still need empty states for this condition.

1 - http://i.imgur.com/zX4vZBs.png
2 - http://i.imgur.com/Sw9OZ8s.png

[lp:~vthompson/ubuntu-weather-app/reboot-location-qml]

45. By Victor Thompson on 2015-05-20

Minimize update of current location.

46. By Victor Thompson on 2015-05-20

Add TODO note

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:46
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/92/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-utopic-amd64-ci/62
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-vivid-amd64-ci/92

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/92/rebuild

review: Approve (continuous-integration)

[Alan Pope 🍺🐧🐱 🦄 (popey)]

Ok, that makes more sense, and works as expected.

review: Approve

[Andrew Hayzen (ahayzen)]

After ensuring that the location service is actually working by using google/here maps. I was able to start weather, accept the location detection and it instantly added it to the top of the list :-)

However it always displays 0°C for the location and it hasn't actually added the location to the home page so this causes issues such as trying to remove the last location as the indexes are out of sync. Or is this what your TODO message is saying will be in a future mp?

Also maybe "undefined" is not the best word to use that could appear in the UI and this string should be translatable.

review: Needs Information

[Alan Pope 🍺🐧🐱 🦄 (popey)]

Agreed on the name. "Detecting location" (or something shorter / snappier) might be more prudent. Perhaps just "Current location" which then switches to the real location name when discovered?

[Victor Thompson (vthompson)]

The intent of this MP is just to add location detection. We'll need more work for managing the current location. I also don't think we'll want to show "Undefined", but this MP does so because without explicitly managing the location that is being detected _something_ needs to be in the model. We could hide it in the future if it's invalid/undefined or add some different text. Right now this just displays the discovered location. This MP doesn't add enough of the discovered data to allow for the Weather API calls to work. So you'll always see 0 degrees. We'll need to determine the minimal set of data (and where it exists) in the GeocodeModel to add it to the weather locations model properly.

[Andrew Hayzen (ahayzen)]

OK well, I think this is clear to land then and the issues stated will be fixed in upcoming mps :-)

review: Approve

[Victor Thompson (vthompson)]

Adding Martin as a reviewer. I'd like to make sure he agrees with proceeding using the existing APIs for reverse geocoding, before we go to far into making additional changes.

[Martin Borho (martin-borho)]

lgtm!

To get it work with the data backend, the current-location-object should follow the format like "location" in http://bazaar.launchpad.net/~ubuntu-weather-dev/ubuntu-weather-app/trunk/view/head:/tests/autopilot/ubuntu_weather_app/files/1.json#L2

Important is here "location.coord" and "location.timezone". I don't know if the timezone is available by reverse geocoding. If not, a "searchByPoint" with lat/long is needed beforehand. (Timezone is needed to calculate the proper local time of the location)

review: Approve

[Victor Thompson (vthompson)]

It doesn't look like Timezone is available from the data available by reverse geocoding. I think we'll still need to use GeoNames (as we do for the other locations) to get this info.

[Victor Thompson (vthompson)]

Top approving so work on Location management can proceed.

=== added file 'app/components/CurrentLocation.qml'
--- app/components/CurrentLocation.qml	1970-01-01 00:00:00 +0000
+++ app/components/CurrentLocation.qml	2015-05-20 00:09:00 +0000
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2015 Canonical Ltd
+ *
+ * This file is part of Ubuntu Weather App
+ *
+ * Ubuntu Weather App is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * Ubuntu Weather App is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import QtLocation 5.3
+import QtPositioning 5.2
+import QtQuick 2.3
+import Ubuntu.Components 1.1
+
+
+Item {
+    id: currentLocation
+
+    property string string: "Undefined"
+
+    PositionSource {
+        id: currentPosition
+        updateInterval: 1000
+        active: true
+
+        onPositionChanged: {
+            var coord = currentPosition.position.coordinate
+            if (coord.isValid) {
+                geocodeModel.query = coord
+                geocodeModel.update()
+            }
+        }
+    }
+
+    Plugin {
+        id: osmPlugin
+        name: "osm"
+    }
+
+    GeocodeModel {
+        id: geocodeModel
+        autoUpdate: false
+        plugin: osmPlugin
+
+        onCountChanged: {
+            // Update the currentLocation if one is found and it does not match the stored location
+            if (count > 0 && currentLocation.string !== geocodeModel.get(0).address.city) {
+                currentLocation.string = geocodeModel.get(0).address.city
+                refreshData(false, true)
+            }
+        }
+    }
+}

=== modified file 'app/ubuntu-weather-app.qml'
--- app/ubuntu-weather-app.qml	2015-05-08 19:44:49 +0000
+++ app/ubuntu-weather-app.qml	2015-05-20 00:09:00 +0000
@@ -129,6 +129,10 @@
         }
     }
 
+    CurrentLocation {
+        id: currentLocation
+    }
+
     Settings {
         id: settings
         category: "weatherSettings"

=== modified file 'app/ui/LocationsPage.qml'
--- app/ui/LocationsPage.qml	2015-04-09 22:22:55 +0000
+++ app/ui/LocationsPage.qml	2015-05-20 00:09:00 +0000
@@ -138,6 +138,10 @@
         locationsModel.clear()
         var loc = {}, data = {},
             tempUnits = settings.tempScale === "°C" ? "metric" : "imperial";
+
+        // TODO: Manage the current location as an entry in the locationsList.
+        locationsModel.append({ "name": currentLocation.string, "temp": "0", "icon": "weather-clear-symbolic" })
+
         for (var i=0; i < weatherApp.locationsList.length; i++) {
             data = weatherApp.locationsList[i];
             loc = {

=== modified file 'ubuntu-weather-app.apparmor'
--- ubuntu-weather-app.apparmor	2015-01-23 23:26:58 +0000
+++ ubuntu-weather-app.apparmor	2015-05-20 00:09:00 +0000
@@ -1,6 +1,8 @@
 {
     "policy_groups": [
-    	"networking"
+    	"location",
+    	"networking",
+    	"sensors"
     ], 
     "policy_version": 1.2
 }