Add an option to disable auto-refresh in onResume

[AndyScherzinger]

This came in via comments on Google Play

Please add an option to disable auto-refresh every time the app becomes visible. This is very annoying on slow connections.

This could be done quite easily with another general option so we do not auto-refresh and only refresh based on the refresh rate that has been configured. This also happens when you switch accounts via action bar.

[willlunniss]

At the moment the calls to async tasks are in onResume, which is what causes a lot of the extra refreshes.

[AndyScherzinger]

So should we implement this or not?

Meaning does this get "better" when we have the code refactored in a way that it doesn't call the tasks in onResume? Or should be have this kind of refreshes configurable via settings in general?

[willlunniss]

It will get better, and I think we can improve it by only initiating the automatic forground sync if it is needed (i.e. haven't done a manual or automatic forground sync recently e.g. < 10 minutes) which should solve switching back to a previously checked account, rotation and exiting/starting the app.

[AndyScherzinger]

We could also compare it to the auto synchronization setting and only do an automatic foreground sync if 50% of the time until the next auto sync would be triggered has passed. Thus the automatic foreground sync would rather match the users update frequency wishes. In case the user has deactivated auto sync then we shouldn't do any automatic foreground syncs and leave it to the users choice to explicitly trigger a refresh manually.

What do you think? Would this be a more intuitive solution? (Even though is is slightly more coding work to be done)

[willlunniss]

I don't think it should be like that. When you open the app up you want it to show up to date information. If you have a high background sync period then even more reason to update when you actively use the app, rather than less reason. What we want to avoid is updating the data when it is unlikely that it has changed which is what happens at the moment by updating it a bit too much.

My view is, cut down on unneeded syncs by not doing it if we did one very recently and as it won't hurt, add a general option that is true by default to automatically sync on startup which users can turn off if they want.

[AndyScherzinger]

You are right. Haven't looked at it that way. So there should be a threshold for these automatic foreground syncs that is fixed. IMHO this threshold could be fairly high since users with high frequency changes might set their auto syncs to a 15 minutes or an hour. I do not really have a feeling for the amount of time to wait but I guess it should be an hour or something like that. 10 to 15 minutes might be a bit short, since users who check their stats "manually" every 15 minutes should set the auto sync to that or manually trigger the sync.

Any idea for the threshold?

[willlunniss]

IMHO it should be an upper bound on what we consider a session of using the app. Remember this is for automatic forground syncing in onCreate so it will only be triggered if you are actively using the app. For example I have my sync to an hour IIRC, but whenever I release an update to an app I often check the app much more frequently for new ratings and comments. I'm expressing my desire to see up to date info by opening the app.

I think we should go for a fairly low interval of ~15 min, maybe 30, with an option to turn it off for those that really want. If we make it too high then there is no point letting you turn it off, and it will lead to users having to press refresh rather than have the app manage that for you. Any other views.

[AndyScherzinger]

Agreed. So let's say 15 minutes and general setting to deactivate it completely?

[willlunniss]

Yes, it should probably be done at the same time as getting rid of all AysncTask creation in onResume and chaning them to be static inner classes which we explicitly attach and detach (#110) to minimise work and testing.

[nelenkov]

It should also be a good idea to split the loading in two parts: show latest data from DB for immediate feedback, and the fetch remote data, if necessary/configured, etc.

[nelenkov]

This is now available in dev/. Stats are only refreshed if at least 15 mins have passed since last refresh; 5 mins for comments. Those values are hard-coded ATM. Please test. Do we really need a setting to toggle this behaviour? You can just press refresh to have it re-load data.

[willlunniss]

I think it works well as is, no need to have a toggle to turn it off.

There was a problem with the comments not saving the update date, and it needs to be per package as when you view comments, it only fetches the comments for that package.

[AndyScherzinger]

I also don't see a reason for beeing able to change the values/timeframes for the refreshes since you already mentioned that people could still press refresh.

Works for me!

[AndyScherzinger]

Why and who closed milestone 2.3?

We haven't released 2.3 and this issue here is assigned to 2.4 whele the next release with the changes present on the dev branch would be 2.3. Right?

[willlunniss]

I went through all the issues a while ago and delayed a few to 2.4 as @nelenkov and I have been quite busy recently and I wasn't sure how much we would get into the next release and therefore wanted to focus it down to what had to be done for the new console related stuff. A side effect of that was that the milestone got closed when the remaining 2.3 issues were fixed. However, @nelenkov has had some free time the last few days so some of the issue got fixed earlier which is hardly a problem.

If we have time we can bring a few more issues forward, but its always best to get a smaller number of issue properly fixed, rather than loads of them partially done. The next release has a massive set of changes (220 commits at the moment). I'll do another pass of the open issues.

[AndyScherzinger]

Sounds find to me. I also have a lot to do at work and can't spent much time (recently none) on this project. Working quite some over hours atm.

So looking at the set of changes it might also be a good idea to open up a new feature branch to get the dev branch stable for a release and work on new features on the feature branch, right?

I'll see if I can re-open 2.3 and assign the commits of the dev branch to 2.3 and 2.4 could then be used for new development.

Any opinions on how we want to handle/use the milestones on gh?

Edit: 2.3 is already reopened :)

[AndyScherzinger]

Issue #179 question: should this issue be closed since it is 'fixed' ?

[willlunniss]

So looking at the set of changes it might also be a good idea to open up a new feature branch to get the dev branch stable for a release and work on new features on the feature branch, right?

We can do if they are big/multi-commit changes that won't get done in one go, but I don't think we have any of those planned at the moment.

Any opinions on how we want to handle/use the milestones on gh?

I think its just useful for putting things into a soon (the next version), after (the version after), not yet categories (none). Doesn't really make much difference.

Yes it apears to be now.

[nelenkov]

I knew I was missing something... Storing the last refresh time per package indeed makes more sense. However, if you have a lot of apps (and possibly multiple developer accounts), this would result in a whole lot of preferences. Might be better to store the last update time in the DB. Maybe add a column to the appinfo table?

[willlunniss]

Yes that might be better and more along the lines of how other things work. We should have a look over the prefs and open issues to see if there is anything else that should be moved over.

[nelenkov]

Pretty much everything that can have multiple values is probably better off in the DB. Account status (hidden, AdMob enabled, etc.) comes up first, but that might be too big a change for 2.3 since it will require some migration code. If we do another (couple of?) beta(s) it could be OK though. Assuming people are actually using thees things, of course. Do we know how many times Beta1 has been downloaded?

[nelenkov]

29 downloads, not too bad, but not much feedback.

[willlunniss]

I guess that means it's working perfectly then :-)

Well we should be able to write the migration code in a helper class, and call it during the db upgrade process. It will be boring to write, but shouldn't cause any problems. Then do one more beta and release.

Anyone seen any info on googles planned timeframe for finishing off the new console?

[willlunniss]

Not quite fixed, my change blocks manual comment refreshing, I'll fix that in the morning.

[willlunniss]

Still not 100%, load more comments doesn't work.

[nelenkov]

Ah, that's one I haven't actually tested. Will try to have a look at it tomorrow.

[nelenkov]

Should be fixed now.

[willlunniss]

Nearly there, however the 'Load more comments' box only shows after a refresh, not after a db cache load. If reload comments, go to app list, and go back to comments you have to press refresh before you can load more.

On a side note, google has started to fill in the user names based on G+ data, so the app is starting to shown things other than 'A Google User'.

[willlunniss]

Another minor bug (more from an assumption made previously), if you rotate the screen when viewing AdMob, you loose the total which is displayed next to the timeframe at the top until you press refresh again.

[nelenkov]

The comments one is a bit tricky. Should work now, but I probably need to have another look.

Calculated values lost on rotation might show up elsewhere as well, because none of the activities that I changed originally saved state (via onSaveInstanceState(), etc.). Might be worth checking throughout the app.

[nelenkov]

Now saving AdMob overall stats as well, should fix the missing total, etc. after rotation.

[willlunniss]

Thats working now. Thanks.