Laden...

Asynchroner Download wartet nicht auf Beendigung in Schleife

Erstellt von DaniBrecht vor 5 Jahren Letzter Beitrag vor 5 Jahren 2.144 Views
D
DaniBrecht Themenstarter:in
6 Beiträge seit 2018
vor 5 Jahren
Asynchroner Download wartet nicht auf Beendigung in Schleife

Hallo zusammen.

Ich will von einem Webserver Filme herunterladen.
Dies funktioniert auch soweit. Leider ladet es aber immer alle Files zusammen, statt einzeln.

Ich benutze den Downloader mit Resume Funktion
https://github.com/Avira/.NetFileDownloader

Dies führt dazu, dass die Progressbar während dem Laden immer zwischen den Files hin und her springt.

foreach (var fileToDownload in doclist)
            {
                await Task.WhenAll(DownloadFileAsync(fileToDownload));

            }

Es soll aber nach dem await auch wirklich warten bis der Hintergrund Prozess fertig ist.
Wenn man ja ohne die Async Methode herunterladet ist zwar alles schön nacheinander, aber die Events für den DownloadProgressChanged werden nicht angestossen zudem friert dann das Fenster ein. Mit der Thread Lösung sowie Monitor wait habe ich auch schon probiert und es kommt die selbe Sympromatik

Ich hoffe ihr habt dazu eine Lösung.



using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using System.Windows.Forms;
using Cinebox_Connect.ResumeDownloader.Download;
using TestProject.ResumeDownload;

namespace TestProject
{


public class DocList
    {
        public string Docuri;
        public string Docfn;
     

        public DocList(string docuri, string docfn)
        {
             Docuri = docuri;
             Docfn = docfn;
        }
    }



    public partial class Test : Form
    {
        public int Refreshcounter;
        public void Test1(int x)
        {
            InitializeComponent();

            var filesToDownload = new List<Tuple<string, string, string, string, string>>
            {
                new Tuple<string, string, string, string, string>
                    ("C:\\temp\\",
                      "https://localhost/dvd1.mpg"
                        ,
                      "", "",
                      "C:\\temp\\a.dat"),

                new Tuple<string, string, string, string, string>
                    ("C:\\temp\\",
                       "https://localhost/dvd2.mpg"
                        , "", "", "C:\\temp\\b.dat"),
                new Tuple<string, string, string, string, string>
                    ("C:\\temp\\",
                        "https://localhost/dvd3.mpg"
                        , "", "", "C:\\temp\\c.dat")
            };

            var docLists = new List<DocList>();

            foreach (var fileToDownload in filesToDownload)
            {
                docLists.Add(new DocList(fileToDownload.Item2, fileToDownload.Item5));
            }

            DownloadMultipleFilesAsync(docLists);
        }
        public async Task DownloadFileAsync(DocList docList)
        {
            try
            {
                using (IFileDownloader fileDownloader = new FileDownloader())
                {
                    fileDownloader.DownloadProgressChanged += (sender2, e) => client_DownloadProgressChanged(sender2, e, docList.Docuri.Substring(docList.Docuri.Length - 15, 15), docList.Docfn);
                    await fileDownloader.DownloadFileAsync(new Uri(docList.Docuri), docList.Docfn);
                }
            }
            catch
                (Exception)
            {
                // ignored
            }
        }

        private async Task DownloadMultipleFilesAsync(List<DocList> doclist)
        {
            foreach (var fileToDownload in doclist)
            {
                await Task.WhenAll(DownloadFileAsync(fileToDownload));

            }
        }

        private void client_DownloadProgressChanged(object sender, DownloadFileProgressChangedArgs e, string received,
            string totalReceived)
        {
            if (received == null) throw new ArgumentNullException("received");

            var bytesIn = double.Parse(e.BytesReceived.ToString());
            var totalBytes = double.Parse(e.TotalBytesToReceive.ToString());
            var percentage = bytesIn/totalBytes*100;
            try
            {
                
                Refreshcounter++;
               if (Refreshcounter%500 == 0)
                {
                    //basicProgressBar1.Invoke(new Action(() =>
                    //{
                    //    basicProgressBar1.Text =
                    //        basicProgressBar1.Text =
                    //            "Downloaded :" + e.BytesReceived + " of " + e.TotalBytesToReceive + "  " + received +
                    //            "  " +
                    //            totalReceived;
                    //    basicProgressBar1.Value = int.Parse(Math.Truncate(percentage).ToString());
                    //    basicProgressBar1.Refresh();
                    //}));
                    
                    textBox1.Invoke(new Action(() =>
                    {
                        textBox1.Text =
                            basicProgressBar1.Text =
                                "Downloaded :" + e.BytesReceived + " of " + e.TotalBytesToReceive + "  " + received +
                                "  " +
                                totalReceived;
                        textBox1.Refresh();
                    }));
                }
            }

            catch (Exception ex)
            {
                // ignored
            }
        }

        //private void client_DownloadFileCompleted(object sender, DownloadFileCompletedArgs e)
        //{
        //    if (IsHandleCreated)
        //    {
        //        BeginInvoke((MethodInvoker)delegate
        //        {
        //            basicProgressBar1.Text = "Completed";
        //        });
        //    }
        //}
    }
} 

Coding is living

T
2.219 Beiträge seit 2008
vor 5 Jahren

Du solltest deinen Code mal aufräumen und sauber strukturieren.
Tupels als Daten Container zu missbrauchen ist nicht nur schlechtes Design sondern macht noch den Code schwerer les- und wartbar.
Hier solltest du dir eine eigene Klasse anlegen, die dann die entsprechenden Eigenschaften hat.
Macht dann auch die Arbeit im ganzen Prozess einfacher und du musst dir nicht merken welche Spalte in der Tupel welchen Zweck erfüllt,

Ist den der FileDownloader wirklich nötig bzw. musst du wirklich große Dateien laden?
Ansonsten würde ich es eher mit .NET Bordmitteln machen.
Für solch simple Mittel würde ich nicht auf externe Komponenten zugreifen wollen.

Ansonsten scheint dein Code auch ziemlich viel mit Tasks zu machen, was im Grund okay ist aber hier hast du schon eine DownloadMultipleFilesAsync die einen Task verwendet und dann noch deine DownloadFileAsync die wiederum einen Task triggert.
Warum machst du deine DownloadFileAsync nicht einfach zu DownloadFile und lässt dort den Task weg.
Da du eh auf die Methode warten musst, ist hier ein extra Task Overhead.
Dann musst du nur deine DownloadMultipleFilesAsync Methode in einem Task synchron die Dateien laden lassen.
Dann sollte dein Problem schon gelöst sein.

T-Virus

Developer, Developer, Developer, Developer....

99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.

D
DaniBrecht Themenstarter:in
6 Beiträge seit 2018
vor 5 Jahren

Vielen Dank für deine Antwort T-Virus. Es hat leider noch nicht geklappt.

Du solltest deinen Code mal aufräumen und sauber strukturieren.
Tupels als Daten Container zu missbrauchen ist nicht nur schlechtes Design sondern macht noch den Code schwerer les- und wartbar.
Hier solltest du dir eine eigene Klasse anlegen, die dann die entsprechenden Eigenschaften hat.
Macht dann auch die Arbeit im ganzen Prozess einfacher und du musst dir nicht merken welche Spalte in der Tupel welchen Zweck erfüllt,

Ok das wusste ich jetzt nicht. Bei Dodotnetperls steht

A summary. The Tuple is a typed, immutable, generic construct. That sounds impressive. Tuple is a useful container for storing conceptually-related data

Werde in dem Fall wieder Klassen für die Datenhaltung benutzen.

Ist den der FileDownloader wirklich nötig bzw. musst du wirklich große Dateien laden?

Der Enduser ladet vom Server Files von 4-10 GB herunter. Daher ist auch die Resumefunktion essentiell.

Warum machst du deine DownloadFileAsync nicht einfach zu DownloadFile und lässt dort den Task weg.

Ich bin deiner Empfehlung gefolgt. Leider wartet er auch dann nicht.
Bei der normaler WebClient Klasse funktioniert es in beiden Fällen wie es soll (Wartet bis einzelnes File fertig) . Nur bei
der externen Komponente bleibt das Problem bestehen.

Hinweis von Coffeebean vor 5 Jahren

Bitte benutze Code-Tags [Hinweis] Wie poste ich richtig?

Coding is living

1.029 Beiträge seit 2010
vor 5 Jahren

Hi,

naja - wenn du File für File herunterladen möchtest ist Task.WhenAll schlicht falsch.
(Das sieht man ja schon an der Beschreibung der Methode)

Wenn du alle einzeln abwarten möchtest - müsste es eher in folgende Richtung gehen:


public async void RunAfterAnother()
        {
            Console.WriteLine($"Started RunAfterAnother");
            var num = new[] { 1, 2, 3, 4, 5 };
            foreach (var n in num)
            {
                await DownloadFileAsync();
                Console.WriteLine($"Finished task {n}");
            }
        }

LG

D
DaniBrecht Themenstarter:in
6 Beiträge seit 2018
vor 5 Jahren

Hallo,

Ich habe den Code angepasst, aber der Download ladet immer noch alle Files gleichzeitig und die Progressbar springt hin und her.

Ich habe im Netz auch keine Beispiele gefunden mit Resume Funktionalität. Nur diverse Async Downloader ohne Resume Funktion.


 public async void DownloadFile(DocList docList)
        {
            try
            {
                using (IFileDownloader fileDownloader = new FileDownloader(new Cache()))
                {
                    fileDownloader.DownloadProgressChanged += (sender2, e) => client_DownloadProgressChanged(sender2, e, docList.RemoteURI.Substring(docList.RemoteURI.Length - 15, 15), docList.LocalDocumentPath);
                    await fileDownloader.DownloadFileAsync(new Uri(docList.RemoteURI), docList.LocalDocumentPath);
                }
            }
            catch
                (Exception)
            {
                // ignored
            }
        }

        private void DownloadMultipleFilesAsync(List<DocList> doclist)
        {
            foreach (var document in doclist)
            {
                 DownloadFile(document);

            }
        }

Coding is living

1.029 Beiträge seit 2010
vor 5 Jahren

Hi,

sry - aber der hier gepostete Teil ist weder vollständig - noch berücksichtigt er das von mir eingebrachte "await" an der korrekten Stelle. Also:


await DownloadFile(document);

Ich habe mir selbst mal das Nuget-Paket der "Avira.FileDownloader" gezogen und muss sagen: ich bin verwirrt, wie das bei dir überhaupt kompilieren kann.

Die Methode "DownloadFileAsync" gibt nicht wie man erwarten sollte einen Task (den man "awaiten" könnte) zurück - sondern "void" zurück, womit es unmöglich ist überhaupt ein await zu verwenden.

Somit die Frage:
Woher kommt deine Codebase? Ist der Code direkt von Github runtergeladen?

Abseits davon:
Der Entwickler scheint nicht verstanden zu haben, wie man bei derlei Implementierungen vorgeht - wenn eine Methode async ist und gar parallel arbeitet - gibt die gefälligst einen Task zurück - und kein void. Ich an deiner Stelle würde mich direkt nach einer anderen Bibliothek umsehen.

LG

2.298 Beiträge seit 2010
vor 5 Jahren

Zusätzlich zum gesagten, kannst du es ja eventuell auch anders angehen.

Konkreten Tipp habe ich nicht aber einen Methodennamen / Stichwort: ContinueWith

Wissen ist nicht alles. Man muss es auch anwenden können.

PS Fritz!Box API - TR-064 Schnittstelle | PS EventLogManager |

D
DaniBrecht Themenstarter:in
6 Beiträge seit 2018
vor 5 Jahren

Ich habe jetzt mal den Code optimiert und die Variante von inflames2k mit ContinueWith benutzt.
Zusätzlich habe ich jetzt den Originalcode vom Repository verwendet.

Jetzt bleibt die Anzeige leer und wird gar nicht aktualisiert obwohl die Events abonniert sind und die Methode async ist(zwar nicht awaitable, aber dafür soll ja continueWith sorgen).


using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using System.Windows.Forms;
using WindowsFormsApplication2.Download;
using FileDownloader;

namespace TestProject
{
    public partial class Test : Form
    {
        public int Refreshcounter;


        public class DocList
        {
            public string Uri { get; }
            public string LocaFileName { get; }

            public DocList(string uri, string locaFileName)
            {
                this.Uri = uri;
                this.LocaFileName = locaFileName;
            }
        }

        public void Test1(int x)
        {
            InitializeComponent();
            var docLists = new List<DocList>
            {
                new DocList(
                    "https://localhost/DVD1.mpg"
                    , "C:\\temp\\a.dat"),
                new DocList(
                    "https://localhost/DVD2.mpg"
                    , "C:\\temp\\b.dat"),
                new DocList(
                    "https://localhost/DVD3.mpg"
                    , "C:\\temp\\c.dat")
            };
            DownloadFiles(docLists);
        }

        public Task DownloadFiles(List<DocList> docList)
        {
            var index = 0;
            return Task.Factory.StartNew(() =>
            {
                Console.WriteLine("Step 1");

                DownloadFile(docList, index);
                index++;
            })
                .ContinueWith((prevTask) =>
                {
                    DownloadFile(docList, index);
                    index++;
                })
                .ContinueWith((prevTask) =>
                {
                    DownloadFile(docList, index);
                });
        }

        private void DownloadFile(List<DocList> docList, int index)
        {
            using (IFileDownloader fileDownloader = new global::FileDownloader.FileDownloader(new Cache()))
            {
                fileDownloader.DownloadProgressChanged +=
                    (sender2, e) =>
                        client_DownloadProgressChanged(sender2, e,
                            docList[index].LocaFileName, "");
                fileDownloader.DownloadFileAsync(new Uri(docList[index].Uri), docList[index].LocaFileName);
            }
        }

        private void client_DownloadProgressChanged(object sender, DownloadFileProgressChangedArgs e, string received,
            string totalReceived)
        {
            if (received == null) throw new ArgumentNullException("received");
            //throw new NotImplementedException();
            Refreshcounter++;

            // Only print to the console once every 500 times the event fires,
            if (Refreshcounter % 5 == 0)
            {
                var bytesIn = double.Parse(e.BytesReceived.ToString());
                var totalBytes = double.Parse(e.TotalBytesToReceive.ToString());
                var percentage = bytesIn / totalBytes * 100;
                try
                {
                    basicProgressBar1.Invoke(new Action(() =>
                    {
                        basicProgressBar1.Text =
                            basicProgressBar1.Text =
                                "Downloaded :" + e.BytesReceived + " of " + e.TotalBytesToReceive + "  " + received +
                                "  " + totalReceived;
                        basicProgressBar1.Value = int.Parse(Math.Truncate(percentage).ToString());
                        basicProgressBar1.Refresh();
                    }));

                }
                catch (Exception ex)
                {
                }
            }
        }
    }
}

 

Coding is living

T
2.219 Beiträge seit 2008
vor 5 Jahren

Das Problem liegt vermutlich an deiner Test1 Methode.
Sieht eher nach dem Konstruktor aus, den du fälschlicherweise zu einer Methode umgebogen hast.
Da die Methode in deinem Code auch nicht aufgerufen wird, wird auch kein Download ausgeführt.

Die InitializeComponent Methode gehört hier auch in den Form Konstruktor, da dort die ganzen Einstellungen deiner Controlls gesetzt werden.
Ebenfalls solltest du das Console.WriteLine aus deinem Form entfernen.
Sowas gehört nicht in ein WinForms Projekt.

DocList ist auch eine ungünstige Bezeichnung für deine Klasse, da diese eine Liste von Dokumenten impliziert.
Besser wäre etwas wie DownloadFile, was schon sprechender ist und klar macht was die Klasse repräsentieren soll.

Dein RefreshCounter sollte auch kein public Feld sein.
Wenn du diese Information nur innerhalb deines Form brauchst, setzte es privat und schreib das Feld auch klein.
Ansonsten mach eine Property daraus, damit du auch eine gewissen Kontrolle hast.
Sonst könnte jemand ungültige/falsche Werte außerhalb deines Form setzen, was nicht korrekt sein dürfte.

Den FileDownloader mit global::FileDownloader.FileDownloader zu initalisieren kannst du dir sparen.
Hier reicht FileDownloader.FileDownloader, der Compiler sollte dies eigentlich ohne Probleme auflösen können.

Es wäre auch sauberer wenn du für deine Progress Methode eine richtige Methode anlegst.
Dann kannst du deinen Code leichter warten als durch eine kryptische anonyme Methode.
Hier müsstest du dir nur über ein Feld den Index deiner Liste merken und nach dem erfolgreichen Download wie gehabt hochzählen.
Dann musst du den Index auch nicht überall durchreichen.

T-Virus

Developer, Developer, Developer, Developer....

99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.

1.029 Beiträge seit 2010
vor 5 Jahren

Hi,

ich möchte dennoch nachdrücklich darauf hinweisen, dass die von dir verwendete Bibliothek das so gar nicht unterstützt... Die Methoden geben schlicht void zurück und verwalten entsprechende Tasks komplett selbst. Du kannst also nicht auf diese Art auf den Abschluss eines Downloads warten.

Denkbar wäre es eine Art Wrapper darum zu bauen, der erst nachdem des Ereignis "DownloadCompleted" ausgelöst wurde einen neuen Download zu starten... Aber so wie du es angehst - kann das eigentlich nichst werden.

LG

D
DaniBrecht Themenstarter:in
6 Beiträge seit 2018
vor 5 Jahren

Hallo Taipi88.

Dann werde ich, bevor ich die Vorschläge von T-Virus umsetze (auf Basis des neuen Frameworks), ein geeignetes Framework suchen welches Async Tasks unterstützt und sich auf 1 Download pro Task einschränken lässt (für die Progressbar).

Notfalls halt mit dem Property -> System.Net.ServicePointManager.DefaultConnectionLimit = 1

ev kennt ihr etwas in diese Richtung?

Ich prüfe mal die Codebasis von https://archive.codeplex.com/?p=scavenger

Ich kann mir nicht vorstellen, dass ich die einzige Person bin die diese Anforderung schon meistern musste. 😃

Coding is living

T
2.219 Beiträge seit 2008
vor 5 Jahren

Wie Taipi88 schon schreibt, wäre es einfacher dies per Event zu lösen.
Jetzt den ganzen Code auf ein neues Framework umzustellen wäre möglich aber wieder Aufwand und du müsstest wieder von 0 anfangen.

Einfacher wäre es deine DocList Klasse um ein IsCompleted Flag zu erweitern.
Du musst dir dann beim start eines Downloads nur das aktive DocList Element merken und im Event dort das IsCompleted auf true setzen.
Dann kannst du den nächsten Eintrag starten.
Der Prozess muss dann aber in einem eigenen asynchronen Prozess laufen.

Der Aufwand für solch eine Umsetzung wäre recht überschaubar im Gegensatz wieder ein neues Framework einzubinden.
Ansonsten wäre dann die Frage ob der Aufwand von anderen Lösungen als einem einfachen WebClient wirklich gerechtfertigt ist.
Wenn du keine Mobile oder eine instabile Verbindung hast und du mehrere GB großen Dateien laden willst, kannst du auch .NET Lösungen verwenden.

Ebenfalls bemerke ich gerade, dass du per https DVD Dateien laden willst.
Besser wäre hier FTP oder eine direkte Freigabe anstelle von http/s.
FTP ist gerade für de Datenübertragung gemacht.
Wenn du nur im lokalen Netz Dateien laden willst, wäre sogar eine Freigabe der einfachere Weg.
Hier kannst du auch ohne zusätzliche Lösungen per FileStream oder File Klasse auf die Dateien zugreifen.
Die Bandbreite sollte im LAN i.d.R. 1GBit/s betragen, da kannst du dann die Daten neu ziehen falls ein Download abbricht.
Ein Fortsetzenfunktion ist bei der Bandbreite dann nur Nice To Have.

T-Virus

Developer, Developer, Developer, Developer....

99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.

D
DaniBrecht Themenstarter:in
6 Beiträge seit 2018
vor 5 Jahren

Ich habe es jetzt mit der Variante mit
.ContinueWith hinbekommen.

Allerdings ist das nicht foreach tauglich

Ich versuche dies noch herauszufinden wie die LINQ Variante formuliert werden muss und poste dann den Code mit den Empfehlungen von T-Virus.

Danke für eure Support bis jetzt.

Coding is living