forecastie icon indicating copy to clipboard operation
forecastie copied to clipboard

Support multiple locations

Open AxelTheGerman opened this issue 8 years ago • 24 comments

I love the simplicity of the app! However, for many users it might be useful to easily access/see the weather for multiple locations (e.g. when traveling, living in multiple places etc)

two ideas for UI design

  • list of recently viewed cities in search dialog
  • slide in side menu OR separate screen with list of saved locations

open to suggestions and willing to work on this feature

Screenshots from Google Weather app on Android 6.0.1 saved location as 'autocomplete' for search input saved location as 'autocomplete' for search input

allow user to easily add searched location to list allow user to easily add searched location to list

AxelTheGerman avatar Apr 09 '16 07:04 AxelTheGerman

I would like to add that such a function would be especially helpful in combination with the offline function when traveling! Because of infrequent opportunities for internet access while backpacking I would have loved it to download forecasts for more than one location.

ppaspp avatar Sep 06 '16 11:09 ppaspp

First step could be do cache last few lookuped locations for a short period of time

erdmark avatar Feb 09 '17 07:02 erdmark

This sounds like the best option for a first cut:

  • list of recently viewed cities in search dialog

Fairly easy to implement, in comparison to the other suggestions.

robinpaulson avatar Jun 08 '17 10:06 robinpaulson

@AxelTheGerman If you're willing to write some code, please do!

robinpaulson avatar Jun 10 '17 06:06 robinpaulson

First step could be do cache last few lookuped locations for a short period of time

this also sounds fairly simple to do, it would require an expansion of what is already stored offline. I presume this is in an sqlite db in /data.

robinpaulson avatar Jun 12 '17 05:06 robinpaulson

I think this is an extremely important feature, as it makes the application even more useful. For some locations, it can be very annoying to enter the name, and simply being able to save, "pin" or cache multiple locations would make everything easier. May I ask: what is the current situation on this issue/request?

Currrupted avatar Apr 26 '19 18:04 Currrupted

May I ask: what is the current situation on this issue/request?

I'm working on this issue at the moment. There are many changes to be done (more than I initially expected :sweat_smile:), and as I'm working on it on my free time, it might still take a while until everything is ready.

Here's a screenshot of how the app looks right now:

Screenshot_20190427-104446

hockdudu avatar Apr 27 '19 08:04 hockdudu

This looks really great, thank you! The interface seems very intuitive. How many locations are allowed currently?

And am I interpreting this correctly that there is a separate entry for the detected location?

Currrupted avatar Apr 27 '19 11:04 Currrupted

There isn't a limit on how many locations are allowed. You can add as many as you want / need.

Yes, there's a separate entry for the detected location. It's shown on the top of the list. If the detected location is also a location the user added, only the detected location is displayed (so no duplicates are shown).

hockdudu avatar Apr 27 '19 12:04 hockdudu

@hockdudu Hi there, did you make any progress on this since we last heard from you? It'd be great to see any progress you made.

robinpaulson avatar Nov 29 '19 14:11 robinpaulson

@robinpaulson Sorry, I haven't had much time lastly for working on this :cry:

I'd be happy if somebody could take over my fork, as I don't really know when I'll be able to get back at working on it.

There are still some things that have to be done, namely fixing the widgets and some rare SQL exceptions, and improving the requests.

hockdudu avatar Nov 29 '19 17:11 hockdudu

No problem, cheers for the link, perhaps someone will take it up.

robinpaulson avatar Nov 30 '19 04:11 robinpaulson

That's what happens with overly ambitious features. A simple cache of the last n search terms would have been enough. :)

alensiljak avatar Jun 08 '20 10:06 alensiljak

That's what happens with overly ambitious features. A simple cache of the last n search terms would have been enough. :)

I couldn't agree more with you :sweat_smile:

I kinda bit more than I could chew on that one. Bear with me, I'm now on 3/4 of my software engineering apprenticeship. This was my first work on a real Android application (besides one or two prototypes for learning purposes).

I have more time (and experience) now for working on it, I might try to finish what I've started, but I can't promise anything.

hockdudu avatar Jun 09 '20 16:06 hockdudu

It's all a learning process. Enjoy it. :)

alensiljak avatar Jun 09 '20 16:06 alensiljak

Thank you for this fantastic app! Its almost my favorite weather app, just because its missing one feature: which is this easy location selection, or history selection. I need the feature as I travel from one city to another sometimes for work, and its weather dependent. Is it implemented already or still worked on? I just downloaded the latest from Fdroid.

Another possible example is like this app which has a places selection, as well as current location, and I can add more locations with the green plus button : Screenshot_20211028-121648_Weather

vijay-prema avatar Oct 27 '21 23:10 vijay-prema

@vijay-prema I was working on it last year, but I had to stop because of my studies.

I also believe this is an important feature for this app to have. Altough, implementing it isn't all that simple, as you require another storage backend, an sensible way to add, show and remove locations, as well as keep widgets compatible, and other small but significant issues.

Having already tried twice to finish this feature, I endear much this app and also feel someway responsible for implementing this feature.

So, as I actually do have more time now, since my apprenticeship finished this summer, I'm going to try and tackle this issue once and for all. Unless there's someone else already working on it (which would be very interesting to see how they are solving the afore-mentioned problems), I plan to be able to start working on it again in two or three weeks.

hockdudu avatar Oct 28 '21 03:10 hockdudu

So, I did get a look at the codebase and also looked back at my previous work trying to implement this feature. I also reflected on what hindered my work previously, and I came to the following conclusion:

There are too many things that must be changed to implement this feature, and it isn't feasible to do it all in one go.

Let me explain:

Implementing this feature requires major changes in the underlying storage system. At the moment, the weather data is saved as SharedPreferences (a key-value storage). But with multiple locations, it won't be possible to save it this way anymore (unless we were to do it really hacky :wink:).

The data then must be persisted elsewhere. For example: In a database or as files in the cache folder. The problem here is, this has some side effects. SharedPreferences can be read in the foreground, but database and cache must be done in the background, Android enforces it. That means, almost everywhere, we must change the code to use listeners. That is, changes in the MainActivity, at all Widgets, and in the notifications.

Additionally, the code currently parses the Json-response at multiple different locations. That must also be changed and simplified, otherwise we risk making the code much more complicated.

So, I believe these are the changes that must be done:

  1. Move weather fetching and parsing to a single place (a weather repository).
  2. Make it asynchronous, but still with SharedPreferences.
  3. Update storage engine to database or file cache (I must still compare and assert which one will be better).
  4. Maybe clean and improve the code where it may be needed.
  5. Finally implement the multiple locations feature

Splitting the changes into smaller, manageable chunks also avoids a huge merge request, which are generally more difficult to review. Additionally, smaller changes can be done faster, which avoids merge conflicts with parallel changes in the main branch.

But that's not all. There are some changes that we may need to discuss as well. I'm talking about the widgets. Currently, there's only one location, so the widgets also show this location. The question arises, when we have many locations: Which location to show? Do we have a "home" location, and use it as default, or do we make it configurable per-widget? Maybe implement the former at first, as it would be easier, and only then implement the latter? The same with the notification. And there would also be the special "GPS location". But let's discuss all this when the time comes.

I cannot give a due date when I'll be finished with it all. But I'll try and keep you informed here when I make the aforementioned changes.

hockdudu avatar Dec 03 '21 07:12 hockdudu

Love the breakdown and makes sense to me!

Just my initial thoughts on the last part, about the widgets... I'd totally do it as you said, have some kind of default or current location and the widget pulls that - for now. Or maybe add a widget later on that also shows multiple locations, e.g. cycles though - I'm not a big widget guy, so guess it depends what makes sense for the users here.

AxelTheGerman avatar Dec 03 '21 16:12 AxelTheGerman

The first batch of changes is now in the pull request https://github.com/martykan/forecastie/pull/643 :clap:

hockdudu avatar Dec 29 '21 21:12 hockdudu

Additionally, the code currently parses the Json-response at multiple different locations. That must also be changed and simplified, otherwise we risk making the code much more complicated.

So, I believe these are the changes that must be done:

  1. Move weather fetching and parsing to a single place (a weather repository).

This is wise for many reasons.

Commonly known as the Single Point of Truth principle or Don't Repeat Yourself rule.

Besides redundant duplication, one significant risk is that a change to one instance won't be applied to all.

Efficiency & modularity for the win.

Lee-Carre avatar Dec 30 '21 18:12 Lee-Carre

@hockdudu

Implementing the different parts in stages is sensible; among other benefits this enables checking for regressions and other (unexpected) issues in a graceful fashion. Refactoring, first (as you're doing), is a good start.

When comes time for tackling / addressing location-detection (especially automating it,) see #611 (comment) (punchline: query the passive location provider).

Edit: see #529.

Lee-Carre avatar Dec 30 '21 18:12 Lee-Carre

@Lee-Carre Thanks for your thoughts on this issue! You are totally right about the coding principles and modularity. It's all about a cleaner code in the end.

Thanks for the suggestion about the passive location provider. I will surely give it a closer look when the time comes.

hockdudu avatar Dec 30 '21 19:12 hockdudu

First part was merged with #643, I'll now work on the next parts.

hockdudu avatar Mar 13 '22 09:03 hockdudu