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

Enhancement request for per-folder(device) sync conditions #110

Closed
Catfriend1 opened this issue Oct 23, 2018 · 16 comments
Closed

Enhancement request for per-folder(device) sync conditions #110

Catfriend1 opened this issue Oct 23, 2018 · 16 comments
Assignees

Comments

@Catfriend1
Copy link
Owner

Quote from @cdrfun at ( #57 (comment) )

Really nice work!
I had some time to test today, and here is what I've found. (If needed I'll create a separate issue(s)). Some of my labels might not be correct, because I'm using the German version. I use numbers for better referencing, not to indicate any importance:

  1. Folder-Sync condition Start at selected Wifi can't be activated when a option page for an folder is opened where start with wifi is activated. When deactivating/activating the option start with wifi, the option Start at selected Wifi can be set.
  2. Currently folder sync conditions can only limit the folders sync conditions compared to the global conditions. I understand that it is good to have some kind of a global limit. But this puts the user in a tricky situation when he wants one folder to "always sync". He has to max out the global conditions and limit all folders bit the always sync folder. additionally he has to remember to limit all new folders, or he will use more traffic than he thought. Perhaps a separation of syncthing starting conditions and default folder conditions would be a solution.
  3. The Folder-Sync condition dialog has a checkmark in the upper right. This is not necessary, because changes are saved when going back (either using the arrow in the upper left or the back button). Additionally when pressing the checkmark the folder options dialog is revealed and here is the trash icon at the same position. This is not consistent.
  4. When wifi connection is lost, and some folders are configured to run on mobile, syncthing seems to restart completely instead of just pausing non mobile folders. When the wifi connection is regained, syncthing just unpauses the folders - which is correct.
@Catfriend1
Copy link
Owner Author

@cdrfun As stated in the other issue > added to my development plan. :)
v0.14.51.8 / 4171 should have point (1) fixed already. I did find another UI glitch that does not impact functionality if you enable "sync on wifi" and "wifi whitelist" was disabled before, the wifi SSIDs must be re-enabled manually by toggling each SSID to enabled. So far I didn't find a reproducer that leaves some switch greyed out wrongly.

@cdrfun
Copy link

cdrfun commented Oct 25, 2018

In v0.14.51.8 / 4171 I can definitely reproduce (1). I'll have a look again when the next apk is released.

I'd like to suggest another enchantment request:

  1. Make battery options available in Folder-Sync conditions.

@Catfriend1
Copy link
Owner Author

Ok noted. Thanks for checking again.

@Catfriend1
Copy link
Owner Author

Just thinking about how to solve this... Maybe a global should be there to say "use global conditions" or "use individual conditions". If we only use individual conditions (4) would automatically be solved as that's coming from the timely manner Android notifies us of connection changes one by one . Later, we could calculate if all folders are paused and then derive the decision to stop syncthing from it.

@Catfriend1
Copy link
Owner Author

@cdrfun How do you reproduce (1)?
Me:
image
=>
image
=>
image
=>
image
=>
image

@Catfriend1
Copy link
Owner Author

Note: (3) was fixed in PR #114

@Catfriend1
Copy link
Owner Author

Long-term improvment noted: (2) + (4)

Catfriend1 pushed a commit that referenced this issue Oct 27, 2018
* Fix issue 110 (3) - remove checkmark

in per object sync conditions dialog

* Fix UI glitches, feedback issue #110 (1)

* Stop onscreen keyboard popping up

when FolderActivity starts

* RunConditionMonitor - Add more logging

* Always save UI state back to prefs

in SyncConditionsActivity until we abandon global run conditions.
@Catfriend1
Copy link
Owner Author

Catfriend1 commented Oct 27, 2018

New release containig the fixes for (1) + (3): v0.14.51.11

@cdrfun
Copy link

cdrfun commented Oct 29, 2018

Great work 👍
Just to keep track I'll list the remaining issues:

  1. Currently folder sync conditions can only limit the folders sync conditions compared to the global conditions. I understand that it is good to have some kind of a global limit. But this puts the user in a tricky situation when he wants one folder to "always sync". He has to max out the global conditions and limit all folders bit the always sync folder. additionally he has to remember to limit all new folders, or he will use more traffic than he thought. Perhaps a separation of syncthing starting conditions and default folder conditions would be a solution.
  2. When wifi connection is lost, and some folders are configured to run on mobile, syncthing seems to restart completely instead of just pausing non mobile folders. When the wifi connection is regained, syncthing just unpauses the folders - which is correct.
  3. Make battery options available in Folder-Sync conditions.

@Catfriend1
Copy link
Owner Author

Personal Dev note: I need to do this after refactoring ConfigXml module.

Catfriend1 added a commit that referenced this issue Dec 1, 2018
@Catfriend1 Catfriend1 self-assigned this Dec 1, 2018
@Catfriend1
Copy link
Owner Author

So, I'm still on it figuring out a better implementation which can handle the per folder/device sync in a "whitelist thinking" manner. The first (great) piece of work is nearly done, as now, the wrapper is able to write the correct folder/device paused (or not paused) state into the config directly when syncthing is not running. This make it more streamlined as it before had to start the sync core and then quickly apply the paused/unpaused flag. Since we are now able to determine and set the right state BEFORE starting the sync core, we have a lot of possibilities to improve further.
In the second piece of work, I'll refactor the run conditions that they are evaluated and applied when the sync core is not running, so pausing and unpausing won't be triggered on the sync core startup but (as it should be) in RunConditionMonitor.
Stay tuned ...

@Catfriend1 Catfriend1 added this to the 0.14.55 milestone Dec 21, 2018
@Catfriend1
Copy link
Owner Author

Related FR: "#145 Always make individual sync conditions UI available"

Catfriend1 pushed a commit that referenced this issue Dec 22, 2018
Changelog:
- "Use default folder path given in config.xml" (#101)
- "IllegalStateException: Fragment already added" (#108)
- "Enhancement request for per-folder(device) sync conditions" (#110)
- "NPE crash after key and config regeneration" (#141)
- "Adjust the folder icon to show if it's send/receive or both" (#143)
- "CPU percentage is not shown on the status tab" (#144)
- "Always make individual sync conditions UI available" (#145)
- "IntroducedBy deviceID lost on config change through wrapper UI" (#146)
- "Wrapper doesn't use the same syntax as syncthing core's web UI for device addresses" (#147)
- "Syncthing wrapper "emergency" shutdown on native binary crash doesn't work" (#148)

Commits:

* WIP

* WIP

* Get folder list and paused setting when syncthing is not running

Preparation to solve #110

* Fix NPE in DeviceListFragment#DEVICES_COMPARATOR

* Remove blank line

* Add ConfigXml#getDevices and comparator

Make ConfigXml#saveChanges public

* SyncthingService evaluates per folder/device

sync conditions when syncthing is not running via ConfigXml

* Fix typos and add stubs

* Fix build errors

* DEBUG - Always run syncthing binary

* Fix NPE at RunConditionMonitor pointer

* Add setFolderPause, setDevicePause

to ConfigXml

* Improve logging

* Remove test mode

* Better log levels

* Make ConfigXml#updateIfNeeded private

* Remove SyncthingService#mStartupTask

AsyncTask no longer needed

* Update model/Options (fixes #101)

* Fix NPE after config regeneration (fixes #140)

* Refactor key and config generation

Refactor ConfigXml public functions to allow checking if a valid config exists and trigger key and config (re)genration if something is corrupted.

* Fix crash on export/import (fixes #142)

* ApiRequest - Disable verbose log in release builds

* ConfigXml#updateIfNeeded - Disable "startBrowser"

because it applies to desktop environments and cannot start a mobile browser app

* MainActivity - Always show all tabs

* Show folder/device tab contents from config.xml

if syncthing is not running

* Update ConfigXml#getDevices return model

- compression
- introducer

* Device tab - Hide in/out rate if syncthing is not running

 or if the device is paused

* Update device item layout

* MainActivity/Devices - Prevent showing outdated status

after syncthing core transitioned from "active" to "disabled"

* MainActivity/Folders - Prevent showing outdated status

after syncthing core transitioned from "active" to "disabled"

* Add ConfigRouter class

Provides a transparent access to the config if ...
a) Syncthing is running and REST API is available.
b) Syncthing is NOT running and config.xml is accessed.

* Add pref - Cache local device ID

* Allow excluding self in ConfigRouter#getDevices

* Allow excluding self in ConfigRouter#getDevices (2)

* Update Folder model default values

* Update Folder model defaults (2)

- copiers
- hashers

* WIP - ConfigXml - FolderActivity

Remove unused pref inject code
Cache local device ID in pref
Reduce verbose logging in release builds
Extend ConfigXml#getFolders
Extend ConfigXml#getDevices
Fix ConfigXml#setDevicePause

ToDo ConfigXml#getFolderIgnoreList needs to be implemented

* Implemented ConfigXml#getFolderIgnoreList

* Extend ConfigXml#getDevices

- device.addresses

* WIP - DeviceActivity

Make it available when syncthing is not running

* Fix unsuccessful API bumps while syncthing is starting

* Fix space

* Adjust the folder icon to show if it's send/receive or both (fixes #143)

* Fix lint - item_device_list

* Preserve active tab when syncthing core transitions between running and not running

* Add xmlns:android to item_folder_list

* Remove unused reference from item_folder_list

* Add device icon to device tab

* Fix CPU percentage not showing (fixes #144)

* SyncthingService - Polish iterator code

* Fix MainActivity#updateViewPager (fixes #108)

* Add ConfigXml#updateFolder, updateDevice (1)

* Add ConfigRouter#updateFolder, updateDevice

* Add missing "final" to ConfigXml#updateDevice

* WIP - FolderActivity - Update updateFolder via ConfigRouter

ToDo: Implement ConfigRouter here.

* ConfigRouter - Fix missing return

* DeviceActivity - Update device via ConfigRouter

* Always make individual sync conditions UI available (fixes #145)

regardless if syncthing core is running or not.
Remove SyncthingService dependency from SyncConditionsActivity

* Fix incorrect folder type icon shown

when syncthing core is not running

* Add "introducedBy" to folder and device model (fixes #146)

* Add Folder#getDevices to model

* ConfigXml#updateFolder - Writeback devices sharing the folder

Support preserving the "introducedBy" model field of Folder.java (fixes #146)

* Add ConfigXml#updateFolder - Versioning

* Remove SyncthingService dependency from FolderPickerActivity

because it is no longer required.

* Update ToDo remarks

* Add ConfigXml#updateDevice - Addresses

* Fix DeviceActivity#persistableAddresses to be more graceful (fixes #147)

and accept the same address syntax as syncthing core web UI does.

* Add ConfigXml#removeFolder, removeDevice

* Add ConfigXml#addDevice, addFolder

- Add ConfigXml#isDeviceIdValid
- Do not allow adding empty folder labels or empty device names.
- Update model Folder.java so ConfigXml can handle the ignorePerms XML attribute

* Fix Syncthing wrapper "emergency" shutdown on native binary crash (fixes #148)

* Update translation de

* Add ConfigXml#postFolderIgnoreList

* Update APK version to 0.14.54.3 / 4182

* Revert DEBUG - Always run syncthing binary

* Update whatsnew
@Catfriend1 Catfriend1 removed this from the 1.0.0 milestone Jan 1, 2019
@Catfriend1
Copy link
Owner Author

I'm currently working on improving the clearness and "thinking positive" when it comes to the run conditions.

@Catfriend1
Copy link
Owner Author

@cdrfun : I've implemented another improvement, see [FR]"Only start syncthing native when at least one folder or device is/would be unpaused #189" which will help getting the last bit of your request done.

@Catfriend1
Copy link
Owner Author

We'll take another way forward and maybe do this later as said in PR:
"Only start syncthing native when at least one folder or device is/would be unpaused", PR #182

@Catfriend1
Copy link
Owner Author

@thumos333 -1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants