Merge lp:~ubuntu-weather-dev/ubuntu-weather-app/new-weather-api into lp:ubuntu-weather-app

[Victor Thompson]

* Update to use new weather API
* Change the trimAPIKey function so it only trims the API Key
* Release 3.2 and bump version to 3.3

Move to the new weather api.

For reference, here is the commit to use the new API in the scope: http://bazaar.launchpad.net/~ubuntuone-hackers/ubuntu-rest-scopes/trunk/revision/505

[Andrew Hayzen (ahayzen)]

OK, this is the first iteration of a 'working' solution, still needs lots of testing :-) Basically as TWC now, from what I understand, requires us to do multiple url calls to get the current, daily and hourly data. I've copied the OWM code and modified it to match the TWC api (as OWM is similar in the fact that it requires multiple url calls).

Main inline comment so far is:
Not sure what todo here? This is for "weather codes" such as, UKXX0085, which we were using, but I cannot see that in the new API? Currently the 1==2 is forcing it to use coordinate based lookups.

Also was the search using TWC? I need to check if that needs to be updated?

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

PASSED: Continuous integration, rev:232
https://core-apps-jenkins.ubuntu.com/job/weather-app-ci/59/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/899/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/weather-app-ci/59/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

Thanks for finishing this off, Andrew!!

I ran AP and all tests pass. Code changes look fine so far, but one thing we might want to take the opportunity to do now (otherwise we may never do so) is to audit our code coverage in WeatherApi.js and remove any dead code so we don't do needless maintenance on unused code in the future. QtCreator doesn't seem to have a coverage tool, but there seem to be numerous third party tools and qml plugins that might be helpful. Maybe just a JS only coverage tool would be sufficient.

I've been testing this change out and have not hit any regressions thus far. Unless we want to look at removing any dead code, I think this is ready for QA.

TODO:
1. Update changelog
2. Bump version number?
3. Potentially look at removing any dead code

review: Needs Fixing

[lp:~ubuntu-weather-dev/ubuntu-weather-app/new-weather-api]

233. By Andrew Hayzen on 2016-06-04

* Release 3.2 and update changelog

234. By Andrew Hayzen on 2016-06-04

* Comment out 'dead code' for now

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

PASSED: Continuous integration, rev:234
https://core-apps-jenkins.ubuntu.com/job/weather-app-ci/60/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/901/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/weather-app-ci/60/rebuild

review: Approve (continuous-integration)

[Andrew Hayzen (ahayzen)]

1) Done
2) Done bumped the version and put the relevant stuff in the sections
3) I've commented out the 'dead code' where it uses the location code for TWC, let me know if you want me to remove

Also, I've spotted one todo on line 44 of the diff
+ var partData = dataObj["metric"] || dataObj["imperial"] || dataObj; // TODO: be more intelligent?

Do you think this is OK? or should it be more like?
if (there is metric) { use metric } else if (there is imperial) { use imperial } else { fallback }

Futhermore the lines below have things like calcFahrenheit() which convert from metric->imperial, so it assumes the data is metric, maybe we should remove the imperial option... so just

+ var partData = dataObj["metric"] || dataObj;

I think i've talked myself into changing it to that :')

review: Needs Information

[lp:~ubuntu-weather-dev/ubuntu-weather-app/new-weather-api]

235. By Andrew Hayzen on 2016-06-04

* Remove dead code
* Fix up imperial todo

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

PASSED: Continuous integration, rev:235
https://core-apps-jenkins.ubuntu.com/job/weather-app-ci/61/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/902/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/weather-app-ci/61/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

This isn't a regression, as I've noticed it before--TWC always displays the high temp as the current temp as soon as the high has been reached. Maybe we're using the API incorrectly to determine today's high? Since it's not a regression, I'm fine leaving it as-is... but we should probably file a bug or add a comment to the code with a TODO to investigate. Bug would probably be preferred.

review: Needs Information

[Victor Thompson (vthompson)]

Actually, maybe it's not a bug. Weather.com seems to do the same. Maybe that's their way of indicating that the temp won't rise?

[lp:~ubuntu-weather-dev/ubuntu-weather-app/new-weather-api]

236. By Andrew Hayzen on 2016-06-04

* Fix for max_temp being null in TWC causing 'high' to be 0 and therefore lower than the 'low', now show a "-"

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

PASSED: Continuous integration, rev:236
https://core-apps-jenkins.ubuntu.com/job/weather-app-ci/62/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/903/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/weather-app-ci/62/rebuild

review: Approve (continuous-integration)

[lp:~ubuntu-weather-dev/ubuntu-weather-app/new-weather-api]

237. By Andrew Hayzen on 2016-06-04

* Use "en dash"

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

PASSED: Continuous integration, rev:237
https://core-apps-jenkins.ubuntu.com/job/weather-app-ci/63/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/904/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/weather-app-ci/63/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

Ok, this looks good now. Approve!

review: Approve

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

PASSED: Continuous integration, rev:237
https://core-apps-jenkins.ubuntu.com/job/weather-app-ci/65/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/910/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/weather-app-ci/65/rebuild

review: Approve (continuous-integration)

=== modified file 'app/data/WeatherApi.js'
--- app/data/WeatherApi.js	2015-10-21 02:16:50 +0000
+++ app/data/WeatherApi.js	2016-06-04 21:12:20 +0000
@@ -100,15 +100,20 @@
 }
 
 
-// Remove anything including and after APPID in the given term
+// Remove just the API key
 function trimAPIKey(data) {
-    var owm = data.indexOf("&APPID=");
-    var twc = data.indexOf("&key=");
+    var owm = data.indexOf("APPID=");
+    var twc = data.indexOf("apiKey=");
+
+    // Key length is 32 char for both APIs
+    var keySize = 32;
 
     if (owm > -1) {
-        data = data.substr(0, owm);
+        var replace = data.substr(owm+6, keySize);
+        data = data.replace(replace,"")
     } else if (twc > -1) {
-        data = data.substr(0, twc);
+        var replace = data.substr(twc+7, keySize);
+        data = data.replace(replace,"")
     }
 
     return data;
@@ -400,7 +405,7 @@
     /**
       provides neccessary methods for requesting and preparing data from OpenWeatherMap.org
     */
-    var _baseUrl = "http://wxdata.weather.com/wxdata/";
+    var _baseUrl = "https://api.weather.com/";
     //
     var _serviceName = "weatherchannel";
     //
@@ -457,65 +462,70 @@
     };
     //
     function _buildDataPoint(date, dataObj) {
-        var data = dataObj["Observation"] || dataObj,
+        var partData = dataObj["metric"] || dataObj;
+        var data = dataObj["observation"] || dataObj,
             result = {
-                timestamp: data.date || data.dateTime,
+                timestamp: data.fcst_valid,
                 date: date,
                 metric: {
-                    temp: data.temp,
-                    tempFeels: data.feelsLike,
-                    windSpeed: data.wSpeed
+                    temp: partData.temp,
+                    tempFeels: partData.feels_like,
+                    windSpeed: partData.wspd
                 },
                 imperial: {
-                    temp: calcFahrenheit(data.temp),
-                    tempFeels: calcFahrenheit(data.feelsLike),
-                    windSpeed: convertKphToMph(data.wSpeed)
+                    temp: calcFahrenheit(partData.temp),
+                    tempFeels: calcFahrenheit(partData.feels_like),
+                    windSpeed: convertKphToMph(partData.wspd)
                 },
                 precipType: (data.precip_type !== undefined) ? data.precip_type : null,
                 propPrecip: (data.pop !== undefined) ? data.pop : null,
-                humidity: data.humid,
-                pressure: data.pressure,
-                windDeg: data.wDir,
-                windDir: data.wDirText,
-                icon: _iconMap[(data.wxIcon||data.icon)],
-                condition: data.text || data.wDesc,
-                uv: data.uv
+                humidity: partData.rh,
+                pressure: partData.mslp,
+                windDeg: data.wdir,
+                windDir: data.wdir_cardinal,
+                icon: _iconMap[data.icon_code],
+                condition: data.phrase_32char,
+                uv: data.uv_index
         };
-        if(_iconMap[data.wxIcon||data.icon] === undefined) {
-            print("ICON MISSING POINT: "+(data.wxIcon||data.icon)+" "+result.condition)
+
+        if (_iconMap[data.icon_code] === undefined) {
+            print("ICON MISSING POINT: " + data.icon_code + " " + result.condition)
         }
+
         return result;
     }
     //
     function _buildDayFormat(date, data, now) {
-        var partData = (now > data.validDate || data.day === undefined) ? data.night : data.day,
+        var partData = (now > data.fcst_valid || data.day === undefined) ? data.night : data.day,
             result = {
             date: date,
-            timestamp: data.validDate,
+            timestamp: data.fcst_valid,
             metric: {
-                tempMin: data.minTemp,
-                tempMax: data.maxTemp,
-                windSpeed: partData.wSpeed
+                tempMin: data.min_temp,
+                tempMax: data.max_temp !== null ? data.max_temp : undefined,
+                windSpeed: partData.wspd
             },
             imperial: {
-                tempMin: calcFahrenheit(data.minTemp),
-                tempMax: calcFahrenheit(data.maxTemp !== undefined ? data.maxTemp : data.minTemp),
-                windSpeed: convertKphToMph(partData.wSpeed)
+                tempMin: calcFahrenheit(data.min_temp),
+                tempMax: data.max_temp !== null && data.max_temp !== undefined ? calcFahrenheit(data.max_temp) : undefined,
+                windSpeed: convertKphToMph(partData.wspd)
             },
             precipType: partData.precip_type,
             propPrecip: partData.pop,
             pressure: null,
-            humidity: partData.humid,
-            icon: _iconMap[partData.icon],
-            condition: partData.phrase,
-            windDeg: partData.wDir,
-            windDir: partData.wDirText,
-            uv: partData.uv,
+            humidity: partData.rh,
+            icon: _iconMap[partData.icon_code],
+            condition: partData.phrase_32char,
+            windDeg: partData.wdir,
+            windDir: partData.wdir_cardinal,
+            uv: partData.uv_index,
             hourly: []
         }
-        if(_iconMap[partData.icon] === undefined) {
-            print("ICON MISSING  DAY: "+partData.icon+" "+result.condition)
+
+        if (_iconMap[partData.icon_code] === undefined) {
+            print("ICON MISSING  DAY: " + partData.icon_code + " " + result.condition)
         }
+
         return result;
     }
     //
@@ -527,35 +537,43 @@
             nowMs = parseInt(now/1000),
             localNow = getLocationTime(now+offset),
             data = {
-                "location": combinedData[0]["Location"],
-                "daily": combinedData[0]["DailyForecasts"],
-                "forecast": combinedData[0]["HourlyForecasts"],
-                "current": combinedData[0]["StandardObservation"],
-                "sunRiseSet": combinedData[0]["SunRiseSet"],
+                "location": combinedData["current"]["metadata"],
+                "daily": combinedData["daily"]["forecasts"],
+                "forecast": combinedData["forecast"]["forecasts"],
+                "current": combinedData["current"]["observation"],
             };
         print("["+location.name+"] "+JSON.stringify(localNow));
         // add openweathermap id for faster responses
         if(location.services && !location.services[_serviceName] && data["location"].key) {
             location.services[_serviceName] = data["location"].key
-        }                
+        }
         // only 5 days of forecast for TWC
-        for(var x=0;x<5;x++) {
+        for(var x=0; x<5; x++) {
             var dayData = data["daily"][x],
-                date = getLocationTime(((dayData.validDate*1000)-1000)+offset); // minus 1 sec to handle +/-12 TZ
-            var sunRiseSet = data["sunRiseSet"][x];
+                date = getLocationTime(((dayData.fcst_valid * 1000) - 1000) + offset);  // minus 1 sec to handle +/-12 TZ
+
+            // Sun{rise,set} is in ISOString format so use getTime() to convert
+            var sunrise = new Date(dayData.sunrise).getTime(),
+                sunset = new Date(dayData.sunset).getTime();
             day = date.year+"-"+date.month+"-"+date.date;
-            if(!todayDate) {
-                if(localNow.year+"-"+localNow.month+"-"+localNow.date > day) {
+
+            if (!todayDate) {
+                if (localNow.year + "-" + localNow.month + "-" + localNow.date > day) {
                     // skip "yesterday"
                     continue;
                 }
+
                 todayDate = date;
             }
+
             tmpResult[day] = _buildDayFormat(date, dayData, nowMs);
+
             var timezoneOffset = new Date().getTimezoneOffset();
             var timesOffset = (location.timezone && location.timezone.dstOffset !== undefined) ? (location.timezone.dstOffset*60 + timezoneOffset)*60*1000: 0
-            var sunrise = new Date(sunRiseSet.rise*1000 + timesOffset);
-            var sunset = new Date(sunRiseSet.set*1000 + timesOffset);
+
+            sunrise = new Date(sunrise + timesOffset);
+            sunset = new Date(sunset + timesOffset);
+
             var options = { timeZone: location.timezone.timeZoneId, timeZoneName: 'long' };
             tmpResult[day].sunrise = sunrise.toLocaleTimeString(Qt.locale().name, options);
             tmpResult[day].sunset = sunset.toLocaleTimeString(Qt.locale().name, options);
@@ -563,8 +581,9 @@
         //
         if(data["forecast"] !== undefined) {
             data["forecast"].forEach(function(hourData) {
-                var dateData = getLocationTime((hourData.dateTime*1000)+offset),
+                var dateData = getLocationTime((hourData.fcst_valid * 1000) + offset),
                     day = dateData.year+"-"+dateData.month+"-"+dateData.date;
+
                 if(tmpResult[day]) {
                     tmpResult[day]["hourly"].push(_buildDataPoint(dateData, hourData));
                 }
@@ -591,48 +610,93 @@
         return result;
     }
     //
-    function _getUrl(params) {
-        var url, serviceId,
-            baseParams = {
-                key: params.twc_api_key,
-                units: (params.units === "metric") ? "m" : "e",
-                locale: Qt.locale().name,
-                hours: "48",
-            },
-            commands = {
-                "mobileaggregation": "mobile/mobagg/",
+    function _getUrls(params) {
+        var baseParams = {
+            units: (params.units === "metric") ? "m" : "e",
+            language: Qt.locale().name.replace("_","-"),
+            apiKey: params.twc_api_key,
+        };
+        var commands = {
+            "geocode": "v1/geocode/",
+        };
+        var urls = {
+            current: "",
+            daily: "",
+            hourly: "",
+        };
+
+        // FIXME: only use coords for now and not location codes (UKXX0085)
+        if (params.location.coord) {
+            var coord = {
+                lat: params.location.coord.lat,
+                lng: params.location.coord.lon
             };
-        if(params.location.services && params.location.services[_serviceName]) {
-            serviceId = encodeURIComponent(params.location.services[_serviceName]);
-            url = _baseUrl+commands["mobileaggregation"]+serviceId+".js?"+parameterize(baseParams);
-        } else if (params.location.coord) {
-            var coord = {lat: params.location.coord.lat, lng: params.location.coord.lon};
-            url = _baseUrl+commands["mobileaggregation"]+"get.js?"+parameterize(baseParams)+"&"+
-                  parameterize(coord);
+
+            urls.current = _baseUrl + commands["geocode"] +
+                    coord.lat + "/" + coord.lng +
+                    "/observations/current.json?" +
+                    parameterize(baseParams);
+            urls.daily = _baseUrl + commands["geocode"] +
+                    coord.lat + "/" + coord.lng +
+                    "/forecast/daily/5day.json?" +
+                    parameterize(baseParams);
+            urls.hourly = _baseUrl + commands["geocode"] +
+                    coord.lat + "/" + coord.lng +
+                    "/forecast/hourly/48hour.json?" +
+                    parameterize(baseParams);
         }
-        return url;
+
+        return urls;
     }
     //
     return {
         getData: function(params, apiCaller, onSuccess, onError) {
-            var url = _getUrl(params),
+            var urls = _getUrls(params),
                 handlerMap = {
-                    all: { type: "all", url: url}
-                },
-                response = {
-                    location: params.location,
-                    db: (params.db) ? params.db : null,
-                    format: RESPONSE_DATA_VERSION
-                },
-                addDataToResponse = (function(request, data) {
-                    var formattedResult;
-                    response["data"] = formatResult(data, params.location);
+                current: {
+                    type: "current",
+                    url: urls.current
+                },
+                daily: {
+                    type: "daily",
+                    url: urls.daily
+                },
+                forecast: {
+                    type: "forecast",
+                    url: urls.hourly
+                }
+            },
+            response = {
+                location: params.location,
+                db: (params.db) ? params.db : null,
+                format: RESPONSE_DATA_VERSION
+            },
+            respData = {},
+            addDataToResponse = (function(request, data) {
+                var formattedResult;
+                respData[request.type] = data;
+
+                if (respData["current"] !== undefined &&
+                        respData["forecast"] !== undefined &&
+                            respData["daily"] !== undefined) {
+
+                    response["data"] = formatResult(respData, params.location);
                     onSuccess(response);
-                }),
-                onErrorHandler = (function(err) {
-                    onError(err);
-                });
-            apiCaller(handlerMap.all, addDataToResponse, onErrorHandler);
+                }
+            }),
+            onErrorHandler = (function(err) {
+                onError(err);
+            }),
+            retryHandler = (function(err) {
+                console.log("retry of " + trimAPIKey(err.request.url));
+                var retryFunc = handlerMap[err.request.type];
+
+                apiCaller(retryFunc, addDataToResponse, onErrorHandler);
+            });
+
+            apiCaller(handlerMap.current, addDataToResponse, retryHandler);
+            apiCaller(handlerMap.forecast, addDataToResponse, retryHandler);
+            apiCaller(handlerMap.daily, addDataToResponse, retryHandler);
         }
     }
 })();

=== modified file 'app/ui/LocationPane.qml'
--- app/ui/LocationPane.qml	2016-02-06 01:17:07 +0000
+++ app/ui/LocationPane.qml	2016-06-04 21:12:20 +0000
@@ -150,7 +150,7 @@
         return {
             day: formatTimestamp(data.date, 'dddd'),
             low: Math.round(data[tempUnits].tempMin).toString() + settings.tempScale,
-            high: (data[tempUnits].tempMax !== undefined) ? Math.round(data[tempUnits].tempMax).toString() + settings.tempScale : "",
+            high: (data[tempUnits].tempMax !== undefined) ? Math.round(data[tempUnits].tempMax).toString() + settings.tempScale : "–",
             image: (data.icon !== undefined && iconMap[data.icon] !== undefined) ? iconMap[data.icon] : "",
             condition: emptyIfUndefined(data.condition),
             chanceOfPrecip: emptyIfUndefined(data.propPrecip, "%"),

=== modified file 'debian/changelog'
--- debian/changelog	2016-03-26 16:38:54 +0000
+++ debian/changelog	2016-06-04 21:12:20 +0000
@@ -1,4 +1,16 @@
-ubuntu-weather-app (3.2) UNRELEASED; urgency=medium
+ubuntu-weather-app (3.3ubuntu1) UNRELEASED; urgency=medium
+
+  [ Andrew Hayzen ]
+  * Release 3.2 and bump version to 3.3
+  * Update to use new weather API
+
+  [ Victor Thompson ]
+  * Update to use new weather API
+  * Change the trimAPIKey function so it only trims the API Key
+
+ -- Andrew Hayzen <[e-mail]>  Sat, 04 Jun 2016 18:31:29 +0100
+
+ubuntu-weather-app (3.2ubuntu1) vivid; urgency=medium
 
   [ Andrew Hayzen ]
   * Release 3.1 and bump version to 3.2
@@ -20,7 +32,7 @@
   [ Vamshi Balanaga ]
   * Added test to swipe between location tabs on home page (LP: #1452492)
 
- -- Andrew Hayzen <[e-mail]>  Thu, 03 Dec 2015 15:20:49 +0000
+ -- Andrew Hayzen <[e-mail]>  Sat, 04 Jun 2016 18:30:00 +0100
 
 ubuntu-weather-app (3.1ubuntu1) vivid; urgency=medium
 

=== modified file 'manifest.json.in'
--- manifest.json.in	2016-02-28 22:15:28 +0000
+++ manifest.json.in	2016-06-04 21:12:20 +0000
@@ -12,7 +12,7 @@
     "maintainer": "Ubuntu App Cats <[e-mail]>",
     "name": "@PROJECT_NAME@",
     "title": "Weather",
-    "version": "3.2.@BZR_REVNO@",
+    "version": "3.3.@BZR_REVNO@",
     "x-source": {
         "vcs-bzr": "@BZR_SOURCE@",
         "vcs-bzr-revno": "@BZR_REVNO@"