GNOME Bugzilla – Bug 704268
Show weather for the current location
Last modified: 2014-04-01 21:07:37 UTC
The most common case for checking the weather is to check where you are now. This would be a great place to use geolocation to automatically figure out where you are. We have two conflicting designs for this. The first uses a separate view for the current location: https://raw.github.com/gnome-design-team/gnome-mockups/master/weather/weather-overview.png The second would be to take the same approach is in Clocks: https://raw.github.com/gnome-design-team/gnome-mockups/master/clocks/geolocation-wires.png The first approach might have a few issues with it: * It isn't clear how we would reconcile the current location also being in the World Weather grid * It isn't clear what the UI would be like if geolocation positioning isn't available So maybe option two?
I like your approach much better.
*** Bug 704397 has been marked as a duplicate of this bug. ***
Doesn't look like this is going to happen for 3.10 - no patch yet. This is something we can probably do for 3.10.1
This is the video link of what I've implemented so far : http://www.youtube.com/watch?v=KjqGsX6l2iY In this video, one can see that the current location (in fact for now the nearest weather station) is added automatically after it is fetched. Need suggestions and hints on following : 1> The new city is appeared automatically suddenly. Is there any better way to make it appear because it takes some time to get details about nearest weather station ? 2> I've fetched Geocode.Location from geoclue which contains the current location. Using that location's coordinates I've gone through the list of all weather-station listed in Location.xml and find out the nearest weather station. For this I tried to use direct find_nearest_city method of GWeatherLocation but it was giving segfault error (unfortunately I can't find the reason) so I've implemented it in javascript and get the nearest weather-station. Now I've directly added this station to locations list and so the city and country name would be of the nearest weather-station instead of this what we want is to show current city name which is stored in description in Geocode.Location. How to merge current-location names in fetched nearest weather-station ? 3> This process happens every-time user starts the app so would it be ok to remove current location from grid on quitting the app ? It seems ok and Alan Day has also suggested that user does not want to have all the cites in grid that he visits and this seems valid point. 4> I want suggestion on what should happen if user keeps open city-view of nearest weather-station and change his location ? 5> Plus we want to add an icon in front of PrimaryText of current location icon in the grid to differentiate it from other locations. How to do this ?
Created attachment 270930 [details] [review] Support to show current location weather-condition
Created attachment 270939 [details] [review] Little modified. In addition to previous patch, I've used reverse geocoding to get place from geocode location so that I can directly use place and coutry name to show in the grid.
Created attachment 270943 [details] [review] Little modified. I've used reverse geocoding to get city and country name from Geocode.Location and used them to show the names in the world-view and city-view for current lcoation.
Review of attachment 270943 [details] [review]: Most of the code doesn't feel Window specific at all. I would add a "CurrentLocationController" off the Application object (like the SearchProvider), that takes care of adding and removing the current location from the WorldModel. Maybe together with a boolean in the model to say that the location is not stored in the settings. ::: src/main.js @@ +64,3 @@ + if (this._win.currentLocation) { + let settings = Util.getSettings('org.gnome.Weather.Application'); + let newLocations = settings.get_value('locations').deep_unpack(); Use the world model, don't poke at settings directly. @@ +69,3 @@ + + for (let i = 0; i < newLocations.length; i++) { + if (newLocations[i].equal(variant)) { Don't check for equal variants, check for equal locations (to avoid double precision errors) @@ +75,3 @@ + } + settings.set_value('locations', new GLib.Variant('av', newLocations)); + } Don't do this onQuit, do it inside vfunc_shutdown() (onQuit is only called if the user uses quit from the app menu) ::: src/window.js @@ +338,3 @@ + if (this._currentPlace) + return [this._currentPlace.get_town(), this._currentPlace.get_country()]; + } Same here, the GWeatherLocation should have the right values. ::: src/world.js @@ +193,3 @@ let newLocations = this._settings.get_value('locations').deep_unpack(); newLocations.push(location.serialize()); + this._currentLocationName = currentLocationName; Wow this is hackish! We should totally have libgweather return the right value from get_city_name() here.
Created attachment 271198 [details] [review] This is properly implemented as suggested in previous patch For this I've used find_nearest_city method of libgweather which is currently buggy on master branch so apply the patch that I submitted on the bug : https://bugzilla.gnome.org/show_bug.cgi?id=725671 thereafter try to run this patch.
Created attachment 271309 [details] [review] With the support to remove added current location on shutting down the app If possible then please push the patch for the bug : https://bugzilla.gnome.org/show_bug.cgi?id=725671 I've used find_nearest_city method so. Further potential modifications : Here for now we are just simply getting nearest_weather_station but after using patch that I submitted for the bug : https://bugzilla.gnome.org/show_bug.cgi?id=725842 we can get current location with its own information in location object and weather conditions of the nearest weather-station using find_nearest_city_async method.
Review of attachment 271309 [details] [review]: Mh, no, add a new column to the world model, indicating if the location should be saved or not, instead of poking through the settings. In the MVC pattern, a controller should never bypass the model to change the data store directly, it's bad coding. Similarly, the task of adding the location to the model is not something the view (window.js) should do.
Created attachment 271338 [details] [review] Modified patch. Improved as suggested in previous patch.
Review of attachment 271338 [details] [review]: Looks good to merge when the development window opens again (after 3.12.0). ::: src/currentLocationController.js @@ +89,3 @@ + "org.freedesktop.GeoClue2", + clientPath); + this._clientProxy.DesktopId = "gnome-weather"; Nope, it's org.gnome.Weather.Application
Pushed.