Combine multiple upload jobs into a single KCompositeJob so only 1 notification will be shown
ClosedPublic

Authored by eduisters on Nov 21 2018, 6:52 PM.

Details

Summary

Combine multiple upload jobs for files into a single KCompositeJob so only 1 notification will be shown
Includes changes introduced in D16279

Test Plan
  1. Share of multiple files is performed using 1 composite job

    Setup:
    • Select multiple (big) files in dolphin and share with an Android device Result:
    • The files will be transferred using 1 CompositeUploadJob and showing only 1 notification
  1. Share of file while another share is already running adds job to existing composite job

    Setup:
    • Select multiple (big) files in dolphin and share with an Android device
    • Share an additional file with the same Android device Result:
    • The files are all transferred using 1 CompositeUploadJob and showing only 1 notification
    • The notification is updated after adding the last file
  1. Other packets are transmitted as usual

    Setup:
      • Setup sharing desktop notification with device
    • Share a big file with an Android device
    • Generate a desktop notification (eg. sending or receiving an email)

      Result:
    • Notification packet is send immediately

Diff Detail

Repository
R224 KDE Connect
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
eduisters created this revision.Nov 21 2018, 6:52 PM
Restricted Application added a project: KDE Connect. · View Herald TranscriptNov 21 2018, 6:52 PM
Restricted Application added a subscriber: kdeconnect. · View Herald Transcript
eduisters requested review of this revision.Nov 21 2018, 6:52 PM

BUG: 390876

BUG: 390876

This is for sending so does not solve 390876. I will do receiving on Android next, then update sending from Android and last but not least BUG 390876

eduisters planned changes to this revision.EditedNov 24 2018, 7:24 PM

Something weird is going on when sending files over a super fast connection. While remote is still receiving a file this side already starts sending the next file when its finished transmitting the previous one.

Turns out that it can happen that the payload is transmitted completely but android is still processing it internally. Fixed the threading issue I had on the Android side

eduisters updated this revision to Diff 46212.Nov 25 2018, 4:46 PM
  • Fixed a typo
nicolasfella requested changes to this revision.Nov 26 2018, 10:38 PM
nicolasfella added a subscriber: nicolasfella.

This needs rebasing to master

This revision now requires changes to proceed.Nov 26 2018, 10:38 PM
eduisters updated this revision to Diff 46302.Nov 27 2018, 10:22 AM
  • Rebased onto master
apol added a subscriber: apol.Nov 28 2018, 2:59 PM

Other than that, LGTM.

cli/kdeconnect-cli.cpp
168
for (const QVariant &variant : qAsConst(urls)) {
eduisters added inline comments.Nov 28 2018, 6:46 PM
cli/kdeconnect-cli.cpp
168

I don't know why I used QVariant here (probably because I was fighting with msg.setArguments for a while). I assume that using for (const QString &url : qAsConst(urls)) is ok as well?

eduisters updated this revision to Diff 46424.Nov 28 2018, 6:56 PM
  • Requested changes by apol
albertvaka accepted this revision.Nov 29 2018, 8:35 PM
albertvaka added a subscriber: albertvaka.

I've been playing with this and it works as it should.

nicolasfella resigned from this revision.Nov 29 2018, 8:36 PM
This revision is now accepted and ready to land.Nov 29 2018, 8:36 PM

The only thing, but seems hard to fix is: when there is a transfer in progress and I add another file, the job is updated to say "sending N+1 files", but on Android it still says "receiving N files".

eduisters added a comment.EditedNov 29 2018, 8:59 PM

The only thing, but seems hard to fix is: when there is a transfer in progress and I add another file, the job is updated to say "sending N+1 files", but on Android it still says "receiving N files".

On android it will be updated when the next share packet is received (see D17157). I've thought about that but it I don't think there is a way to improve this without a 2-way protocol or introducing a new share packet to update the running transfer info

The only thing, but seems hard to fix is: when there is a transfer in progress and I add another file, the job is updated to say "sending N+1 files", but on Android it still says "receiving N files".

On android it will be updated when the next share packet is received (see D17157). I've thought about that but it I don't think there is a way to improve this without a 2-way protocol or introducing a new share packet to update the running transfer info

Indeed, it's complicated to fix. It's quite minor anyway.

eduisters updated this revision to Diff 46528.Nov 30 2018, 7:25 AM
  • Rebased onto master
This revision was automatically updated to reflect the committed changes.