nurupo / ProjectTox-Qt-GUI

A cross-platform front end for ProjectTox Core library, written in C++11 with use of Qt5
GNU General Public License v3.0
342 stars 116 forks source link

Implement file send/receive function #85

Open aitjcize opened 10 years ago

aitjcize commented 10 years ago

Implement file receiving function.

The UI is ugly for now but the functionality works.

nurupo commented 10 years ago

Travis encountered an error while building with clang. Also, we don't use exceptions.

Have you read https://github.com/nurupo/ProjectTox-Qt-GUI/issues/68?

Please note that I probably won't be able to go over your PR earlier than the week of December 16th, my final exams are starting very shortly.

aitjcize commented 10 years ago

Hi,

I've remove the exception in the latest commit. I haven't read #68 yet, but I do so later. My current implementation put q QProgressbar directly in the message display widget. I think a better way to do it is to create a new widget that show the filename, time, status, speed(?) and progress bar, but I currently does not have time for it. Also the send file button is currently just a QPushButton next to the emoticon button, which is very ugly..., but since I'm focusing on the functionality of file sending/receiving, I hope someone who is better in UI design can fix it :P

BTW, is their any reason that you don't use QtDesigner to design the UI? I think building GUI with the graphical editor is much easier than doing it with code.

nurupo commented 10 years ago

is their any reason that you don't use QtDesigner to design the UI?

https://github.com/nurupo/ProjectTox-Qt-GUI/pull/53

aitjcize commented 10 years ago

I see. BTW, I think we should have a global(or perhaps in Core) data structure(QMap?) for looking up userId and nickname. Currently the only way to do it is using PageWidget::widget to get a ChatPageWidget then get the username from it. For example if I would to separate the file transfer related code to a new widget(say FileTrasnferManager), it would be a lot easier by just looking up from the QMap instead of having to use the widget() function and all that.

nurupo commented 10 years ago

What do you need userId and username for?

aitjcize commented 10 years ago

I meant to lookup username by userId. An example is that when popping a dialog telling you that someone is sending you a file(Although this should probably be show in the message view instead of using a QMessageBox...) or displaying the the sender info in the FileTransfer widget.

nurupo commented 10 years ago

An example is that when popping a dialog telling you that someone is sending you a file(Although this should probably be show in the message view instead of using a QMessageBox...)

Yeah, there should be a message in the Message Display widget saying that someone whats to send you a file. The File Transfer widget on the top would show some one-line info with filename and filesize, and buttons: save, save as, deny. The expanded state of File Transfer widget should also show those.

I meant to lookup username by userId.

The File Transfer widget is a part of ChatPageWidget, which knows username and status of a user. If you need those, you can just connect add an appropriate line to the corresponding methods. For example, in

void ChatPageWidget::setUsername(const QString& newUsername)

add

fileTransferWidget->setUsername(newUsername);

There is usually no need to query anything from the Core since everything is event-based.

aitjcize commented 10 years ago

I see, I'll try to modify the code to match the design you mentioned in #68 until you have time to review the patches. Thanks :)

nurupo commented 10 years ago

You are welcome to join the IRC channel mentioned in the readme.

fcore117 commented 10 years ago

How is progress in behind the scenes with file sending and other features? QT 5.2 is released too btw.

aitjcize commented 10 years ago

@fcore117 file sending works but the UI is different from the mock up. I need to make time for the GUI stuff.

aitjcize commented 10 years ago

I no longer have to time work on this patch, feel free to merge it to another branch and use it as base. The file transfer functionalities are there, just the UI is not implemented like the mockup.

aitjcize commented 10 years ago

Just done rebase to current master branch so the patch can build.

aitjcize commented 10 years ago

Thanks for Schlumpf's commit.

nurupo commented 10 years ago

Built it to test without looking at the code yet - seems to crash when sending big files (~3.24gb). Well, that will require rework anyway.

nurupo commented 10 years ago

@Schlumpf I see you are working on the new message widget thingy (I'm so happy!) : ) We settled on a separate widget for file transfers, apart from message widget, but I'm curious if you now know if it's possible to include a custom widget in a message?

aitjcize commented 10 years ago

@nurupo Crash with big files? Should I investigate and fix it or you want to redo that part?

nurupo commented 10 years ago

You could fix it, if you don't mind.

fcore117 commented 10 years ago

Currently at least it works somehow so at least we can keep up with Tox and release alpha when it comes. It would be really good to test at least text chat in beginning.

aitjcize commented 10 years ago

@nurupo, sure will do.

aitjcize commented 10 years ago

@nurupo how did you manage to test a 3.8G file? the transfer speed is so damn slow even with two local clients... lol

nurupo commented 10 years ago

I think I just selected a file to send, started sending it and the app crashed right away(?) Can try it again later today, maybe I will even get a back trace or something.

nurupo commented 10 years ago

I had a 3.2 gb iso file on one drive and tried to transfer it to another, from Alice to Bob. The file was created but filetransfer was progressing very slowly. 10kb/s, maybe?

Then I tried to move the same file within the same drive now, while another transfer was going, from Bob to Alice this time. It did create a file, wrote 220kb of something in it and then threw "QIODevice::write: ReadOnly device" in the console, spawned a lot of message boxes (just look at that black boarder around the message box) and gfhchcvhc crashed.

The stack trace doesn't really tell anything, it's all about the message box until one comments out the message box creation in the code and gets that stack trace:

0   QScopedPointer<QObjectData, QScopedPointerDeleter<QObjectData> >::data  qscopedpointer.h    143 0x6ba3a0e0  
1   qGetPtrHelper<QScopedPointer<QObjectData> > qglobal.h   907 0x6b8a734e  
2   QIODevice::d_func   qiodevice.h 166 0x6ba34567  
3   QIODevice::write    qiodevice.cpp   1305    0x6b8a63b4  
4   FileTransferState::writeData    filetransferstate.cpp   67  0x42108b    
5   ChatPageWidget::fileDataReceived    chatpagewidget.cpp  169 0x407724    
6   PagesWidget::fileData   pageswidget.cpp 145 0x40836c    
7   QtPrivate::FunctorCall<QtPrivate::IndexesList<0, 1, 2>, QtPrivate::List<int, unsigned char, QByteArray const&>, void, void (PagesWidget::*)(int, unsigned char, QByteArray const&)>::call   qobjectdefs_impl.h  508 0x45fad3    
8   QtPrivate::FunctionPointer<void (PagesWidget::*)(int, unsigned char, QByteArray const&)>::call<QtPrivate::List<int, unsigned char, QByteArray const&>, void>    qobjectdefs_impl.h  527 0x462ec6    
9   QtPrivate::QSlotObject<void (PagesWidget::*)(int, unsigned char, QByteArray const&), QtPrivate::List<int, unsigned char, QByteArray const&>, void>::impl    qobject_impl.h  149 0x4605f3    
10  QtPrivate::QSlotObjectBase::call    qobject_impl.h  132 0x6ba34fad  
11  QMetaCallEvent::placeMetaCall   qobject.cpp 479 0x6b93fb27  
12  QObject::event  qobject.cpp 1141    0x6b9408f9  
13  QWidget::event  qwidget.cpp 8354    0xa5af1aa   
14  QFrame::event   qframe.cpp  534 0xa6ae836   
15  QStackedWidget::event   qstackedwidget.cpp  291 0xa704589   
16  QApplicationPrivate::notify_helper  qapplication.cpp    3467    0xa57de0f   
17  QApplication::notify    qapplication.cpp    3432    0xa57dc96   
18  QCoreApplication::notifyInternal    qcoreapplication.cpp    878 0x6b91b572  
19  QCoreApplication::sendEvent qcoreapplication.h  232 0x6b9bee2b  
20  QCoreApplicationPrivate::sendPostedEvents   qcoreapplication.cpp    1482    0x6b91c708  
... <More>              

It crashes on no. 4, in FileTransferState::writeData on filetransferstate.cpp:67 line. Debugger says that this is null, which is bad because we are trying to access file _member_ variable, which translates to this->file. Only sender crashed btw. Also, the other client had first file's progress bar still progressing even with the receiver crashed...

I restarted both clients and tried moving a smaller file from one drive to another. Source path was pretty lengthy and contained unicode characters. The source file was named "main" and was about 1.2mb. It created the dest. file, progress bar reached 100%, but the file kept growing and growing, even after reaching 100%. Also, dest.'s file name was different from the source's. It was just "d".

aitjcize commented 10 years ago

Windows ... It worked fine on Linux though. I'll fix it, thanks for the detailed backtrace.