Setting to run when plugged in?

[bradmont]

SyncThing seems to burn through quite a bit of battery on my phone; it might be useful to have a "sync when plugged in" setting, so the service launches when the phone is plugged in, then shuts down when unplugged.

[Nutomic]

Thanks for reminding me that we can just shut it down, instead of using a (non-existant) pause funcionalitiy.

But I would do it the other way round: Have options "disable sync on battery" and "disable sync on mobile data", and the service always runs by default (but I'll leave the exit button in). What do you think about that?

[bradmont]

I don't think there's really a semantic or technical difference between the two, so yes, this would certainly satisfy the use case.

I guess the difference is that you want to communicate to the user in terms of the "normal" way to use the app is to have it always running. Makes sense to me.

[Nutomic]

I mostly think it would be complicated to combine "sync when plugged in" with "pause on mobile data", that's all ;)

[bradmont]

Oh, I see what you mean. Maybe calling the options "Sync only on power" and "Sync only on wifi" makes it easier to visualise (or "disable sync on battery" and "disable sync on mobile data"). Either way, though, you're listening for the same system broadcasts (system start, power state change, network state change), and when one is received, checking if the current system state (plugged in, wifi, etc) matches our conditions for running, then starting or stopping the service accordingly.

[Nutomic]

Yeah right, it's only about semantics. And now I'm torn between "sync only on ..." and "disable sync on ...", thanks for that :D

[bradmont]

Haha, no problem. My hunch would be "sync only on," but honestly, just code it, and if it's confusing to the user, it just takes two lines in strings.xml and a couple of !s to switch. ;)

[Nutomic]

Haha sure, I'll make an option for the options text :P

[bradmont]

You should also add an option for whether the user wants to be able to change that option from the settings screen, since, y'know, some users like a simple UI, and some like confiugrability.

[Nutomic]

We better stop this now, or I'll have to rewrite Android from scratch in a bit.

[bradmont]

So I adjusted my rescan and reconnect intervals in the settings, changing both to 3600 (one hour), and that has essentially fixed the battery drain. For me this is a reasonable long-term solution, since I don't really need syncs on my phone to be real time. I don't know how others feel about that, but it might be worth considering changing the default values on the android client, and then maybe adding a "force sync now" function or widget that temporarily sets them back to a minute.

[Nutomic]

"Force sync" needs upstream support, which isn't there yet, afaik.

@calmh For the future, it would be nice if updated files could be supplied via API, and scanning disabled. That way, Java (using Android APIs) could handle change detection and just pass it on. That might also solve problems with inotify on various platforms (as long as there is a native GUI handling it, so full scans would only be a fallback).

[bradmont]

I don't think it would require upstream changes; maybe call it "sync now" instead of "force sync", but all it would need to do is change "rescan interval" to one second (or maybe five to be safe), then change it back a few seconds later, once the scan has been triggered. It would have to be an android UI button, but the rest calls to change the configuration are all there already.

[Nutomic]

What you propose is a quite ugly hack. For one, we can't make sure that exactly one scan is performed. Also, I might have to change this soon, when better ways for handling this are added.

All in all, I'd rather work on things that can be implemented cleanly, and won't be useless any time soon (trust me, there's enough of that).

[calmh]

Yeah... I'm not opposed to adding an API for "rescan these files now: ..." which could be used in combination with a "do not rescan automatically" option.

[icaruseffect]

It should be considered that sync via mobile network should be off by default. Otherwise the could be bad suprises for people with big repositorys. Thus i would prefer checkboxes for

• [ ] sync on mobile data
• [ ] sync only when plugged in

[Nutomic]

The main problem right now is that there's no pause functionality upstream.

calmh apparently started it here, so I'll wait until that's finished, rather than going for ugly hacks.

[bradmont]

Hey Numotic,

I'm a little curious as to why you consider it bad design to just shut down the service when the user doesn't want it using data or battery power? I feel like it's a pretty logical and relatively clean solution, and I think it would probably better serve the goals of saving data and battery power to do it that way. I mean, I totally understand wanting to do it right so as not to have to recode features in the future, but I don't really see the downside to just keeping it that way long term.

The reason I ask is because if I get some free time in the next week or so I might take the time to submit a pull request, but if you're not interested in going that route, I won't bother.

[Nutomic]

There are two problems:

1. The web GUI is coming from the syncthing binary. If we stop it, it won't be reachable.
2. I do lots of API access in the "native-gui" branch, which would all need special handling in case the service is not reachable.

That said, it just occured to me how it could be done somewhat cleanly:

• On device boot/power connect/wifi connect, we check that all conditions are met, and if so, call startService
• While the service is running, if power disconnect/wifi disconnect is detected and that should pause the sync, call stopService AND stop the activity
• when the activity is stated, the service is also started, no matter what
• once syncthing has pause functionality, we can just drop that in instead of startService/stopService

Is that an acceptable (temporary) compromise?

If you do implement this (much appreciated), please do so on top of the "native-gui" branch, otherwise merging will be a pain.

[bradmont]

Yes, that seems pretty much along the same lines as I was thinking. If the user opens the app while the service is off, rather than simply starting the service it might be better to ask whether they want to start it, especially if it's off because the phone is on mobile data. Accidents do happen, after all.

I'm not sure if I agree with your third point, though. I actually think shutting down the service is a better solution than pausing the repos, for a couple of reasons:

• From a user experience point of view, the long-running notification for the foreground service should signal to the user that the app is doing something, rather than simply that the service is hanging out in the background. Again, I'm thinking of the use case of wanting Syncthing off on mobile data and battery. If the user sees the notification icon, it's an immediate reminder that sync is on and consuming resources; maybe they forgot to turn it off after starting it manually. If the foreground service is always present, the user won't have that reminder.
• Looking at the pause branch you linked, it appers all it would do is pause the individual repos, but keep the node connections active. This will continue to use data and CPU time. Of course, it may just be that there hasn't been any code written for pausing node connections, but at that point the service isn't doing anything other than making the UI available. It seems reasonable to me to not have the UI available if the service isn't running, especially if we explicitly start the service whenever the user opens the Activity.
• It's minor, but keeping an inactive service is going to consume some resources, even if it's just a little bit of ram. This probably isn't a big deal, but on low-spec systems, every little bit counts.

Thoughts?

Regarding the native-gui branch, how close are you to having it replace the webview?

[Nutomic]

• I think you misunderstood, I mean if syncthing is paused, the service should be stopped and the notification removed. But if you manually open the app while paused, the service should be started again so the GUI works (this is only if we don't do it the way I describe it below).
• @calmh Can you tell if you are planning for a "stop all connections" mode in addition to the "pause repo" setting?
• That's true. Though Android should kill services it deems unnecessary on its own (This will be more likely when we can remove the permanent notification). The other option is to check if the API is available whenever it is called. This might be possible just in the RestApi class, and/or GetTast/PostTask classes, by not executing if the binary is stopped. We'd have to try if that works.

[bradmont]

Hmm, I think we're either saying the same thing with different words, or using common vocabulary to refer to different things. By "the service should be stopped and the notification removed" do you mean that the syncthing binary will be terminated, or just that the android.app.Service will end, removing the notification, but leaving the binary running? I'm not sure that the latter is actually possible to do reliably; that's the whole point of the foreground service and showing the notification -- it signals to the system that the process should not be killed to reclaim its memory; without it, a long-running process will almost surely be killed by the OS. It's also frowned upon by google to hide running services like that. :/

[Nutomic]

Yeah the service should be stopped of course.

So we were saying exactly the same :D

[Nutomic]

Here's the upstream issue for pause: calmh/syncthing#215

[bradmont]

Hey @Nutomic , I totally didn't notice your refactoring branch until just now and went ahead and implemented this on master today. I guess there's probably not much use for it at this point, but I uploaded my code to a fork here: https://github.com/bradmont/syncthing-android if you're interested.

[Nutomic]

Yours looks pretty similar to my implementation (but you also allow enable/disable autostart, which I have always enabled). Also, I made it somewhat more complicated by using intent actions instead of starting/stopping the service, but I don't remember why.

What do you use the android.intent.action.MY_PACKAGE_REPLACED broadcast for?

Anyway, this is unfortunately too late to add (I would only put such a big change into a new minor version, and porting this to the refactoring branch doesn't make much sense).

[bradmont]

Yeah, I didn't bother to check the bug tracker before implementing it. Had I seen your code, I wouldn't have bothered. Oh well, I'll keep on my phone it until your implementation makes its way to Google Play. ;)

the MY_PACKAGE_REPLACED broadcast is called after the apk gets updated. So when the app is killed during the update, this allows it to restart immediately. It pretty much serves the same purpose as BOOT_COMPLETED; any time you set an alarm or service on boot, you should also do it on package replace. I recall a rather popular alarm clock app once forgot to do this, and caused major problems for their users when everybody's alarms failed to go off the morning after a google play update...

[Zillode]

Please use positive sentences for check boxes because negated ones will confuse people. It also makes the options more consistent. So no "disable" or "don't" options, ty:)

[Nutomic]

Yeah I don't like those either.

Maybe "sync on battery" and "sync on mobile data"?