Created synchronization from QuickSettingsTileForce

[normanre]

Description

The pull request fixes the sync issue with the Force QuickSetting Tile.
mentioned here #42 (comment)

[researchxxl]

@normanre would you mind to document a short summary what the repro of the problem is and how it looks like with this pr as a solution applied?? it looks like a larger refactor to me so i am hesitant with merging right away

@nel0x could you pls help reviewing this

[normanre]

This isn’t a major refactor compared to #42. I mainly moved some duplicated logic into reusable methods.

Regarding the problem:

1. The tile currently only checks whether it is disabled when the Quick Settings bar becomes visible (onStartListening()).

2. My logic—which you have already merged—updates the tile state when the SyncthingService state changes.

3. This logic was slightly wrong, because in this situation the state can only change between STATE_ACTIVE and STATE_INACTIVE, even though it should switch to STATE_UNAVAILABLE when the force logic is enabled.

4. Therefore, I added explicit logic to check whether the tile should change to STATE_UNAVAILABLE. This logic previously existed only in onStartListening(), but is now encapsulated in setTileUnavailable().

5. This left one remaining issue: when the state of the force-tile changes without any change to the SyncthingService state (for example, switching from “Force Stop” to “No Force Stop” while no run condition is met), the tile stays in STATE_UNAVAILABLE even though it should move to STATE_INACTIVE. To solve this, the force-stop tile now notifies the schedule tile via a static method—which admittedly isn’t very nice. Any alternative would have required a much larger refactor.

6. I also did some minor refactoring to align variable naming with the rest of the codebase.

I could revert the less-clean solution described in point 5, which would leave changes 1–4 and 6 as a cleaner, self-contained improvement.

[researchxxl]

@normanre thanks for the details and well done writeup, i will check it loading the debug build :-)

[normanre]

Could you tell me how I can run the linter locally? Is it just a gradle task?

And for this result. I was expecting this could be a problem. This is the not so nice solution for 5.

I can revert that but then the button will not always be synced with the force button.

Or I could create a service binder to connect the schedule service to the force service. This would mean a bigger refactor which I wanted to avoid.

[researchxxl]

@normanre
you can run gradlew lintDebug to generate this report..
i wonder why the run for x minutes tile is correctly synced with the force start stop follow buttons on the status tab whilst the force qs tile brings it out of sync which requires the fix of this pr now
do you have a idea why the force qs tile does sync more bad than the ui itself with the run qs tile??
asking because it might give you idea on how to fix this more easily in shorter code if the ui force button code could be adapted to sync the force qs tile as well

[normanre]

The Force Sync Button never interacted directly with the schedule button before. When you look at the current release version. The force button never changes the schedule button as long as you see the quick settings menu. You have to close it and open it again so that onStartListening() refreshes the state.

With #42 the SyncThingService triggers a refresh of the schedule button even when the menu is open. In this way the force button interacts with the schedule button. Here the force button changes the SyncThingService state what in turn changes the schedule button. The problem is I made the schedule button only change between ACTIVE and INACTIVE.
With this PR I corrected that.

Now only the fix in 5. remains which is causing the linter (rightfully) to mark it.

[researchxxl]

@normanre some thoughts on the duplicate check code in QuickSettingsTileForce and QuickSettingsTileSchedule
imo we should not assume that the presence of the service equals syncthing is running but respect the state active, inactive and so on instead

root cause of the bug is we know if force run or stop is set.. this allow us to derive that syncthing is running or stopped reliably
but we do not know if syncthing is running or not when follow run is set..

you can take this info from onShouldRunDecisionChanged pls see RunConditionMonitor and SyncthingService for details
i suggest we transform onShouldRunDecisionChanged into a listener which our qs tiles then subscribe to and avoid code dup
whats your opinion on this??

[normanre]

@researchxxl Thats what I did. I am not checking not only for the presence of the service. I registered a onServiceStateChanged()-Listener on the schedule tile. This way the tile state is only active when the SyncthingService is actually active. The problem comes only from the interaction with the force tile. Here the only problem is that switching from “Force Stop” to “No Force Stop” while no run condition is met will not change the state of the SynctingService (it stays in state disabled). Therefor the schedule tile does not change its state.

Anyways, I think transforming onShouldRunDecisionChanged into a listener is a great idea. I just wanted avoid changing code outside of the tile service classes. If you are fine with that I can implement it in the following days.
Im not sure when I will find the time for that though...

[researchxxl]

Anyways, I think transforming onShouldRunDecisionChanged into a listener is a great idea.

you are welcome to proceed if you like ❤️

[normanre]

I checked the code and changing to a listender would not change anything.

The problem is, when the state of the SyncthingService does not change, but the state for the force button changes, the schedule tile has to change.

For Example:

1. You set your Syncthing to run every 3h (SyncthingService is not running)
2. You press on Fore-Run to start force running SyncthingService (SyncthingService changes to running -> Notifies QS Schedule Tile)
3. QS Schedule Tile changes to UNVAILABLE because force start is active
4. You press Fore-Run again to force stop SyncthingService (SyncthingService changes to stopped ->Notifies QS Schedule Tile)
5. QS Schedule Tile does not change because force stop is active
6. You press Fore-Run again to follow run conditions (deactivate force stop / start) (SyncthingService stays stopped and does not notify QS Schedule tile)

Thats the problem because when changing from stopped to stopped neither the syncthing service nor the RunCondtionMonitor fire any events because there is no change.

The QS Schedule Tile should in this case change from UNVAILABLE to INACTIVE eventhough the state of the SyncthingService does not change. This can also only be seen when using the force tile because when QS bar is closed the tile gets initialized in onStartListening

Adding a listener in the RunCoditionMonitor or SynchtingService would not be good solution because right now only events are triggered when the SycnthingService state changes and I would not want to change it.

My solution is now to register an OnSharedPreferenceChangeListener which only checks for this weird interaction with the force button.

[researchxxl]

My solution is now to register an OnSharedPreferenceChangeListener which only checks for this weird interaction with the force button.

okay let us try that :)

[researchxxl]

getting back to you soon :)

[researchxxl]

cleaned up the runconditionmonitor code a little bit to be able to test this better, let me update from main.. hold on.