Martchus / syncthingtray

Tray application and Dolphin/Plasma integration for Syncthing
https://martchus.github.io/syncthingtray/
Other
1.5k stars 42 forks source link

"Sync Of Remote Folder Complete" setting doesn't seem to do anything #232

Closed artjomsR closed 4 months ago

artjomsR commented 4 months ago

Relevant components

Environment and versions

Client 1:

Client 2:

Bug description For background, I'm using syncthing tray to sync my obsidian/logseq DB across my devices, and I'm trying to achieve a balance where notifications are present but not overwhelming. I would like to receive a notification when a remote device (i.e. the one that is receiving my PC's changes) finishes syncing. It seems that sync of remote folder complete (found in Settings > Tray > Notifications) would do what I want (unless I'm misunderstanding its use or syncthing terminology) but currently I've not been able to trigger a notification when this option is checked.

Steps to reproduce

  1. Create and share a folder on the PC, accept and make an initial sync on the phone
  2. Kill the st client on the phone, such that only the PC is on
  3. Make a file change (and refresh) to issue a new outgoing sync
    • If both sync of local folder complete and sync of remote folder complete options are set:
    • 2 notifications are issued on the PC:
    • Synchronization of local folder FOLDER_NAME complete
    • If only sync of local folder complete option is set:
    • Synchronization of local folder FOLDER_NAME complete (Same as the above)
    • If only sync of remote folder complete option is set:
    • No notifications are issued
  4. On the iPhone, turn on the sync client to receive the file
    • If both sync of local folder complete and sync of remote folder complete options are set:
    • Another notification is issued on the PC:
    • Synchronization of PC on iPhone complete
    • If only sync of local folder complete option is set:
    • Synchronization of PC on iPhone complete (Same as the above)
    • If only sync of remote folder complete option is set:
    • No notifications are issued

I've also tried the same scenario as above with a SteamDeck instead of an iPhone. I've installed SyncThingy v0.7.4 from the Discover store which came with Syncthing v1.27.3. The resulting notifications are the same in both cases

Expected behavior According to https://docs.syncthing.net/intro/getting-started.html: The “local device” is the one you are configuring, viewing status for, etc., while the “remote device” is the other machine.

With the remote devices being the device on the receiving end, I would expect current notifications to be split between both sync of local folder complete / sync of remote folder complete options, instead of sync of local folder complete managing both of them. In other words, Synchronization of PC on iPhone complete should be issued when sync of remote folder complete options is checked.

Additional notes I've not been able to trigger any notifications when only the sync of remote folder complete option is set.

Martchus commented 4 months ago

Sounds like a bug, indeed. I'll have to see whether I can reproduce it.

Note that the notification about the local sync is expected to show immediately after the scan if enabled. Of course you should nevertheless also get a notification once the remote device has completed the sync.

Martchus commented 4 months ago

I could not really reproduce it.

I first tried with both notification types enabled:

When making a local modification that is big enough to take a few seconds to synchronize I get the following behavior:

  1. Notification about concluded local sync after scanning has concluded
  2. Notification about concluded remote sync after a few seconds when the remote device is back in sync
  3. There's only the second notification about the remote sync is visible in the notifications panel.

When making a local modification that is very small so it is synced in an instant I get the following behavior:

  1. Notification about concluded remote sync almost immediately.
  2. Notification about concluded local sync very shortly afterwards, replacing the first notification.
  3. There's only the second notification about the local sync is visible in the notifications panel.

It is strange that the order in which the notifications occur was changed in the second test. I don't know enough about Syncthing's internal event handling to have an explanation for it. Syncthing Tray is merely evaluating events in the order they come in via Syncthing's event API.

Note that I conduced both tests multiple times and the results were always consistent.

It is not ideal that notifications are replacing each other. Unfortunately Qt's notification API doesn't allow to tweak that. For GNU/Linux I've already implemented the notification handling on my own to avoid Qt's limitations. Not sure whether I'm willing to spend the effort for Windows as well.

I tested it again with only one of the notifications about concluded syncs enabled. Regardless which I disable, I now get not notifications anymore at all. This is clearly a bug.

Martchus commented 4 months ago

I tested it again with only one of the notifications about concluded syncs enabled. Regardless which I disable, I now get not notifications anymore at all. This is clearly a bug.

This was just broken in the way that the setting for local notifications was used in both cases. This change will fix it: https://github.com/Martchus/syncthingtray/commit/df01c0538f3cbacc077e778967fdc3305f450d76

With this change and when only enabling notifications for remote syncs the notification about remote syncs is not replaced and shown long enough to be well regognizable.

With both settings enabled there's still the problematic replacing behavior but apparently my build with Qt 6.7.0 seems to behave differently in the way that now both notifications are shown in the panel.

I also added logging on the console for easier debugging. This also shows that definitely both notifications are generated, e.g.:

Info: docs-misc wurde auf martchus-server synchronisiert
Info: docs-misc wurde lokal synchronisiert
artjomsR commented 4 months ago

Thank you so much! 🙏🙌 I’ll test it with the next release :)