Review Request 123094: Add support for sending and receiving payloads. (KDE)

[Saikrishna Arcot]

> On March 27, 2015, 3:26 a.m., Albert Vaca Cintora wrote:
> > core/backends/bluetooth/bluetoothdownloadjob.cpp, line 32
> > <https://git.reviewboard.kde.org/r/123094/diff/2/?file=356764#file356764line32>
> >
> >     Are you making a copy this way??

Yes. I needed to have the `BluetoothDownloadJob` class take ownership of the network package. However, because copies cannot be made, and because having the network package be on the heap complicates things a little in terms of memory managment, I chose to serialize it into a `QByteArray` and then unserialize it at the other end.


> On March 27, 2015, 3:26 a.m., Albert Vaca Cintora wrote:
> > core/filetransferjob.cpp, line 155
> > <https://git.reviewboard.kde.org/r/123094/diff/2/?file=356767#file356767line155>
> >
> >     Why is this change needed?

`QBuffer` doesn't have a `disconnected()` signal, since there's nothing for it to disconnect from. Therefore, if left as-is, the `FileTransferJob` class will close the buffer at line 185, but the `sourceFinished()` method never gets run. This causes the file transfer to never fully complete (even though the file is fully written out to disk and is viewable/usable).


> On March 27, 2015, 3:26 a.m., Albert Vaca Cintora wrote:
> > core/backends/bluetooth/bluetoothdevicelink.cpp, line 46
> > <https://git.reviewboard.kde.org/r/123094/diff/2/?file=356762#file356762line46>
> >
> >     The previous approach (with a buffer) makes more sense: if the file is really big we don't want to load it in memory to send it.

Will change.


- Saikrishna


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


On March 27, 2015, 2:19 a.m., Saikrishna Arcot wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123094/
> -----------------------------------------------------------
> 
> (Updated March 27, 2015, 2:19 a.m.)
> 
> 
> Review request for kdeconnect.
> 
> 
> Repository: kdeconnect-kde
> 
> 
> Description
> -------
> 
> When sending a payload,  a `===BEGIN PAYLOAD===` and `===END PAYLOAD===` markers are sent with the payload. On the receiving end, the data between these two markers is captured, and the size is checked. If the size doesn't match, then the current implementation just returns a package without a payload (which might be good or bad).
> 
> In addition, `core/filetransferjob.cpp` was edited to listen to the `aboutToClose()` signal instead of the `disconnected()` signal, which is guaranteed to exist on all implementations of `QIODevice`. I'm not sure of the side effects for the LAN device link.
> 
> 
> Diffs
> -----
> 
>   core/backends/bluetooth/CMakeLists.txt 125fa87a825b056395a8ce5ef0298665fd2e6293 
>   core/backends/bluetooth/bluetoothdevicelink.h 199d9ee4c6b89065154e82b6fcd2cea204c0ef31 
>   core/backends/bluetooth/bluetoothdevicelink.cpp e3c1e3335a312a2b9289a7806e6a4d9c9174c73c 
>   core/backends/bluetooth/bluetoothdownloadjob.h PRE-CREATION 
>   core/backends/bluetooth/bluetoothdownloadjob.cpp PRE-CREATION 
>   core/backends/devicelinereader.h a5255c77d95c13e5f806576bac2697fb4bc94708 
>   core/backends/devicelinereader.cpp bba0bdaae878be95680aa3c84aed8df9d9b81a8a 
>   core/filetransferjob.cpp 66866906a6509ebb0ba00c1b48647c5807262120 
> 
> Diff: https://git.reviewboard.kde.org/r/123094/diff/
> 
> 
> Testing
> -------
> 
> Android to KDE: Tested, and works. Sent a 1.1 MB picture from Android to KDE.
> 
> KDE to Android: To be tested.
> 
> 
> Thanks,
> 
> Saikrishna Arcot
> 
>

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