[Solved] Cannot download using QNetworkAccessManager

sierdzio

You are reading the data too soon. QNAM is asynchronous. You need to wait for the reply to come, before you can read it.

igorland

Hm, thanks! Too bad the examples that I have seen omit that. I will search for the solution.
Again, thanks a lot!

EDIT:
So I have found that I need to have
[code]
QEventLoop loop;
loop.exec();
[/code]

Can you prompt please where exactly it goes in the code?

Thanks!

sierdzio

They don't. You are simply doing it wrong in your MainWindow. Downloader class is done correctly.

igorland

sierdzio. Thank you again. Can you please elaborate what exactly I am doing wrong? Cheers!

sierdzio

@
Downloader d;
d.doDownload();

QString metarData;
// This is WAY too soon!
metarData = d.getData();
@

instead of getting the data in the very same method, you need to wait until the reply comes. Either use a forever loop (ugly solution!), or, just like you are doing in Downloader: respond to replyFinished().

igorland

sierdzio,

Sorry, I am trying to understand what you mean by "like you are doing in Downloader: respond to replyFinished()."

I have been searching more on this and another suggestion that I have come across with was to put the d object on the heap. I changed it to:

[code]
Downloader* d = new Downloader;
d->doDownload();

QString metarData;
metarData = d->getData();
ui->plainTextEdit->setPlainText(metarData);

[/code]

Still no luck... Sorry for being so n00b.. ))

sierdzio

When the button is clicked, only send the request (doDownload). Then getData in a slot connected to finished(). You need to refactor the code bit. Here is a small helper:
@
// Downloader
signals:
void downloadFinished(Downloader *d);
...
void Downloader::replyFinished (QNetworkReply *reply) {
...
emit downloadFinished(this);
}

// MainWindow
void MainWindow::on_pushButton_clicked()
{
Downloader *d = new Downloader(this);
connect (d, SIGNAL(downloadFinished(Downloader *)), this, SLOT(someSlot(Downloader *)));
d->doDownload();
...
}
...
void MainWindow::someSlot(Downloader *down)
{
QString metarData;
metarData = down->getData();
// Optional:
down->deleteLater();
}
@

igorland

sierdzio. You do belong to the Hall of Fame for your support and knowledge. I am still trying to nail it down. This is what I have now:

[code]
#include "Downloader.h"

Downloader::Downloader(QObject *parent) :
QObject(parent)
{
}

void Downloader::doDownload()
{
manager = new QNetworkAccessManager(this);
connect(manager, SIGNAL(finished(QNetworkReply*)),this, SLOT(replyFinished(QNetworkReply*)));
manager->get(QNetworkRequest(QUrl("ftp://ftp.com/ftp.TXT")));

}

void Downloader::replyFinished (QNetworkReply *reply)
{
if(reply->error())
{
qDebug() << "ERROR!";
qDebug() << reply->errorString();
}
else
{
dataByte = reply->readAll();
emit downloadFinished(this);
reply->deleteLater();
}
}

void Downloader::downloadFinished(Downloader *dfinished)
{
QString dataStr(dataByte);
dataFinal = dataStr;
dfinished->deleteLater();
}

QString Downloader::getData()
{
return dataFinal;
}

[/code]

MainWindow.cpp
[code]
#include "mainwindow.h"
#include "ui_mainwindow.h"
#include "Downloader.h"

MainWindow::MainWindow(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::MainWindow)
{
ui->setupUi(this);
}

MainWindow::~MainWindow()
{
delete ui;
}

void MainWindow::on_pushButton_clicked()
{
d = new Downloader(this); // declared in header
connect(d, SIGNAL(downloadFinished(Downloader *)), this, SLOT(someSlot(Downloader *)));
d->doDownload();

ui->plainTextEdit->setPlainText(metarData);

}

void MainWindow::someSlot(Downloader* down)
{
metarData = d->getData();
down->deleteLater();
}
[/code]

Now, I get a weird error saying that
[code]
multiple definition of Downloader::downloadFInished(Downloader*)
first define here
[/code]

Weird, because I only have it defined in Downloader.h
[code]
#ifndef DOWNLOADER_H
#define DOWNLOADER_H

#include <QObject>
#include <QNetworkAccessManager>
#include <QNetworkRequest>
#include <QNetworkReply>
#include <QUrl>
#include <QDateTime>
#include <QFile>
#include <QDebug>
#include <QEventLoop>

class Downloader : public QObject
{
Q_OBJECT
public:
explicit Downloader(QObject *parent = 0);
void doDownload();
QString getData();

signals:
void downloadFinished(Downloader *dfinished);

public slots:
void replyFinished (QNetworkReply *reply);

private:
QNetworkAccessManager *manager;
QByteArray dataByte;
QString dataFinal;
};

#endif // DOWNLOADER_H
[/code]

I love Qt, but it is so discouraging to spend many days on something that took me a day to figure out how to do it in libcurl and Java.

sierdzio. Thank you again!

sierdzio

If you want easy, go with QHttp or QFtp ;) QNAM is powerful, but also more complicated. The asynchronous paradigm is much better, but also more complicated and harder to get.

In your case, you've hit a small detail that some people new to Qt do stumble upon: a signal is not a method, only the slots need implementation. So, you should delete all this:
@
// Delete!
void Downloader::downloadFinished(Downloader *dfinished)
{
QString dataStr(dataByte);
dataFinal = dataStr;
dfinished->deleteLater();
}
@

And only leave this line in the header file:
@
signals:
void downloadFinished(Downloader *dfinished);
@

igorland

sierdzio. You are the MAN! Thank you. Now the file is getting downloaded. I see it in qDebug. One problem still remains though due the asynchronous process, I believe. The thing is that when in the following code:

[code]
void MainWindow::on_pushButton_clicked()
{
d = new Downloader(this);
d->doDownload();
connect(d, SIGNAL(downloadFinished(Downloader *)), this, SLOT(someSlot(Downloader *)));

qDebug() << "MetarData: " <&lt; metarData;
ui-&gt;plainTextEdit->setPlainText(metarData);

}

void MainWindow::someSlot(Downloader* down)
{
metarData = d->getData();
down->deleteLater();
}
[/code]

when I do this:
[code]
ui->plainTextEdit->setPlainText(metarData);
[/code]

it does not do anything. Apparently, it does not wait for the file to be first downloaded and goes straight to setPlainText because

[code]
qDebug() << "MetarData: " << metarData
[/code]

returns an empty string. In my main application, where metarData is further passed to other functions, I even get a CTD.
I thought that connect should take care of this and the code would wait for the full data to be downloaded first.

Thanks!

igorland

On the other hand, I can move it to the someSlot function and let it be there. Anyways, thanks a lot for your great help!

sierdzio

You assign the variable at the wrong time. Do this instead:
@
void MainWindow::someSlot(Downloader* down)
{
metarData = d->getData();
ui->plainTextEdit->setPlainText(metarData);
down->deleteLater();
}
@

igorland

Yes. Thanks a lot. Very much appreciated.