syncthing-android icon indicating copy to clipboard operation
syncthing-android copied to clipboard

Compact persistent notification

Open KopfKrieg opened this issue 6 years ago • 15 comments

I've seen all the other issues complaining about the persistent notification (#981, #994, #998, #1021), but I understand it's currently necessary (and, honestly, I don't have a problem with it. I like to know what's running in the backround).

Therefore, I suggest to make use of the compact message feature. For example, the app "Smarter Wi-Fi Manager" already supports this (tested on Lineage OS 15.1 / Android 8.1). It will look like this:

compact-notification

It will only show a very minimalistic notification. I think that's a good compromise and would be useful for Syncthing on Android too.

The notification itself can be expanded if necessary. I'm not sure how exactly this is implemented, and I couldn't find an answer in the developer's guide (maybe I just didn't see it), but I hope someone can figure this one out.

KopfKrieg avatar Apr 06 '18 15:04 KopfKrieg

You can do this yourself by setting the persistent notification channel to low.

This should (IMHO) be the default thought.

wweich avatar Apr 06 '18 16:04 wweich

You can do this yourself by setting the persistent notification channel to low.

This only removes the icon from the top bar, but doesn't compress the notification itself into something smaller. And of course, even if there's the option to do this, having it as default would still be nice :)

Default priority (on my system, but I didn't change anything about notifications) is "Medium". But that might be a bug, because I've found posts of people who can't set their app's notification priority below medium.

KopfKrieg avatar Apr 06 '18 16:04 KopfKrieg

@capi did your pr fix this by any chance?

AudriusButkevicius avatar Apr 06 '18 18:04 AudriusButkevicius

@AudriusButkevicius My PR separates the channels, so you can have different priorities for "deactivated" and "active" (I like to see when it's active, but hide it when in background). It is still up to the user to change the channel's priority of the individual channels.

I personally leave it at "off", so that it is not shown at all. When I set the priority to "low", it is compressed. But I noticed that in this case it works worse than with "off", because the service still seems to be killed, since startForeground() requires a medium priority notification.

Also, due to the requirement of startForeground(), I was unable to set the default to "low". But as I said, it seems the user can still reduce the channel's priority manually afterwards.

If I turn it "off" the "Application is using power in the background" annoying notification of Android 8+ is triggered. But I have set the notification priority of this system notification to "low".

I am on LineageOS 15.1, there I can set this priority to "low". If this is due to LineageOS or due to 8.1, I don't know. Also, like I said, if you see the background service being killed with the notification's priority set to "low", you might have a try with "OFF" instead.

Some research on the web also indicates, that there are changes between 8.0 and 8.1 what you can do with the individual channels, because Google itself found that 8.0's behavior is unfortunate in this area. At least in 8.1 it seems the user can hide the "using power in the background" notification without workaround apps that periodically mute exactly this notification for two hours.

capi avatar Apr 06 '18 18:04 capi

What I also noticed: if you change the notification's priority, the effect (if it is a collapsed or expanded notification) takes place only after the notification is re-created by Syncthing again, e.g. by changing between "active" and "deactived" (after my PR).

capi avatar Apr 06 '18 18:04 capi

Edit: Restarting syncthing after setting priority to low allows the notification to be collapsed. I totally missed that previously.

KopfKrieg avatar Apr 06 '18 20:04 KopfKrieg

@KopfKrieg I don't know of any way to actually improve the current behavior which requires the manual configuration of the priority. The requirement to restart the app in order to have the icon be collapsed is not good, but that seems more a limitation of the Android notification system at the moment and I don't know any way around it.

Does this solve this issue and we can close it?

capi avatar Apr 06 '18 21:04 capi

I don't know of any way to actually improve the current behavior which requires the manual configuration of the priority.

Creating the foreground service notification with low priority would fix my issue. According to the documentation this should be possible:

To request that your service run in the foreground, call startForeground(). This method takes two parameters: an integer that uniquely identifies the notification and the Notification for the status bar. The notification must have a priority of PRIORITY_LOW or higher.


@capi:

But I noticed that in this case it works worse than with "off", because the service still seems to be killed, since startForeground() requires a medium priority notification.

This shouldn't be the case. Documentation says low priority is allowed.

Also, like I said, if you see the background service being killed with the notification's priority set to "low", you might have a try with "OFF" instead.

Huh? Low should leave the service running, while Off might allow Android to kill it faster if necessary.

I'm on LineageOS 15.1 too, and here I don't have any problems with priority set low.


I found a (for me) confusing part in this function: If I understand this correctly, the default priority is already low?!

This is confusing, especially because after installing the app the priority was set to medium on my device (and I've done a completely clean install [wiped everything] only a few days ago, so there shouldn't be any leftovers from previous installations).

Could someone explain why this is the case? Because, if it's already set to low, I'd just close the issue.

KopfKrieg avatar Apr 08 '18 13:04 KopfKrieg

@KopfKrieg You are right, the "being killed" on PRIORITY_LOW must have been some artifacts during development. I have tested it with PRIORITY_LOW for the weekend, works as expected, like you say. And I must have mixed up some priorities in my head.

I have not really debugged the existing parts when splitting the channels. I can imagine that the if in line 122 (https://github.com/syncthing/syncthing-android/blob/7194e25a5cacff14b3df85f4cf2e17d415a20038/app/src/main/java/com/nutomic/syncthingandroid/service/NotificationHandler.java#L122-L123) does not trigger. And I don't know what a notification channel with IMPORTANCE_MIN does with a default priority notification.

Unfortunately this method is a mix of old and new behavior and has grown with the Android versions. I'll need to work through it what it does without default settings.

capi avatar Apr 08 '18 18:04 capi

I can imagine that the if in line 122 does not trigger.

Unfortunately, I don't know either.

I'll need to work through it what it does without default settings.

Sure, no problem.


Edit: Documentation says the hierarchy of priorities is min < low < default < high < max. I don't know, but maybe PRIORITY_MIN falls back to PRIORITY_DEFAULT because foreground services can't be started with a PRIORITY_MIN as PRIORITY_LOW is required as minimum? Just a guess, though.

KopfKrieg avatar Apr 09 '18 16:04 KopfKrieg

Is there a reason why this notification is needed at all? I realize that Oreo doesn't allow background apps to listen for implicit broadcasts anymore, but the proper solution to that is not to just run a foreground service 24/7. Instead what should happen is that Syncthing should use the JobScheduler to tell Android to run a sync the next time Wi-Fi is available (and/or power is connected).

Ajedi32 avatar Apr 23 '18 15:04 Ajedi32

@Ajedi32 you are completely right about that, and hopefully in the future this can be realized. It is a little bit complicated with backwards compatibility, since JobScheduler is available only since API level 21, we currently support back to API level 14.

So the short-term workaround was to have the service run with foreground-priority, which on the other hand requires the notification.

capi avatar Apr 23 '18 22:04 capi

Conclusion of the other issues: JobScheduler should be checked if it could be implemented Android version specific. Faq should be updated

Catfriend1 avatar May 07 '18 06:05 Catfriend1

I think it advisable to add an option to run without persistent notifications in root mode

qianbinbin avatar Feb 15 '22 11:02 qianbinbin

FYI, it looks like the new recommended way to do this is with WorkManager. It's part of Android Jetpack and should have better comparability with older API levels.

Ajedi32 avatar Feb 15 '22 15:02 Ajedi32