-
Notifications
You must be signed in to change notification settings - Fork 198
Added option to stop and resume download #732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -42,6 +42,8 @@ Page { | |||||||||||||||||||||
| qsTr("Preparing %1").arg(file) | ||||||||||||||||||||||
| else if (currentStatus === Units.DownloadStatus.Ready) | ||||||||||||||||||||||
| qsTr("Ready to write %1").arg(file) | ||||||||||||||||||||||
| else if (currentStatus === Units.DownloadStatus.Paused) | ||||||||||||||||||||||
| qsTr("%1 has been paused").arg(file) | ||||||||||||||||||||||
| else if (currentStatus == Units.DownloadStatus.Failed_Download) | ||||||||||||||||||||||
| qsTr("Failed to download %1").arg(file) | ||||||||||||||||||||||
| else | ||||||||||||||||||||||
|
|
@@ -78,12 +80,12 @@ Page { | |||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| QQC2.Label { | ||||||||||||||||||||||
| visible: currentStatus == Units.DownloadStatus.Downloading | ||||||||||||||||||||||
| visible: currentStatus == Units.DownloadStatus.Downloading || currentStatus == Units.DownloadStatus.Paused | ||||||||||||||||||||||
| text: downloadPage.leftStr | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| QQC2.Label { | ||||||||||||||||||||||
| visible: currentStatus == Units.DownloadStatus.Downloading | ||||||||||||||||||||||
| visible: currentStatus == Units.DownloadStatus.Downloading || currentStatus == Units.DownloadStatus.Paused | ||||||||||||||||||||||
| text: downloadPage.rightStr | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
@@ -135,6 +137,14 @@ Page { | |||||||||||||||||||||
| wrapMode: QQC2.Label.Wrap | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| QQC2.Label { | ||||||||||||||||||||||
|
||||||||||||||||||||||
| id: messageContinueDownload | ||||||||||||||||||||||
| visible: currentStatus === Units.DownloadStatus.Paused | ||||||||||||||||||||||
| text: qsTr("Download has been paused.") | ||||||||||||||||||||||
| wrapMode: QQC2.Label.Wrap | ||||||||||||||||||||||
| width: infoColumn.width | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| QQC2.Label { | ||||||||||||||||||||||
| id: messageSelectedImage | ||||||||||||||||||||||
| visible: releases.selected.isLocal | ||||||||||||||||||||||
|
|
@@ -228,16 +238,29 @@ Page { | |||||||||||||||||||||
| releases.variant | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| }, | ||||||||||||||||||||||
| State { | ||||||||||||||||||||||
| name: "paused" | ||||||||||||||||||||||
| when: currentStatus === Units.DownloadStatus.Paused | ||||||||||||||||||||||
| PropertyChanges { | ||||||||||||||||||||||
| target: progressBar; | ||||||||||||||||||||||
| value: releases.variant.progress.ratio | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| PropertyChanges { | ||||||||||||||||||||||
| target: messageContinueDownload; | ||||||||||||||||||||||
| visible: true | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| ] | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // There will be only [Finish] button on the right side so [Cancel] button | ||||||||||||||||||||||
| // is not necessary | ||||||||||||||||||||||
| previousButtonVisible: currentStatus != Units.DownloadStatus.Finished | ||||||||||||||||||||||
| previousButtonText: qsTr("Cancel") | ||||||||||||||||||||||
| previousButtonText: { | ||||||||||||||||||||||
| return qsTr("Cancel") | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+243
to
+245
|
||||||||||||||||||||||
| previousButtonText: { | |
| return qsTr("Cancel") | |
| } | |
| previousButtonText: qsTr("Cancel") |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you mean [Resume] and [Pause]? Also, in case of the verification, I don't think there is a sensible action besides [Cancel] to do here.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, I don't think we should allow to stop the verification process, not sure there is a point doing so.
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When resuming from a paused state, the code attempts to write to a drive even if the download hasn't completed yet. The resume logic should first complete the download before attempting to write. The current implementation will call both download() and write() simultaneously, which could cause unexpected behavior. Consider only calling download() when resuming from PAUSED status, and let the normal flow handle writing once the download is complete and the status becomes READY.
| if (selectedOption != Units.MainSelect.Write) | |
| releases.variant.download() | |
| if (drives.length) { | |
| drives.selected.setImage(releases.variant) | |
| drives.selected.write(releases.variant) | |
| } | |
| if (selectedOption != Units.MainSelect.Write) | |
| releases.variant.download() | |
| // When resuming from Paused, only resume download. | |
| // Writing will be handled by the normal flow once status is Ready. |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -373,12 +373,13 @@ class ReleaseVariant : public QObject, public DownloadReceiver | |
| public: | ||
| enum Type { LIVE = 0, NETINSTALL, FULL, ATOMIC }; | ||
| Q_ENUMS(Type) | ||
| enum Status { PREPARING = 0, DOWNLOADING, DOWNLOAD_VERIFYING, READY, WRITING_NOT_POSSIBLE, WRITING, WRITE_VERIFYING, FINISHED, FAILED_VERIFICATION, FAILED_DOWNLOAD, FAILED }; | ||
| enum Status { PREPARING = 0, DOWNLOADING, DOWNLOAD_VERIFYING, READY, PAUSED, WRITING_NOT_POSSIBLE, WRITING, WRITE_VERIFYING, FINISHED, FAILED_VERIFICATION, FAILED_DOWNLOAD, FAILED }; | ||
| Q_ENUMS(Status) | ||
| const QStringList m_statusStrings{tr("Preparing"), | ||
| tr("Downloading"), | ||
| tr("Checking the download"), | ||
| tr("Ready to write"), | ||
| tr("Download has been paused"), | ||
|
||
| tr("Image file was saved to your downloads folder. Writing is not possible"), | ||
| tr("Writing"), | ||
| tr("Checking the written data"), | ||
|
|
@@ -414,7 +415,6 @@ class ReleaseVariant : public QObject, public DownloadReceiver | |
|
|
||
| Status status() const; | ||
| QString statusString() const; | ||
| void setStatus(Status s); | ||
| QString errorString() const; | ||
| void setErrorString(const QString &o); | ||
|
|
||
|
|
@@ -439,6 +439,7 @@ class ReleaseVariant : public QObject, public DownloadReceiver | |
| public slots: | ||
| void download(); | ||
| void resetStatus(); | ||
| void setStatus(Status s); | ||
|
|
||
| private: | ||
| QString m_temporaryIso{}; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, following UX advice from Gemini, the main text should remain unchanged, but we should change the text below: