Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Quick Setting Tiles (fixes #444) #768

Merged
merged 25 commits into from
Apr 3, 2021
Merged

Conversation

Helium314
Copy link
Contributor

@Helium314 Helium314 commented Mar 26, 2021

EDIT: (fixes #719)

This adds two QS tiles.

One is for force-starting and returning to follow run conditions.
This tile works as intended and changes label and state (enables/disabled) depending on current force-setting.
If the setting is force-stop, clicking the tile will return to follow run conditions, and then switch between run conditions and force-run on further clicks.
Only problem is that if the app is open, the force-buttons (in status) are not correctly updated.
Should tile labels reflect the current state or what will happen on click? Or rather be static like in most other tiles (e.g. simply 'force run' and use the tile state as indicator)?

The other tile starts a run when run on schedule is active. In case syncthing is currently running, the 5 min period is ended immediately.
Basically this is working, but it would be really helpful if the tile could indicate the current run status. How can I check whether syncthing is currently running?

Concerning both tiles:

  • Currently there are no proper strings (i.e. using strings.xml) for the text, will add them later.
  • again, I'm re-using random icons and have no idea which one to use or how to create any
  • In the tiles I use SharedPreferences and Context, but I feel like I have very little understanding of both. Are the uses in a correct / safe way?

icons and proper label strings missing
long click on tiles will start MainActivity
@Helium314 Helium314 changed the title Add Quick Setting Tiles Add Quick Setting Tiles (fixes #444) Mar 26, 2021
@Catfriend1 Catfriend1 self-requested a review March 26, 2021 11:43
@Catfriend1 Catfriend1 linked an issue Mar 26, 2021 that may be closed by this pull request
@Catfriend1
Copy link
Owner

Just a quick note, will review in deep later. I've already made assets "in Windows Paint" (use any program you like) and then I've converted them in to the correct format/transparency with the toolbox ( https://onlinepngtools.com/create-transparent-png ). If I have them at "xxxhdpi" size (= original self-created image source), I generate the assets Android requires (= 5 more files in lower resolutions) via this toolbox ( http://nsimage.brosteins.com/ ). Maybe this helps you a bit. Or you could a least "paint" something and fill it in as a placeholder and schematic how the final asset should look like. There are also some (license fee) free image source websites out there, check them out: e. g. https://material.io/resources/icons/?style=baseline .

@Helium314
Copy link
Contributor Author

First I think it would be good to know what should actully be in the tile icons...
The syncthing logo is probably almost necessary to see which app the tile belongs to.

Then for the 5-min start/stop some play/pause button?
logo2b

For the force / normal run condition tile I'm not sure. Maybe some kind of play button with '!'?

@Helium314
Copy link
Contributor Author

After seeing how badly my phone (Android 9) handles this:
meh
and reading https://medium.com/androiddevelopers/quick-settings-tiles-e3c22daf93a8
I decided that SVG icons are the way to go. A first test with just the syncthing logo looked promising, now I need to work on the SVG files.

The article linked above mentions "active tiles", which might be better at least for the 5-min-run tile. I'll try switching, but I'll need a hint on how to get updated on the running/stopped state of syncthing.

@Helium314
Copy link
Contributor Author

next_try
This looks better... but maybe a bit less clear that it's syncthing.

@Catfriend1
Copy link
Owner

Looks good to me.

@Helium314
Copy link
Contributor Author

Vector icons are done.

Now for the text/label: on my other (default/Android) QS tiles the text does not change, so it might be good to stick to this.

The 'force run' tile now simply has "Force run", which should be clear enough. Currently the tile (icon) is active when any 'force' is on, and can be used to disable 'force stop'. This might be confusing, especially because it is not explained anywhere (and there is not enough avablable space for much text). So maybe it would be better to set the tile state to 'unavailable' if force stop is active? Much clearer, but less functionality.

The 'start/stop 5 min' tile needs something better... right now I have "Start scheduled run", but this is long and not really clear, and wrong if ST is within a 'run' period.
Maybe switching between "run scheduled" and "running on schedule" is ok? But "running on schedule" is long, and it so far I have no Idea how I could avoid changing the tile label.

@Catfriend1
Copy link
Owner

Catfriend1 commented Mar 28, 2021

Thanks a lot , you've put some good point to discussion. I should maybe test this on the emulator soon too to get a first impression and ideas of feedback. A while ago when I also drafted qs tiles (without code) I thought about a three state switch.

Follow conditions -> force start -> force stop -> ...

this could be easily reflecting the status of the buttons in the app's status tab. well, your idea is also good.

But I somehow dislike (start/stop 5 min) as a qs tile. That's because a manual override by the user via ui is in my opinion an indefinite exceptional decision the user does make, because run conditions regularly do not apply for example. Therefore the user should remember turning the exception back to normal. Maybe we shouls have to qs tile buttons for convenience of the exceptional ;).

button 1) tri state
button 2) trigger one shot 5 minute sync

@Helium314
Copy link
Contributor Author

I also thought about start / stop / follow run conditions for the force-tile, but I somehow really dislike having to often set the "wrong" force state to get where I want. (e.g. force starting syncthing just to force stop).

As for the 2nd button I'm ok with only starting a 5 min sync.
What should happen if a 5 min sync is already running? Simply continue, or re-schedule the alarm to stop the sync 5 min after button click?

@Catfriend1 Catfriend1 added this to the v1.15.0 milestone Mar 29, 2021
@Catfriend1
Copy link
Owner

but I somehow really dislike having to often set the "wrong" force state to get where I want. (e.g. force starting syncthing just to force stop).

Yes, given the wrapper must do a lot technical things correct while starting/stopping it is ugly but I've put so many work here for reliabality of starting stopping without damaging the database I think it's fine. Simplicity for the user first here, please :-).

In case you'd like to: Four states. Conditionally.
IF b) was last THEN a) run conditions > force start > run conditions
IF a) was last THEN b) run conditions > force stop > run conditions

What should happen if a 5 min sync is already running? Simply continue, or re-schedule the alarm to stop the sync 5 min after button click?

If it's not much code (extra work, maintenance ...) we should re-schedule another 5 minutes on top of the already elapsed sync interval. If too complicated to code, just let the 5 minutes from the first button click run out.

@Helium314
Copy link
Contributor Author

In case you'd like to: Four states. Conditionally.
IF b) was last THEN a) run conditions > force start > run conditions
IF a) was last THEN b) run conditions > force stop > run conditions

Hmm... I think it might be better, but it has more potential for causing confusion. So I'd rather implement the tri-state button...

If it's not much code (extra work, maintenance ...) we should re-schedule another 5 minutes on top of the already elapsed sync interval. If too complicated to code, just let the 5 minutes from the first button click run out.

should be no problem, I think it would work by scheulding 5 minutes of running when extraBeginActiveTimeWindow is true, just need to add it to the intent coming from the QS tile

@Helium314 Helium314 marked this pull request as ready for review March 29, 2021 11:11
@Helium314
Copy link
Contributor Author

Done.
I changed the behavior regarding extraBeginActiveTimeWindow in the 'less_strict_schedule' branch (where it would be good to have it anyway). When using the "default" scheduling, it already works as intended.

I think this is ready for testing now.

Copy link
Owner

@Catfriend1 Catfriend1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will build when on pc next time :)

Copy link
Owner

@Catfriend1 Catfriend1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a recompile, I get this crash: Can we catch it somehow?

E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.github.catfriend1.syncthingandroid.debug, PID: 7718
    java.lang.RuntimeException: Unable to start service com.nutomic.syncthingandroid.service.SyncthingService@c36c3bd with Intent { cmp=com.github.catfriend1.syncthingandroid.debug/com.nutomic.syncthingandroid.service.SyncthingService }: java.util.MissingFormatArgumentException: Format specifier '%2$d'
        at android.app.ActivityThread.handleServiceArgs(ActivityThread.java:4344)
        at android.app.ActivityThread.access$1800(ActivityThread.java:237)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1951)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:223)
        at android.app.ActivityThread.main(ActivityThread.java:7656)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)
     Caused by: java.util.MissingFormatArgumentException: Format specifier '%2$d'
        at java.util.Formatter.format(Formatter.java:2529)
        at java.util.Formatter.format(Formatter.java:2459)
        at java.lang.String.format(String.java:2870)
        at com.nutomic.syncthingandroid.service.RunConditionMonitor.decideShouldRun(RunConditionMonitor.java:524)
        at com.nutomic.syncthingandroid.service.RunConditionMonitor.updateShouldRunDecision(RunConditionMonitor.java:313)
        at com.nutomic.syncthingandroid.service.RunConditionMonitor.<init>(RunConditionMonitor.java:179)
        at com.nutomic.syncthingandroid.service.SyncthingService.onStartCommand(SyncthingService.java:287)
        at android.app.ActivityThread.handleServiceArgs(ActivityThread.java:4326)
        at android.app.ActivityThread.access$1800(ActivityThread.java:237) 
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1951) 
        at android.os.Handler.dispatchMessage(Handler.java:106) 
        at android.os.Looper.loop(Looper.java:223) 
        at android.app.ActivityThread.main(ActivityThread.java:7656) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947) 

E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.github.catfriend1.syncthingandroid.debug, PID: 7800
    java.lang.RuntimeException: Unable to create service com.nutomic.syncthingandroid.service.QuickSettingsTileForce: java.lang.ClassNotFoundException: Didn't find class "com.nutomic.syncthingandroid.service.QuickSettingsTileForce" on path: DexPathList[[zip file "/data/app/~~xmMouuWJ-24snhAPA0iFcg==/com.github.catfriend1.syncthingandroid.debug-BcnCUa1P9xlz2R0Vxgx8iw==/base.apk"],nativeLibraryDirectories=[/data/app/~~xmMouuWJ-24snhAPA0iFcg==/com.github.catfriend1.syncthingandroid.debug-BcnCUa1P9xlz2R0Vxgx8iw==/lib/x86, /data/app/~~xmMouuWJ-24snhAPA0iFcg==/com.github.catfriend1.syncthingandroid.debug-BcnCUa1P9xlz2R0Vxgx8iw==/base.apk!/lib/x86, /system/lib, /system_ext/lib]]
        at android.app.ActivityThread.handleCreateService(ActivityThread.java:4204)
        at android.app.ActivityThread.access$1500(ActivityThread.java:237)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1932)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:223)
        at android.app.ActivityThread.main(ActivityThread.java:7656)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)
     Caused by: java.lang.ClassNotFoundException: Didn't find class "com.nutomic.syncthingandroid.service.QuickSettingsTileForce" on path: DexPathList[[zip file "/data/app/~~xmMouuWJ-24snhAPA0iFcg==/com.github.catfriend1.syncthingandroid.debug-BcnCUa1P9xlz2R0Vxgx8iw==/base.apk"],nativeLibraryDirectories=[/data/app/~~xmMouuWJ-24snhAPA0iFcg==/com.github.catfriend1.syncthingandroid.debug-BcnCUa1P9xlz2R0Vxgx8iw==/lib/x86, /data/app/~~xmMouuWJ-24snhAPA0iFcg==/com.github.catfriend1.syncthingandroid.debug-BcnCUa1P9xlz2R0Vxgx8iw==/base.apk!/lib/x86, /system/lib, /system_ext/lib]]
        at dalvik.system.BaseDexClassLoader.findClass(BaseDexClassLoader.java:207)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:379)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:312)
        at android.app.AppComponentFactory.instantiateService(AppComponentFactory.java:129)
        at androidx.core.app.CoreComponentFactory.instantiateService(CoreComponentFactory.java:75)
        at android.app.ActivityThread.handleCreateService(ActivityThread.java:4183)
        at android.app.ActivityThread.access$1500(ActivityThread.java:237) 
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1932) 
        at android.os.Handler.dispatchMessage(Handler.java:106) 
        at android.os.Looper.loop(Looper.java:223) 
        at android.app.ActivityThread.main(ActivityThread.java:7656) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947) 

@Catfriend1
Copy link
Owner

I've found a bug: When I force start/stop the buttons status on the status tab do not reflect the new status. See ( https://user-images.githubusercontent.com/16361913/113125597-0de5d000-9217-11eb-9a05-f2a4b1ab41e2.mp4 )

If the app is not running, the qs tile to force/start and the tile to run for 5 minutes do nothing. This could be left as-is but nicer would be - if possible - to grey them out or at least display a message "open the app first".

@@ -1084,4 +1084,11 @@ Please report any problems you encounter via Github.</string>
<string name="custom_sync_conditions_description">Allows you to specify exceptions to the global run conditions.</string>
<string name="custom_sync_conditions_dialog">Specify sync conditions</string>

<!-- Quick Setting icon names and labels -->
<string name="qs_force_label">Force run</string>
<string name="qs_force_running">Force running</string>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be "forced_to_stop" / "forced_to_run"

@Catfriend1
Copy link
Owner

Ok, I've merged the other two PR's. Now everything seems to work right. Maybe a merging problem previously was in my beta branch?! Sorry.

The "force" button now is tri-state. Yeehaaa, great! :-)

Could you please do a "play"/"stop" image in the right corner of the qs tile? I'm not familiar with SVG's. If I have the qs bar not expanded, I cannot distinguish which state the "force on" (blue tile) means.

image
image
image

@Catfriend1 Catfriend1 linked an issue Apr 2, 2021 that may be closed by this pull request
@Catfriend1
Copy link
Owner

@Helium314 I'm currently dreaming of this, do you think this is do-able in SVG? I hope you still have time to do this, would have done this myself but don't have any SVG tools at hand.

image

Copy link
Owner

@Catfriend1 Catfriend1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SVG optimization : forced to start image, force to stop image

@Catfriend1 Catfriend1 removed the testing Community test case - Notice: Consists of maybe unstable syncthing version or code label Apr 2, 2021
@Helium314
Copy link
Contributor Author

Ok, I've merged the other two PR's. Now everything seems to work right. Maybe a merging problem previously was in my beta branch?! Sorry.

Maybe it really didn't work before... I didn't test this thoroughly as a 'standalone' change, most of the time I had it together with the other 2 PRs.

Could you please do a "play"/"stop" image in the right corner of the qs tile? I'm not familiar with SVG's. If I have the qs bar not expanded, I cannot distinguish which state the "force on" (blue tile) means.

@Helium314 I'm currently dreaming of this, do you think this is do-able in SVG? I hope you still have time to do this, would have done this myself but don't have any SVG tools at hand.

Will do, it should not be much work. Although color is not possible, this is a limitation of the QS tiles.

Actually the easiest way I found for working with SVGs is online: https://svg-edit.github.io/svgedit/dist/editor/index.html
Hardest part was learning that Android doesen't support everything that can be done in SVG 😐

@Helium314
Copy link
Contributor Author

Is this big enough for you?

hm

@Catfriend1
Copy link
Owner

Catfriend1 commented Apr 2, 2021

yeah, perfectly done in my opinion. Should we merge it now?

@Helium314
Copy link
Contributor Author

yeah, perfectly done in my opinion. Should we merge it now?

I guess it's done.
Maybe except for the bad behavior you encountered... but this has not been the case since you merged the other PRs, right?

@Catfriend1
Copy link
Owner

Correct. So I'll merge it next time on PC :-).

Copy link
Owner

@Catfriend1 Catfriend1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's done.

@Helium314
Copy link
Contributor Author

Should I add some comments?
The QS tiles look rather easy to understand, but I am not neutral on this 😉

The maybe least obvious part is when searching through services whether syncthing is active, so the tiles are only enabled if the app is running

@Catfriend1
Copy link
Owner

If you like, yes please add comments. It will help us both maintain it in the future. I've already come to the point where I found my past me added a useful comment how something worked without it I'd had to reread the whole thing again using time.

@Catfriend1 Catfriend1 merged commit 4d566a7 into Catfriend1:main Apr 3, 2021
@Catfriend1
Copy link
Owner

Thank you very much !!! :-)

@Helium314 Helium314 deleted the qs_tiles branch February 17, 2022 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run according to schedule - manual override Add quick settings tile for forced start/stop
2 participants