Review Request 121980: Added progress bar in notification while sending file.

[Albert Vaca Cintora]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121980/#review73782
-----------------------------------------------------------


I like the approach, but I found some issues in my phone and some things to improve in the code.


src/org/kde/kdeconnect/Plugins/SharePlugin/ShareToReceiver.java
<https://git.reviewboard.kde.org/r/121980/#comment51331>

    For some reason, in my Nexus 5 Lollipop, the notification doesn't update in real time. Instead, when the file transfer finishes, my phone starts executing all the notifications at once, rumbling for 5 seconds straight. We prorably shouldn't activate the rumble for this, but still that's not the root cause of the problem. Maybe runOnUiThread does some batching in Lollipop to execute several operations at the same time. Can you try changing this by having a handler on the UI thread, and using .post(Runnable) to the Handler? Using handlers is also a more lightweight mechanism that runOnUiThread if you have to call it a lot.



src/org/kde/kdeconnect/Plugins/SharePlugin/ShareToReceiver.java
<https://git.reviewboard.kde.org/r/121980/#comment51335>

    You might want to save the old progress, and if the new progress is not different than at least 1% (but could be even 5%) don't update the notification. The calculation can be done before calling the UI thread, so this way we save lots of calls to the UI.



src/org/kde/kdeconnect/Plugins/SharePlugin/ShareToReceiver.java
<https://git.reviewboard.kde.org/r/121980/#comment51332>

    Shouldn't this be inside the if?


- Albert Vaca Cintora


On gen. 11, 2015, 10:42 a.m., Vineet Garg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121980/
> -----------------------------------------------------------
> 
> (Updated gen. 11, 2015, 10:42 a.m.)
> 
> 
> Review request for kdeconnect and Albert Vaca Cintora.
> 
> 
> Repository: kdeconnect-android
> 
> 
> Description
> -------
> 
> Added progress bar in notification while sending file.
> 
> Current changes:
> * Renamed SendPackageFinishedCallback -> SendPackageStatusCallback and added progressChanged to it.
> * Made necessary changes so that the callback instance can reach to sendPayload for getting progress.
> * Made changes in ShareToReceiver to show progress.
> 
> 
> Diffs
> -----
> 
>   src/org/kde/kdeconnect/Device.java b9a876b 
>   res/values/strings.xml 23d10dc 
>   src/org/kde/kdeconnect/Backends/BaseLink.java 242ff67 
>   src/org/kde/kdeconnect/Backends/LanBackend/LanLink.java ca81c13 
>   src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java 19038bb 
>   src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackLink.java 0461cf1 
>   src/org/kde/kdeconnect/Plugins/SharePlugin/ShareToReceiver.java 170b7eb 
> 
> Diff: https://git.reviewboard.kde.org/r/121980/diff/
> 
> 
> Testing
> -------
> 
> Testing done for both file and content.
> 
> Current issues:
> * sendSuccessful is called much before during file is being sent.
> * Exception in onNotificationPosted due to early call of sendSuccessful.
> 
> Here is logcat output : http://pastebin.com/MvznkrAd
> 
> 
> Thanks,
> 
> Vineet Garg
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20150111/a7d360d6/attachment.html>