Entwickler-Ecke

WinForms - Keine Rückmeldung wegen zu vieler Kopierbefehle


UserTom - Mo 16.05.22 22:50
Titel: Keine Rückmeldung wegen zu vieler Kopierbefehle
Hallo Gemeinde,

Ich bin neu hier bei Euch. Ich bin SPSler und habe mit C# normalerweise nichts zu tun.
Mit einigen gut bebilderten Internetseiten habe ich es geschafft mit einem Winforms Projekt mich mit einer SPS zu verbinden. (Sharp7.dll)
Über ein Trigger Signal von der SPS möchte ich einige Kopieraktionen durchführen. Es gibt auch verschiebeaktionen. Innnerhalb Lokal und von anderen PCs über das Netzwerk auf Lokal. Das kopieren der Dateien und auch Ordner habe ich umsetzen können. Das Ergebenis ist aber eher schlecht als recht. Mit einem Timer rufe ich einen Backgrounworker auf um die ganzen kopieraktionen im Hintergrund auszuführen. Gui blockiert aber trotzdem. Ich habe auch schon probiert mit Task.Run das kopieren auszulagern.
Hat auch nicht geholfen. Die Oberfläche blockiert (keine Rückmeldung) in dem Moment wo das kopieren beginnt bis es fertig ist. Auf einer Internetseite habe ich gelesen das gleichzeitige kopieren eh nichts bringen soll wegen den gleichzeitigen schreibzugriffen.
Auf einer anderen Seite habe ich gelesen Threads ist besser wie der Backgroundworker.
Hat jemand von Euch damit Erfahrung und kann mir sagen wie ich das umsetzen muss?
Vielleicht hat auch der Timer schuld daran, aber den brauch ich als Loop für die Verbindung mit der SPS. Alle 65s steht ein Triggersignal an und das darf ich ja nicht verpassen.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48:
49:
50:
51:
52:
53:
54:
55:
56:
57:
58:
59:
60:
61:
62:
63:
64:
65:
66:
67:
68:
69:
70:
71:
72:
73:
74:
75:
76:
77:
78:
79:
80:
81:
82:
83:
84:
85:
86:
87:
88:
89:
90:
91:
92:
93:
94:
95:
96:
97:
98:
99:
100:
101:
102:
103:
104:
105:
106:
107:
108:
109:
110:
111:
112:
113:
114:
115:
116:
117:
118:
119:
120:
121:
122:
123:
124:
125:
126:
127:
128:
129:
130:
131:
132:
133:
134:
135:
136:
137:
138:
139:
140:
141:
142:
143:
144:
145:
146:
147:
148:
149:
150:
151:
152:
153:
154:
155:
156:
157:
158:
159:
160:
161:
162:
163:
164:
165:
166:
167:
168:
169:
170:
171:
172:
173:
174:
175:
176:
177:
178:
179:
180:
181:
182:
183:
184:
185:
186:
187:
  private void EventLoop_Tick(object sender, EventArgs e)
        {
            EventLoop.Enabled = false;

            if (BW_Event.IsBusy != true)
            {
                BW_Event.RunWorkerAsync();
            }
        }

        private void BW_Event_DoWork(object sender, DoWorkEventArgs e)
        {
            BW_Event.ReportProgress(0);
        }

        private void BW_Event_ProgressChanged(object sender, ProgressChangedEventArgs e)
        {
            try
            {
                S7VariablenResult s7VariablenResult = LeakResult();

                string createTargetPath = "";

                if (s7VariablenResult.TrigSaveFiles && (s7VariablenResult.TypLinks || s7VariablenResult.TypRechts) && !s7VariablenResult.RoboterDSLeer && !s7VariablenResult.FMBauteil && !s7VariablenResult.RoboterDSLeer)
                {

                    lblPraegeCode.Text = s7VariablenResult.PraegeCodeNr; // It’s already a string
                    lblFolderName.Text = s7VariablenResult.FolderName;
                    lblPraegeCode2.Text = s7VariablenResult.PraegeCode2Nr;

                    createTargetPath = Path.Combine(@"D:\S7_Export\AppData", lblFolderName.Text);
                    try
                    {
                        //Ordner erstellen
                        if (!Directory.Exists(createTargetPath))
                        {
                            Directory.CreateDirectory(createTargetPath);
                        }
                    }
                    catch (Exception ex)
                    {
                        MessageBox.Show(this, ex.Message, "Ordner erstellen fehlgeschlagen!", MessageBoxButtons.OK, MessageBoxIcon.Error);
                    }

                    
                    // 10R01 Tox Clinchnietbolzen

                    //IEnumerable<string> Tox10R01files = Directory.EnumerateFiles(@"\\tox10r01\AppData", "*", System.IO.SearchOption.TopDirectoryOnly).Where(x => x.Contains(lblPraegeCode.Text));
                    IEnumerable<string> Tox10R01files = Directory.EnumerateFiles(@"\\computer\PDE\tox10r01\AppData""*", System.IO.SearchOption.TopDirectoryOnly).Where(x => x.Contains(lblPraegeCode.Text));

                    foreach (string file in Tox10R01files)
                    {

                        try
                        {
                            File.Copy(file, $"{Path.Combine(@"D:\S7_Export\AppData", lblFolderName.Text + "\\")}{Path.GetFileName(file) }");

                        }
                        catch (DirectoryNotFoundException ex)
                        {
                            ex.SaveErrorMessage();
                        }
                        catch (UnauthorizedAccessException ex)
                        {
                            ex.SaveErrorMessage();
                        }
                    }


                    
                    //15R01 Tox Kalottenprägen

                    //IEnumerable<string> ToxKPr15R01files = Directory.EnumerateFiles(@"\\tox15r01\AppData\", "*", System.IO.SearchOption.TopDirectoryOnly).Where(x => x.Contains(lblPraegeCode.Text));
                    IEnumerable<string> ToxKPr15R01files = Directory.EnumerateFiles(@"\\computer\PDE\tox15r01\AppData""*", System.IO.SearchOption.TopDirectoryOnly).Where(x => x.Contains(lblPraegeCode.Text));

                    foreach (string file in ToxKPr15R01files)
                    {

                        try
                        {
                            File.Copy(file, $"{Path.Combine(@"D:\S7_Export\AppData", lblFolderName.Text + "\\")}{Path.GetFileName(file) }");

                        }
                        catch (DirectoryNotFoundException ex)
                        {
                            ex.SaveErrorMessage();
                        }
                        catch (UnauthorizedAccessException ex)
                        {
                            ex.SaveErrorMessage();
                        }
                    }

                    
                    //15R02 Tox Clinchnietbolzen

                    //IEnumerable<string> Tox15R02files = Directory.EnumerateFiles(@"\\tox15r02\AppData\", "*", System.IO.SearchOption.TopDirectoryOnly).Where(x => x.Contains(lblPraegeCode.Text));
                    IEnumerable<string> Tox15R02files = Directory.EnumerateFiles(@"\\computer\PDE\tox15r02\AppData""*", System.IO.SearchOption.TopDirectoryOnly).Where(x => x.Contains(lblPraegeCode.Text));

                    foreach (string file in Tox15R02files)
                    {

                        try
                        {
                            File.Copy(file, $"{Path.Combine(@"D:\S7_Export\AppData", lblFolderName.Text + "\\")}{Path.GetFileName(file) }");

                        }
                        catch (DirectoryNotFoundException ex)
                        {
                            ex.SaveErrorMessage();
                        }
                        catch (UnauthorizedAccessException ex)
                        {
                            ex.SaveErrorMessage();
                        }
                    }

                    
                    //15R02 Intec Kleben

                    //IEnumerable<string> Kle15R02dirs = Directory.EnumerateDirectories(@"\\kl15r02\AppData\", "*", System.IO.SearchOption.AllDirectories).Where(x => x.Contains(lblPraegeCode.Text));
                    IEnumerable<string> Kle15R02dirs = Directory.EnumerateDirectories(@"\\computer\PDE\kle15r02\AppData""*", System.IO.SearchOption.AllDirectories).Where(x => x.Contains(lblPraegeCode.Text));

                    foreach (string dir in Kle15R02dirs)
                    {
                        try
                        {
                            // ohne Backslash
                            //FileSystem.CopyDirectory(dir, dir.Replace(@"\\kl15r02\AppData", Path.Combine(@"D:\S7_Export\AppData", lblFolderName.Text)), true);
                            FileSystem.CopyDirectory(dir, dir.Replace(@"\\computer\PDE\kle15r02\AppData", Path.Combine(@"D:\S7_Export\AppData", lblFolderName.Text)), true);

                        }
                        catch (DirectoryNotFoundException ex)
                        {
                            ex.SaveErrorMessage();
                        }
                        catch (UnauthorizedAccessException ex)
                        {
                            ex.SaveErrorMessage();
                        }
                    }
 private S7VariablenResult LeakResult()
        {
            S7VariablenResult s7VariablenResult = new S7VariablenResult();
            // Reads the buffer.
            int dbnumber = short.Parse(txtDBNr.Text);
            int dboffset = short.Parse(txtDBOffsetNr.Text);
            int dbsize = short.Parse(txtDBSizeNr.Text);

            int PraegeCodeDBPos = short.Parse(txtPraegeCodeDBPos.Text);
            int FolderNameDBPos = short.Parse(txtFolderNameDBPos.Text);
            int PraegeCode2DBPos = short.Parse(txtPraegeCode2DBPos.Text);

            int TrigSaveFilesByte = short.Parse(txtTrigSaveFilesByte.Text);
            int TrigSaveFilesBit = short.Parse(txtTrigSaveFilesBit.Text);

            int TypLinksByte = short.Parse(txtTypLinksByte.Text);
            int TypLinksBit = short.Parse(txtTypLinksBit.Text);

            int TypRechtsByte = short.Parse(txtTypRechtsByte.Text);
            int TypRechtsBit = short.Parse(txtTypRechtsBit.Text);

            int RoboterDSLeerByte = short.Parse(txtRoboterDSLeerByte.Text);
            int RoboterDSLeerBit = short.Parse(txtRoboterDSLeerBit.Text);

            int FMBauteilByte = short.Parse(txtFMBauteilByte.Text);
            int FMBauteilBit = short.Parse(txtFMBauteilBit.Text);

            int HeartByte = short.Parse(txtHeartByte.Text);
            int HeartBit = short.Parse(txtHeartBit.Text);

            byte[] dbbuffersize = new byte[dbsize];

            s7Client.DBRead(dbnumber, dboffset, dbsize, dbbuffersize);

            s7VariablenResult.PraegeCodeNr = S7.GetStringAt(dbbuffersize, PraegeCodeDBPos);
            s7VariablenResult.FolderName = S7.GetStringAt(dbbuffersize, FolderNameDBPos);
            s7VariablenResult.PraegeCode2Nr = S7.GetStringAt(dbbuffersize, PraegeCode2DBPos);
            s7VariablenResult.TrigSaveFiles = S7.GetBitAt(dbbuffersize, TrigSaveFilesByte, TrigSaveFilesBit);
            s7VariablenResult.TypLinks = S7.GetBitAt(dbbuffersize, TypLinksByte, TypLinksBit);
            s7VariablenResult.TypRechts = S7.GetBitAt(dbbuffersize, TypRechtsByte, TypRechtsBit);
            s7VariablenResult.RoboterDSLeer = S7.GetBitAt(dbbuffersize, RoboterDSLeerByte, RoboterDSLeerBit);
            s7VariablenResult.FMBauteil = S7.GetBitAt(dbbuffersize, FMBauteilByte, FMBauteilBit);
            s7VariablenResult.HeartBeat = S7.GetBitAt(dbbuffersize, HeartByte, HeartBit);

            return s7VariablenResult;
        }


Es wäre echt toll wenn einer von Euch eine Idee für mich hätte.

Grüße Tom


Th69 - Di 17.05.22 09:18

Hallo und :welcome:,

asynchrone Programmierung kann ersteinmal ein großer Stolperstein für Anfänger sein.

Dein Hauptfehler ist, daß du die eigentliche Kopieraktion im ProgressChanged durchführst, anstatt im DoWork (das ProgressChanged-Ereignis läuft im UI-Thread und blockiert daher die GUI - es sollte nur für kurze Ausgabeaktualisierungen benutzt werden)!

PS: Der Backgroundworker wird in modernem C# eigentlich nicht mehr benutzt und stattdessen mit Tasks und den Schlüsselwörtern async und await gearbeitet. Für (einfache) WinForms-Projekte halte ich es jedoch noch OK.

PPS: Dateipfade solltest du besser konfigurierbar halten (anstatt fest im Code als Stringliterale zu verwenden)!
Und den eigentlichen Kopiercode solltest du in eine eigene Methode auslagern und mit passenden Parametern aufrufen (anstatt Copy&Paste-Code).


UserTom - Di 17.05.22 10:58

Hallo TH69,

Vielen Dank für deine Antwort.

Zitat:
asynchrone Programmierung kann ersteinmal ein großer Stolperstein für Anfänger sein.

Das kannst du laut sagen. Dazu gleich eine Frage das ich den RunWorkerAsync im Timer aufrufe ist ok? mit viele Sekunden sollte er ticken?

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
 private void EventLoop_Tick(object sender, EventArgs e)
        {
            EventLoop.Enabled = false;

            if (BW_Event.IsBusy != true)
            {
                BW_Event.RunWorkerAsync();
            }
        }


Zitat:
Dein Hauptfehler ist, daß du die eigentliche Kopieraktion im ProgressChanged durchführst, anstatt im DoWork (das ProgressChanged-Ereignis läuft im UI-Thread und blockiert daher die GUI - es sollte nur für kurze Ausgabeaktualisierungen benutzt werden)!

Ich hatte alles im DoWork
Wegen dem Label: lblPraegeCode.Text = s7VariablenResult.PraegeCodeNr; habe ich alles in das ProgressChanged getan.
Ich muss also nur drauf aufpassen das keine Zugriff auf Gui-Elemente im DoWork passieren.

Wann und wo muss ich BW_Event.ReportProgress(0); aufrufen? Im DoWork nach dem kopieren?

Zitat:
PS: Der Backgroundworker wird in modernem C# eigentlich nicht mehr benutzt und stattdessen mit Tasks und den Schlüsselwörtern async und await gearbeitet. Für (einfache) WinForms-Projekte halte ich es jedoch noch OK.

Hättest du ein Beispiel oder ein guten Link in Zusammenhang Winforms und kopieren und Timer als Loop.

Zitat:
PPS: Dateipfade solltest du besser konfigurierbar halten (anstatt fest im Code als Stringliterale zu verwenden)!

Ja da hast du recht, es gibt auch eine Variante, aber wegen des Backgroundworker habe ich es fest reingeschrieben.

Zitat:
Und den eigentlichen Kopiercode solltest du in eine eigene Methode auslagern und mit passenden Parametern aufrufen (anstatt Copy&Paste-Code).

Ich frage lieber nach als dass ich mir noch weitere doppelte Arbeit mache, die ich zur Genüge hatte.
Jeder Kopiercode in eine eigene Methode oder alle Kopieraktionen in eine Methode.
Könntest du mir das mit den passenden Parametern verdeutlichen?

Na da bin ich ja mal gespannt, ob es jetzt besser funktioniert.

Vielen Dank nochmal für deine Hilfe.

Grüße Tom

Moderiert von user profile iconTh69: C#-Tags hinzugefügt


Ralf Jansen - Di 17.05.22 13:13

Zitat:
Das kannst du laut sagen. Dazu gleich eine Frage das ich den RunWorkerAsync im Timer aufrufe ist ok? mit viele Sekunden sollte er ticken?

Das kann man kaum beantworten weil das Muster das du da benutzt eher undurchsichtig ist ;) Möglicherweise steckt da zuviel SPS denkweise drin.
Ein Timer der was aynchron startet wie hier ist schwer zu kontrollieren. Du versuchst dich ja schon mit abfragen von IsBusy dich zu schützen.
Möglicherweise erklärst du uns kurz warum da regelmäßig und scheinbar oft kopiert werden muß. Dann haven wir vermutlich ein passenderes Muster parat.


Zitat:
Wann und wo muss ich BW_Event.ReportProgress(0); aufrufen? Im DoWork nach dem kopieren?

Jedesmal wenn du die UI updaten willst. Der eigentlich Code gehört ja in DoWork und da bleibt für ReportProgress nur die Aufgabe irgendwo mitzuteilen wie weit der Fortschritt ist.
Wenn du nirgendwo den Fortschritt anzeigen willst mußt du ReportProgress im Zweifel gar nicht aufrufen/benutzen.
Zitat:
Jeder Kopiercode in eine eigene Methode oder alle Kopieraktionen in eine Methode.
Könntest du mir das mit den passenden Parametern verdeutlichen?

Deine Kopierschleifen sehen immer gleich aus bis auf das sie jedemal eine andere File Liste benutzen.
Dieser Code sollte einfach eine eigene Methode sein wo die FileListe reingeht. Dann hast du den immer gleichen Code auch tatsächlich nur einmal.

Und möglicherweise den Zielordner reinreichen bzw. aus einer Klassenvariablen so das du den vorher festlegen kannst. Im Moment holst du den direkt aus dem UI Control. Und wie wir schon festgestellt haben muß dieser Code später wieder aus DoWork aufgerufen wo keine UI Zugriffe stattfinden sollten.
Also am besten den Zielordner vor DoWork in einer Variablen speichern und von dort benutzen.

Da stellt sich mir auch die Frage ob du das überhaupt so willst. Würde das mit dem im Hintergrund ausführen tatsächlich so funktionieren könnte jeder zu jedem Zeitpunkt parallel das Ziel ändern.


UserTom - Di 17.05.22 14:02

Hallo Ralf,

Vielen Dank für deine Hilfe.

Zitat:
Möglicherweise erklärst du uns kurz warum da regelmäßig und scheinbar oft kopiert werden muß.

Alle 65s ist ein Bauteil fertig. Und mit dem Fertigsignal (Trigger) müssen von allen Applikations-PCs über Netzwerk
die Daten abgeholt werden. Auf dem PC wo die Winform läuft sind auch Daten die abgeholt werden müssen.
Der Zielpfad ist Path.Combine(@"D:\S7_Export\AppData", lblFolderName.Text). Das Label lblFolderName.Text ändert sich mit jedem Bauteil.

Der Timer ist dafür gedacht das er ständig die Variablen von der SPS überwacht damit das Triggersignal das kopieren anstossen kann.

Diese Methode ist zum abrufen der SPS Signal zuständig.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
        private S7VariablenResult LeakResult()
        {
            S7VariablenResult s7VariablenResult = new S7VariablenResult();
            // Reads the buffer.
            int dbnumber = short.Parse(txtDBNr.Text);
            int dboffset = short.Parse(txtDBOffsetNr.Text);
            int dbsize = short.Parse(txtDBSizeNr.Text);

            int PraegeCodeDBPos = short.Parse(txtPraegeCodeDBPos.Text);
            int FolderNameDBPos = short.Parse(txtFolderNameDBPos.Text);
            int PraegeCode2DBPos = short.Parse(txtPraegeCode2DBPos.Text);

            int TrigSaveFilesByte = short.Parse(txtTrigSaveFilesByte.Text);
            int TrigSaveFilesBit = short.Parse(txtTrigSaveFilesBit.Text);

            int TypLinksByte = short.Parse(txtTypLinksByte.Text);
            int TypLinksBit = short.Parse(txtTypLinksBit.Text);

            int TypRechtsByte = short.Parse(txtTypRechtsByte.Text);
            int TypRechtsBit = short.Parse(txtTypRechtsBit.Text);

            int RoboterDSLeerByte = short.Parse(txtRoboterDSLeerByte.Text);
            int RoboterDSLeerBit = short.Parse(txtRoboterDSLeerBit.Text);

            int FMBauteilByte = short.Parse(txtFMBauteilByte.Text);
            int FMBauteilBit = short.Parse(txtFMBauteilBit.Text);

            int HeartByte = short.Parse(txtHeartByte.Text);
            int HeartBit = short.Parse(txtHeartBit.Text);

            byte[] dbbuffersize = new byte[dbsize];

            s7Client.DBRead(dbnumber, dboffset, dbsize, dbbuffersize);

            s7VariablenResult.PraegeCodeNr = S7.GetStringAt(dbbuffersize, PraegeCodeDBPos);
            s7VariablenResult.FolderName = S7.GetStringAt(dbbuffersize, FolderNameDBPos);
            s7VariablenResult.PraegeCode2Nr = S7.GetStringAt(dbbuffersize, PraegeCode2DBPos);
            s7VariablenResult.TrigSaveFiles = S7.GetBitAt(dbbuffersize, TrigSaveFilesByte, TrigSaveFilesBit);
            s7VariablenResult.TypLinks = S7.GetBitAt(dbbuffersize, TypLinksByte, TypLinksBit);
            s7VariablenResult.TypRechts = S7.GetBitAt(dbbuffersize, TypRechtsByte, TypRechtsBit);
            s7VariablenResult.RoboterDSLeer = S7.GetBitAt(dbbuffersize, RoboterDSLeerByte, RoboterDSLeerBit);
            s7VariablenResult.FMBauteil = S7.GetBitAt(dbbuffersize, FMBauteilByte, FMBauteilBit);
            s7VariablenResult.HeartBeat = S7.GetBitAt(dbbuffersize, HeartByte, HeartBit);

            return s7VariablenResult;
        }


Zitat:
Wenn du nirgendwo den Fortschritt anzeigen willst mußt du ReportProgress im Zweifel gar nicht aufrufen/benutzen.

Nein einen Fortschrittbalken brauche ich nicht unbedingt. Aber ein paar SPS Variablen werden in Labels angezeigt.
Das kann vor dem kopieren passieren weil diese Variablen werden auch für das kopieren gebraucht. Oder danach so zu sagen als Complet Meldung.

Zitat:
Dieser Code sollte einfach eine eigene Methode sein wo die FileListe reingeht. Dann hast du den immer gleichen Code auch tatsächlich nur einmal.

Wenn ich dich richtig verstanden habe nehme ich alle meine Kopierbefehle und packe sie in eine Methode und kann so zum Beispiel auch die Variable für den Zielpfad mit übergeben. Die Quellpfade sollen über Textboxen als Variablen übergeben werden.

Vielen Dank nochmal für deine Antwort

Grüße Tom

Moderiert von user profile iconTh69: C#-Tags hinzugefügt


Ralf Jansen - Di 17.05.22 20:07

Zitat:
Wenn ich dich richtig verstanden habe nehme ich alle meine Kopierbefehle und packe sie in eine Methode und kann so zum Beispiel auch die Variable

Nein du packst einen Kopierbefehl in eine neue Methode und rufst diese Methode mehrmals auf. Ziel ist Code zu deduplizieren den du einfach fast identisch mehrmals hast.
Ein Beispiel mit Teilen deines Codes (nicht kompilierend weil hier im Editor "programmiert")


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
   private const string DefaultPath = "D:\S7_Export\AppData";
   private S7VariablenResult s7VariablenResult;
   
   private void EventLoop_Tick(object sender, EventArgs e)
   {
        if (!BW_Event.IsBusy)
        {
            // der ganze Quatsch mit UI Bezug hierhin 
            // kann sich so konkurierend zum Backgroundworker thread nicht ändern und ist damit Threadsafe
            s7VariablenResult = LeakResult(); 
        
            BW_Event.RunWorkerAsync();
        }
   }
   
   private void BW_Event_DoWork(object sender, DoWorkEventArgs e)
   {
        var bw = sender as Backgroundworker;

        var filter = $"*{s7VariablenResult.PraegeCodeNr}";
        var targetPath = Path.Combine(DefaultPath, s7VariablenResult.FolderName)
        Directory.CreateDirectory(targetPath);    
   
        bw.ReportProgress(0);
        CopyFiles(@"\\computer\PDE\tox10r01\AppData", targetPath , filter);
        bw.ReportProgress(10);
        CopyFiles(@"\\computer\PDE\tox15r01\AppData", targetPath , filter);      
        bw.ReportProgress(20);  
        // .usw
   }   
   
   private static void CopyFiles(string source, string target, string filter)
   {
        foreach (FileInfo file in new DirectoryInfo(source).GetFiles(filter))
            file.CopyTo(Path.Combine(target, file.Name));
   }


UserTom - Mi 18.05.22 10:51

Hallo Ralf,

Vielen Dank für deine Antwort und für den Code.

Dein Beispiel ändert alles und es war wieder falsch nur einen kleinen Abschnitt zur Verfügung zu stellen.
Allein wegen DoWork müsste ich so viel umschreiben. Meine ganze Arbeit umsonst, ich hätte euch früher fragen sollen.

Zitat:


C#-Quelltext
1:
private const string DefaultPath = "D:\S7_Export\AppData";                    



Kann ich über Konstanten alle meine Oberflächenelemente in eine Variable speichern?
Oder wie würdet Ihr das machen?

Ich habe in jeder Methode Zugriffe auf die Gui-Elemente, die ich gegen Variablen austauschen muss.
Sonst kann ich die Methoden ja nicht in Dowork aufrufen, oder?

Ich hänge mal mein eigentliches Projekt mit an, vielleicht kannst du mal hineinschauen und mir sagen, was ich ändern muss.
Ich habe 4 große Methoden drin für Dateien kopieren, Ordner kopieren, Dateien verschieben und Ordner verschieben.
Eine Methode heißt Ready, die ein Signal an die SPS zurückgibt, die wird nach dem Kopieren aufgerufen.
Ob noch im Backgroundworker oder anders weiß ich nicht wirklich.
Bitte nicht erschrecken über die vielen Wiederholungen ich hatte keine Ahnung von Deduplizieren oder besser gesagt
Ich wusste nicht, wie ich es hätte besser machen sollen. Ich bin froh über das, was ich geschafft habe.
Es läuft und macht genau, was es soll. Nur das, wenn das Kopieren angestoßen wird gibt es für 30 sek. keine Rückmeldung.
Es wäre schön, wenn du mir nur dabei hilfst den Backgroundworker da rein zu bekommen. Das Hauptproblem für mich sind
die Variablen für die Gui-Elemente, wenn ich das umgebaut habe kann ich die Methoden für das Kopieren im DoWork aufrufen.

Vielen Dank nochmal für deine Unterstützung.

Grüße Tom


Th69 - Mi 18.05.22 12:21

user profile iconUserTom hat folgendes geschrieben Zum zitierten Posting springen:
Dein Beispiel ändert alles und es war wieder falsch nur einen kleinen Abschnitt zur Verfügung zu stellen.
Allein wegen DoWork müsste ich so viel umschreiben. Meine ganze Arbeit umsonst, ich hätte euch früher fragen sollen.

Das Refactoring gehört zur täglichen Arbeit eines Software-Entwicklers dazu und manchmal sind es eben auch größere Codeänderungen (VS unterstützt aber z.B. das Auslagern in eigene Methoden sehr gut: Refactoring von Code [https://docs.microsoft.com/de-de/visualstudio/ide/refactoring-in-visual-studio]).

Zitat:
Kann ich über Konstanten alle meine Oberflächenelemente in eine Variable speichern?
Oder wie würdet Ihr das machen?

Ich habe in jeder Methode Zugriffe auf die Gui-Elemente, die ich gegen Variablen austauschen muss.
Sonst kann ich die Methoden ja nicht in Dowork aufrufen, oder?

So wie Ralf schon geschrieben hat, merke dir vor dem BW_Event.RunWorkerAsync() alle aus der GUI benötigten Werte in Membervariablen (bzw. erstelle dafür eine eigene Hilfsklasse).

PS: Gib mal keine Zahlen in deine Textboxen ein... (was ich meine: du solltest statt short.Parse besser TryParse verwenden, um Eingabefehler abzufangen).
Und warum verwendest du short.Parse, wenn du es einer int-Variablen zuweist? Benutze int.Parse.

PPS: Dein Code, gerade bzgl. deiner Control-Arrays, kommt mir irgendwie bekannt vor (Stichwort: dynamisches Erstellen von Controls). Du solltest dann aber auch die Arrays in deinem Code konsequent benutzen (also Schleifen, anstatt manuell die Controls, Zeile für Zeile, anzusprechen)!
Das würde dir deinen Code um mind. die Hälfte reduzieren. Merke dir außerdem, daß der Code in einer Klasse nur so max. 500-1000 Zeilen groß sein sollte, um noch überschaubar zu sein.
Die eigentlichen IO-Operationen solltest du in eine eigene Klasse auslagern und dann nur von deiner Form (mit passenden Parametern) aufrufen.


UserTom - Mi 18.05.22 13:29

Hallo TH69,

Vielen Dank für deine Antwort und die Ratschläge.

Zitat:

So wie Ralf schon geschrieben hat, merke dir vor dem BW_Event.RunWorkerAsync() alle aus der GUI benötigten Werte in Membervariablen


Ok, werde ich versuchen, umzusetzen.
Eine Frage noch dazu, wenn ich keine Informationen über das Kopieren haben möchte, kann ich doch BW_Event_ProgressChanged weglassen, oder?

Zitat:

(bzw. erstelle dafür eine eigene Hilfsklasse).


Das wäre perfekt aber nichts für mich ich bekomme es nicht hin wie die Klassen mit der Form kommunizieren.
Alle Klassendateien, die in diesem Projekt sind, habe ich als Copy-and-paste aus dem Internet.
Außer die Settings.cs, da hat mir jemand aus einem Forum bei geholfen.

Zitat:

Und warum verwendest du short.Parse, wenn du es einer int-Variablen zuweist? Benutze int.Parse.


Vielen Dank, das habe ich von einer Internetseite so übernommen. Ich habe es angepasst.
Da es keinen Fehler ausgelöst hat, bin ich davon ausgegangen, dass das so richtig ist.

Zitat:

PPS: Dein Code, gerade bzgl. deiner Control-Arrays, kommt mir irgendwie bekannt vor (Stichwort: dynamisches Erstellen von Controls). Du solltest dann aber auch die Arrays in deinem Code konsequent benutzen (also Schleifen, anstatt manuell die Controls, Zeile für Zeile, anzusprechen)!
Das würde dir deinen Code um mind. die Hälfte reduzieren. Merke dir außerdem, dass der Code in einer Klasse nur so max. 500-1000 Zeilen groß sein sollte, um noch überschaubar zu sein.


Wie mit den Hilfsklassen bin ich mit Schleifen erstellen überfordert. Ich bastle jetzt schon seit über 1 Jahr an diesem Projekt.
Ich habe nicht immer Zeit dafür. Dann wollte ich in den Foren nicht so viel nachfragen, weil ich wollte es alleine schaffen.
Das Ergebnis siehst du. Ich hoffe, ich bekomme das mit dem Backgroundworker noch hin.

Zitat:

Die eigentlichen IO-Operationen solltest du in eine eigene Klasse auslagern und dann nur von deiner Form (mit passenden Parametern) aufrufen.

Würde ich gerne, aber daran verzweifle ich. Habe ich ja weiter oben schon erwähnt.

Ich beneide jeden von euch. Wenn ich das vor 30 Jahren schon gewusst hätte, dann hätte ich einen anderen Weg eingeschlagen.


Grüße Tom


Ralf Jansen - Mi 18.05.22 15:20

Zitat:
Eine Frage noch dazu, wenn ich keine Informationen über das Kopieren haben möchte, kann ich doch BW_Event_ProgressChanged weglassen, oder?

Ja. Wenn du kein Progress brauchst mußt du die Funktion für den Progress nicht aufrufen ;)
Zitat:
Das wäre perfekt aber nichts für mich ich bekomme es nicht hin wie die Klassen mit der Form kommunizieren.

a.) Vermutlich eher andersherum. Die Form kommuniziert mit der Klasse die die Funktionalität enthält.
b.) Und das geht dann ungefähr genauso wie du das jetzt schon tust ;) Deine S7VariablenResult Klasse ist sowas. Diese enthält vermutlich gerade nur Daten könnte aber genauso gut Logik enthalten.
Zitat:
Da es keinen Fehler ausgelöst hat, bin ich davon ausgegangen, dass das so richtig ist.

Das besagt das es mal gehen kann ist aber was anderes als immer ;)


Th69 - Mi 18.05.22 15:23

Ja, das ProgressChanged-Event kannst du weglassen, wenn du es nicht brauchst. Da der Kopiervorgang jedoch bei dir ca. 30s benötigt, wie du schreibst, wäre es doch trotzdem gut, diesen (als Rückmeldung) anzuzeigen (und wenn es nur vor bzw. nach jeden der Kopierschleifen ist).

Und mit Hilfsklasse für die Speicherung der UI-Elemente meine ich einfach soetwas:

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
// als Unterklasse in deiner Form-Klasse
class UIValues
{
  public bool Checked1 { get; set; }
  // ...
}

private UIValues UIValues = new();

Und dann (genauso wie bei der S7VariablenResult-Klasse) entsprechend die Werte setzen:

C#-Quelltext
1:
UIValues.Checked1 = chkActivateNetApp1.Checked;                    

(auch hier solltest du dann ein Array für Checked benutzen, s.u.).
Und bei den Kopierschleifen greifst du dann auf UIValues.CheckedX (bzw. als Array auf UIValues.Checked[n] zu).

Und bzgl. der Schleifen hier zwei Code-Verbesserungen:
1. statt Z. 119-165

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
const int MaxLocalApps = 8;
for (int n = 0; n < MaxLocalApps; n++)
{
    cboLocalApps[n].Enabled = true;
    tbSourceFilePathLocalApps[n].Enabled = true;
    cbActivateLocalApps[n].Enabled = true;
    cbLocalSubFolderUses[n].Enabled = true;
    cbActivateCode2LocalApps[n].Enabled = true;
}

(also Code-Ersparnis (= 47 - 9) 38 Zeilen!)
Und dasselbe dann für die NetApps.

2. statt 223 -233:

C#-Quelltext
1:
2:
3:
4:
for (int n = 0; n < MaxLocalApps; n++)
{
    cboLocalApps[n].Items.AddRange(txtLocalApplication.Lines);  
}

Und dasselbe dann für die NetApps darunter.

Und die gleiche Art von Schleife dann in SaveLocalPathConfigSettings() sowie SaveNetPathConfigSettings(). Idealerweise, da es derselbe Code ist, diese Schleife(n) in eine eigene Methode auslagern.

Und auch die ganzen txtXXX.Enabled-Aufrufe könnte man einfacher mittels einer Schleife über das Controls-Array lösen.
Aber als erstes würde es reichen, den Code dafür in eine eigene Methode auszulagern (da du den Codeblock mehrfach hast):

C#-Quelltext
1:
2:
3:
4:
5:
6:
private void EnableTextBoxes(bool enable)
{
  txtIPNr.Enabled = enable;
  // ...
  txtHeartBit.Enabled = enable;
}

Und dann nur EnableTxtBoxes(true) bzw. EnableTextBoxes(false) aufrufen. ;-)

Wenn du dann noch die von Ralf geschriebene Code-Auslagerung der Kopierfunktionen machst, dann ist dein Code (einigermaßen) aufgeräumt und lesbar.

Schau dir am besten in deinem C#-Buch bzw. Arrays (C#-Programmierhandbuch) [https://docs.microsoft.com/de-de/dotnet/csharp/programming-guide/arrays/] ff., wie man Arrays im Zusammenhang mit Schleifen benutzt.


UserTom - Do 19.05.22 09:58

Servus,

Vielen Dank an Euch für Eure Hilfe, Ideen und die Codebeispiele.

Zitat:
a.) Vermutlich eher andersherum. Die Form kommuniziert mit der Klasse, die die Funktionalität enthält.

Da kannst Du mal sehen, was für eine Ahnung ich habe.

Zitat:
Und dann (genauso wie bei der S7VariablenResult-Klasse) entsprechend die Werte setzen:

Wie Du siehst, sind da nur Variablen drin. Ich habe leider keine Klassendatei, die ich als Vorlage nehmen könnte.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
namespace PDE
{
    // Variablen von der SPS
    class S7VariablenResult
    {
        public string PraegeCodeNr;       // Prägecode Hauptteil
        public string FolderName;       // Ordnername
        public string PraegeCode2Nr;      // Prägecode Zusatzteil
        public bool TrigSaveFiles;      // PDE Daten kopieren
        public bool TypLinks;           // Typ Links
        public bool TypRechts;          // Typ Rechts
        public bool RoboterDSLeer;      // Roboter hat kein Teil im Greifer
        public bool FMBauteil;          // Fertigmeldung Bauteil
        public bool HeartBeat;          // Heartbeat
    }
}


Zitat:
Und mit Hilfsklasse für die Speicherung der UI-Elemente meine ich einfach soetwas:


Also um ganz klein anzufangen könntest Du mir bitte erklären wie ich für die Textbox txtDBNr.Text über die Klasse UIValues eine Variable zur
Verfügung stelle.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
private void Ready()
        {
            int S7AreaDB = 0x84;
            int dbnumber = int.Parse(txtDBNr.Text);
            int TrigSaveFilesByte = int.Parse(txtTrigSaveFilesByte.Text);
            int TrigSaveFilesBit = int.Parse(txtTrigSaveFilesBit.Text);
            int dboffset = (TrigSaveFilesByte * 8) + TrigSaveFilesBit;
            int S7WLBit = 0x01;
            byte[] buffersize = new byte[1];

            int Result = s7Client.WriteArea(S7AreaDB, dbnumber, dboffset, 1, S7WLBit, buffersize);
            ShowResult(Result);

            EventLoop.Enabled = true;
        }


Hier habe ich Deinen Anfang. Ich werde es mit Google weiter versuchen.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
namespace PDE
{
    class UIValues
    {
       public int DBNr { get; set; }
      
    }

    private UIValues UIValues = new();
}




C#-Quelltext
1:
private UIValues UIValues = new();                    

Das kann ich nicht benutzen, mein Projekt ist mit NET Framework 4.8 erstellt. Dafür brauche ich C#9 .NET Core 5
Wie würdest Du NET Framework 4.8 in .NET Core 5 konvertieren?

Zitat:
Und bzgl. der Schleifen hier zwei Code-Verbesserungen:

Vielen Dank, das ist sehr zuvorkommend von Dir.
Das habe ich mir auch als Anreiz für die for Scheifen für das Kopieren genommen. Siehe weiter unten.

Zitat:
Und die gleiche Art von Schleife dann in SaveLocalPathConfigSettings() sowie SaveNetPathConfigSettings(). Idealerweise, da es derselbe Code ist, diese Schleife(n) in eine eigene Methode auslagern.

Da weiß ich nicht genau, worauf Du Dich beziehst. Kannst Du mir die Zeilennummer bitte sagen?

Zitat:
Und auch die ganzen txtXXX.Enabled-Aufrufe könnte man einfacher mittels einer Schleife über das Controls-Array lösen.

Ich tue mich sehr schwer damit, ich habe gute 20min gerätselt, wie Du das meinst. Das = enable hat mich total verwirrt.
Im Anhang ist dann der neu komplette Code. Du kannst ja mal schauen, ob Du das so gemeint hattest.

Zitat:
Wenn du dann noch die von Ralf geschriebene Code-Auslagerung der Kopierfunktionen machst, dann ist dein Code (einigermaßen) aufgeräumt und lesbar.

Ich habe schon angefangen, habe aber mit einer Variable Probleme.
Die Idee mit der Konstante von Ralf für den Defaultpath ist super und habe ich auch umgesetzt.

Da ist das i von der For-Schleife mit drin und somit geht das nicht mit Konstanten. Oder?

C#-Quelltext
1:
string sourceFilePath = tbSourceFilePathLocalApps[i].Text;                    



C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48:
49:
50:
51:
52:
53:
54:
55:
56:
57:
58:
59:
60:
61:
62:
63:
64:
65:
66:
67:
68:
        private const string DEFAULTTARGETPATH = @"D:\S7_Export\AppData";

        private void BW_Event_DoWork(object sender, DoWorkEventArgs e)
        {
            try
            {
                S7VariablenResult s7VariablenResult = LeakResult();

                string createTargetPath = "";

                if (s7VariablenResult.TrigSaveFiles && (s7VariablenResult.TypLinks || s7VariablenResult.TypRechts) && !s7VariablenResult.FMBauteil)
                {
                    createTargetPath = Path.Combine(@"D:\S7_Export\AppData", s7VariablenResult.FolderName);

                    try
                    {
                        //Ordner erstellen
                        if (!Directory.Exists(createTargetPath))
                        {
                            Directory.CreateDirectory(createTargetPath);
                        }
                    }

                    catch (Exception ex)
                    {
                        MessageBox.Show(this, ex.Message, "Ordner erstellen fehlgeschlagen!", MessageBoxButtons.OK, MessageBoxIcon.Error);
                    }

                    for (int i = 0; i < cbActivateLocalApps.Length; i++)
                    {
                        if (cbActivateLocalApps[i].Checked == true)
                        {
                            string praegeCodeNrFolderName = s7VariablenResult.FolderName;
                            string praegeCodeNrFileName = s7VariablenResult.PraegeCodeNr;
                            string praegeCode2NrFileName = s7VariablenResult.PraegeCode2Nr;
                            string sourceFilePath = tbSourceFilePathLocalApps[i].Text;
                            string targetPath = DEFAULTTARGETPATH;

                            MoveFilesfromLocal(praegeCodeNrFolderName, praegeCodeNrFileName, sourceFilePath, targetPath);
                            MoveFilesCode2fromLocal(praegeCodeNrFolderName, praegeCode2NrFileName, sourceFilePath, targetPath);
                        }
                    }

                    for (int i = 0; i < cbActivateNetApps.Length; i++)
                    {
                        if (cbActivateNetApps[i].Checked == true)
                        {
                            string praegeCodeNrFolderName = s7VariablenResult.FolderName;
                            string praegeCodeNrFileName = s7VariablenResult.PraegeCodeNr;
                            string praegeCode2NrFileName = s7VariablenResult.PraegeCode2Nr;
                            string sourceFilePath = tbSourceFilePathNetApps[i].Text;
                            string targetPath = DEFAULTTARGETPATH;

                            CopyFilesfromNetwork(praegeCodeNrFolderName, praegeCodeNrFileName, sourceFilePath, targetPath);
                        }
                    }

                }
            }
            catch (Exception ex)
            {
                MessageBox.Show("Error:\n\n" + ex.Message, "System", MessageBoxButtons.OK, MessageBoxIcon.Error);
            }

            Ready();

            BW_Event.ReportProgress(0);
        }

Ich weiß nicht, ob das so gut ist, wie ich das umgesetzt habe. Ich habe zwar nur noch 3 Methoden aber könnte es nicht sein das
durch diese ganzen For-Schleifen der Code langsamer wird???


Vielen Dank nochmal für alles.

Grüße Tom


Th69 - Do 19.05.22 12:35

Statt nur new() mußt du bei älteren C#-Versionen noch den Typen hinschreiben:

C#-Quelltext
1:
private UIValues UIValues = new UIValues();                    

Aber ich schrieb

C#-Quelltext
1:
// als Unterklasse in deiner Form-Klasse                    
(also direkt in deiner MainForm-Klasse einfügen, damit UIValues auch ein Member der Klasse wird - nicht außerhalb nur im Namespace, das erzeugt einen Compilerfehler).

Und für dbnumber erzeugst du dann eine weitere Eigenschaft (oder simpel als Feld) in der UIValues und setzt diesen Wert entsprechend.

In SaveLocalPathConfigSettings() und SaveNetPathConfigSettings() hast du doch schon die Schleife (mit MAXLOCALAPPS bzw. MAXNETAPPS) jetzt eingebunden. Aber da du hier auch den Code einfach kopiert hast, könntest du eben auch dafür passende Methoden EnableLocalAppControls() und EnableNetAppControls() erstellen.

Und das mit dem enable ist doch einfach zu verstehen. Dort kommt beim Aufruf entweder true oder false rein, d.h. du benötigst nicht zwei Methoden zum Setzen, sondern eben nur eine.
Mit EnableTextBoxes(true) setzt du alle Textboxen textBoxX.Enabled = true und mit EnableTextBoxes(false) entsprechend textBoxX.Enabled = false (d.h. = disabled).
Du kannst also die Methode DisableTextBoxes() entfernen und statdessen EnableTextBoxes(false) benutzen.

Bei

C#-Quelltext
1:
string sourceFilePath = tbSourceFilePathLocalApps[i].Text;                    
kannst du keine Konstante benutzen, sondern müßtest auch diesen Wert in die UIValues-Klasse packen, d.h. hier als Array

C#-Quelltext
1:
public string[] SourceFilePathLocalApps = new string[MaxLocalApps]; // die Konstante MaxLocalApps dafür als Klassenmember anlegen!                    

Und jetzt dann entsprechend vorher (in einer Schleife) die Werte zuweisen (und dasselbe für alle anderen Controlwerte, welche du für den Kopiervorgang benötigst - so wie du es ja für die S7VariablenResult-Werte auch gemacht hast).

Und die Frage bzgl. "Code wird langsamer" finde ich immer wieder erstaunlich bei Anfängern. Die meiste Prozessorzeit wird hier doch sowieso durch die IO-Operationen benötigt.
Es geht um die Lesbarkeit von Code (und je kürzer und eleganter, desto besser - im Hinblick auch auf Wiederverwendbarkeit und Wartbarkeit von Code).

PS: Ansonsten sieht dein Code aber schon aufgeräumter und lesbarer aus (hast ja auch nur noch ca. 900 statt 2500 Zeilen)!
Als letzte größere Codeoptimierung könntest du noch den !string.IsNullOrEmpty(...) && ...-Abfrageblock als eigene Methode bool AreAllTextBoxesFilled() auslagern (da du ihn bisher 4x benutzt).


UserTom - Do 19.05.22 21:33

Hallo Th69,

Vielen Dank für deine Hilfe.

Zitat:
(also direkt in deiner MainForm-Klasse einfügen, damit UIValues auch ein Member der Klasse wird)

Also ich habe jetzt für die Textbox txtDBNr.text eine Variable über die Klasse erstellt und hoffe ich habe das richtig gemacht.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
namespace PDE
{
    class UIValues
    {
        private string dbnr;
        public string DBNr
        {
            get { return dbnr; }
            set { dbnr = value; }
        }        
    }
}



C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
namespace PDE
{

    public partial class MainForm : Form
    {
        private S7Client s7Client;

        private ComboBox[] cboNetApps;
        private TextBox[] tbSourceFilePathNetApps;
        private CheckBox[] cbActivateNetApps;
        private CheckBox[] cbNetSubFolderUses;
        private CheckBox[] cbActivateCode2NetApps;

        private const string DEFAULTTARGETPATH = @"D:\S7_Export\AppData";  
      
################################################################################
        private UIValues uivalues = new UIValues();        
################################################################################

        public MainForm()
        {
            InitializeComponent();


Ist das richtig hier im MainForm_Load die Variable zu initialsieren?
Ich habe mit einer MessageBox kontroliert ob ein Wert drin steht.
Es funktioniert aber das heißt noch lange nicht das es richtig ist, so wie es sein sollte.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
 private void MainForm_Load(object sender, EventArgs e)
        {
            GetLocalPathConfigSettings();
            GetNetPathConfigSettings();
            GetSPSConfigurationSettings();

            const int MAXLOCALAPPS = 8;
            for (int n = 0; n < MAXLOCALAPPS; n++)
            {
                cboLocalApps[n].Items.AddRange(txtLocalApplication.Lines);
            }

            const int MAXNETAPPS = 8;
            for (int n = 0; n < MAXNETAPPS; n++)
            {
                cboNetApps[n].Items.AddRange(txtNetApplication.Lines);
            }

            BW_Event.WorkerReportsProgress = true;
            BW_Event.WorkerSupportsCancellation = true;

            uivalues.DBNr = txtDBNr.Text;
            MessageBox.Show(uivalues.DBNr);
        }



Hier wird sie dann später genutzt.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
 private void Ready()
        {
            int S7AreaDB = 0x84;

####################################################################################
            int dbnumber = int.Parse(uivalues.DBNr);
####################################################################################

            int TrigSaveFilesByte = int.Parse(txtTrigSaveFilesByte.Text);
            int TrigSaveFilesBit = int.Parse(txtTrigSaveFilesBit.Text);
            int dboffset = (TrigSaveFilesByte * 8) + TrigSaveFilesBit;
            int S7WLBit = 0x01;
            byte[] buffersize = new byte[1];

            int Result = s7Client.WriteArea(S7AreaDB, dbnumber, dboffset, 1, S7WLBit, buffersize);
            ShowResult(Result);

            EventLoop.Enabled = true;
        }


Zitat:
In SaveLocalPathConfigSettings() und SaveNetPathConfigSettings() hast du doch schon die Schleife (mit MAXLOCALAPPS bzw. MAXNETAPPS) jetzt eingebunden. Aber da du hier auch den Code einfach kopiert hast, könntest du eben auch dafür passende Methoden EnableLocalAppControls() und EnableNetAppControls() erstellen.

Und die Schleifen packe ich in die Methoden?

Zitat:
Und das mit dem enable ist doch einfach zu verstehen.

Ich habe es ja jetzt verstanden. Vielen Dank. Du beherrscht das alles aus dem FF. Ich brauche sehr lange. Du kannst mir glauben,
wenn es am einfachsten ist, mache ich es zu kompliziert.

Zitat:
Und jetzt dann entsprechend vorher (in einer Schleife) die Werte zuweisen

Hier möchte ich dich bitten es mir genauer zu beschreiben. Was gehört in die Klasse was in die Main und wo muss ich was für eine Schleife aufbauen?

Zitat:
Als letzte größere Codeoptimierung könntest du noch den !string.IsNullOrEmpty(...) && ...-Abfrageblock als eigene Methode bool AreAllTextBoxesFilled() auslagern (da du ihn bisher 4x benutzt).

Ja habe ich um setzen können.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
        private bool AreAllTextBoxesFilled()
        {
            if (!string.IsNullOrEmpty(txtIPNr.Text)
                && !string.IsNullOrEmpty(txtRackNr.Text)
                && !string.IsNullOrEmpty(txtSlotNr.Text)
                && !string.IsNullOrEmpty(txtDBNr.Text)
                && !string.IsNullOrEmpty(txtDBOffsetNr.Text)
                && !string.IsNullOrEmpty(txtDBSizeNr.Text)
                && !string.IsNullOrEmpty(txtPraegeCodeDBPos.Text)
                && !string.IsNullOrEmpty(txtFolderNameDBPos.Text)
                && !string.IsNullOrEmpty(txtPraegeCode2DBPos.Text)
                && !string.IsNullOrEmpty(txtTrigSaveFilesByte.Text)
                && !string.IsNullOrEmpty(txtTrigSaveFilesBit.Text)
                && !string.IsNullOrEmpty(txtTypLinksByte.Text)
                && !string.IsNullOrEmpty(txtTypLinksBit.Text)
                && !string.IsNullOrEmpty(txtTypRechtsByte.Text)
                && !string.IsNullOrEmpty(txtTypRechtsBit.Text)
                && !string.IsNullOrEmpty(txtRoboterDSLeerByte.Text)
                && !string.IsNullOrEmpty(txtRoboterDSLeerBit.Text)
                && !string.IsNullOrEmpty(txtFMBauteilByte.Text)
                && !string.IsNullOrEmpty(txtFMBauteilBit.Text)
                && !string.IsNullOrEmpty(txtHeartByte.Text)
                && !string.IsNullOrEmpty(txtHeartBit.Text))
            {
                return true;
            }

            return false;
        }



C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
private void Connect_Click(object sender, EventArgs e)
{
    ######################################################################
    if (AreAllTextBoxesFilled() == true)
    ######################################################################
{
    int Result;
    int Rack = Convert.ToInt32(txtRackNr.Text);
    int Slot = Convert.ToInt32(txtSlotNr.Text);
    Result = s7Client.ConnectTo(txtIPNr.Text, Rack, Slot); // 0 means Success
    ShowResult(Result); // The status bar result display
}


Eine Frage noch die for-Schleifen für das kopieren aufrufen und die Methoden für das Kopieren sind in Ordnung so?

So nochmals vielen Dank für deine Unterstützung

Grüße Tom


Th69 - Do 19.05.22 23:35

Die UIValues-Werte solltest du erst kurz vor Aktivierung des BackgroundWorkers ermitteln (am besten wohl in Connect_Click bevor EventLoop.Enabled = true gesetzt wird). In MainForm_Load ist ja gerade erst die Form erzeugt worden und der Anwender kann noch gar nicht die TextBoxen gefüllt haben.

Ansonsten ist dein Code aber vom Aufbau richtig.

Das Parsen der Eingabe solltest du aber auch schon vorher machen, d.h. speichere den Wert schon direkt als int ab (anstatt string).
Wenn du jedoch generell nur Zahlen in der Eingabe zulassen willst, dann solltest du gleich das NumericUpDown [https://docs.microsoft.com/de-de/dotnet/api/system.windows.forms.numericupdown]-Steuerelement benutzen (da kannst du dann auch Min- und Maxwerte vorgeben). Oder aber du benutzt meine dazu erstellte Komponente NumericTextBox (basierend auf dem Windows-Standard-Control) [https://mycsharp.de/forum/threads/80261/numerictextbox-basierend-auf-dem-windows-standard-control].

Läuft denn dein Programm bisher ohne Exceptions? Es kann sein, daß ein lesender Zugriff auf die Controls aus dem BackgroundWorker funktioniert - auch wenn es nicht zu empfehlen ist und du den Weg über die UIValues konsequent umsetzen solltest (auch wenn es einige Daten sind).
Eigentlich würde ich dafür sogar empfehlen, den gesamten BackgroundWorker-Code in eine eigene Klasse auszulagern, weil dann würde man zuerst Fehlermeldungen bei der Verwendung der Form-Controls erhalten (welche man dann sukzessive ersetzen würde), aber das mute ich dir jetzt (noch) nicht zu...

UserTom hat folgendes geschrieben:
Und die Schleifen packe ich in die Methoden?

Ja, genau. Wie schon geschrieben kannst du dafür auch das VS-Refactoring benutzen. Selektiere den gesamten Schleifencode und öffne dann das Kontextmenü und wähle "Quick Actions and Refactorings" (Ctrl+.) und dann "Extract Method" und schon wird der Code in eine neue Methode erstellt, dessen Namen man gleich vergeben kann.

Und an der anderen (äquivalenten) Codestelle mußt du dann selbstverständlich diese Schleife löschen und die Methode stattdessen aufrufen.

Der bisherige Umbau der Kopieraktionen mit den for-Schleifen und das Aufteilen in die einzelnen Methoden sieht gut aus (auch wenn man ihn sicherlich noch optimieren könnte). :zustimm:


UserTom - Fr 20.05.22 08:40

Servus Th69,

Vielen Dank für deine Antwort.

Zitat:
Die UIValues-Werte solltest du erst kurz vor Aktivierung des BackgroundWorkers ermitteln

Ok habe ich geändert.

Ich kann das leider nicht richtig abschließen, weil ich nicht weiß wie ich

C#-Quelltext
1:
string sourceFilePath = tbSourceFilePathLocalApps[i].Text;                    


In die UIValues.cs einbinden muss.

Zitat:


C#-Quelltext
1:
public string[] SourceFilePathLocalApps = new string[MaxLocalApps]; // die Konstante MaxLocalApps dafür als Klassenmember anlegen!                    


Ich vermute, das kommt in die Klasse?

Kannst du mir das bitte genauer erklären?

Zitat:
Das Parsen der Eingabe solltest du aber auch schon vorher machen, d.h. speichere den Wert schon direkt als int ab (anstatt string).

Wie meinst du das mit vorher?
Du meinst die s7VariablenResult oder?
Wenn ich die gleich als int abspeichere, dann muss ich aber die Textboxen gegen NumericUpDown austauschen.
Ahh toll die Werte vom NumericUpDown sind decimal die müssen auch konvertiert werden. Da haben wir ja nichts gewonnen!

Du meinst das bestimmt anders mit den direkt abspeichern? Ein Beispiel wäre nicht schlecht!


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
            S7VariablenResult s7VariablenResult = new S7VariablenResult();
            // Reads the buffer.
            int dbnumber = int.Parse(txtDBNr.Text);
            int dboffset = int.Parse(txtDBOffsetNr.Text);
            int dbsize = int.Parse(txtDBSizeNr.Text);

            int PraegeCodeDBPos = int.Parse(txtPraegeCodeDBPos.Text);
            int FolderNameDBPos = int.Parse(txtFolderNameDBPos.Text);
            int PraegeCode2DBPos = int.Parse(txtPraegeCode2DBPos.Text);

            int TrigSaveFilesByte = int.Parse(txtTrigSaveFilesByte.Text);
            int TrigSaveFilesBit = int.Parse(txtTrigSaveFilesBit.Text);

            int TypLinksByte = int.Parse(txtTypLinksByte.Text);
            int TypLinksBit = int.Parse(txtTypLinksBit.Text);

            int TypRechtsByte = int.Parse(txtTypRechtsByte.Text);
            int TypRechtsBit = int.Parse(txtTypRechtsBit.Text);

            int RoboterDSLeerByte = int.Parse(txtRoboterDSLeerByte.Text);
            int RoboterDSLeerBit = int.Parse(txtRoboterDSLeerBit.Text);

            int FMBauteilByte = int.Parse(txtFMBauteilByte.Text);
            int FMBauteilBit = int.Parse(txtFMBauteilBit.Text);

            int HeartByte = int.Parse(txtHeartByte.Text);
            int HeartBit = int.Parse(txtHeartBit.Text);


Zitat:
Läuft denn dein Programm bisher ohne Exceptions?


Es startet und ich kann es auch mit der SPS verbinden (Connect_Click).
Da ich noch das UIValues Problem habe, konnte ich noch nicht mehr testen.

Zitat:
Möglicherweise steckt da zu viel SPS Denkweise drin.

Ralf hatte recht. Ich weiß nicht, ob ihr schon mal einen AWL Code gesehen habt, der geht von oben nach unten.
Der jetzt optimierte Code ist natürlich besser aber ich muss mich jetzt erstmal dran gewöhnen das sehr viele schleifen enthalten sind
und mir das Nachverfolgen erschwert.

Vielen Dank nochmal für deine Tipps.

Grüße Tom


Th69 - Fr 20.05.22 10:52

UserTom hat folgendes geschrieben:
Ich vermute, das kommt in die Klasse?

Ja, in die UIValues-Klasse.

Und anstatt eines einzelnen Wertes setzt du dann in einer Schleife alle Array-Werte:

C#-Quelltext
1:
2:
for (int n = 0; n < UIValues.SourceFilePathLocalApps.Length; n++)
  UIValues.SourceFilePathLocalApps[n] = tbSourceFilePathLocalApps[n].Text;


Und bzgl. int:
Bisher speicherst du ja DBNr als String ab und benutzt int.Parse(uivalues.DBNr) erst in der Ready()-Methode.
Wenn du aber direkt schon den geparsten Wert als int in die UIValues abspeicherst, brauchst du nachher keine Umwandlung mehr (gilt auch für die anderen Werte).
Und wie ich schon schrieb, solltest du besser int.TryParse(...) benutzen, damit du Eingabefehler besser abfangen kannst (ansonsten kommt nämlich eine InvalidCastException und das gesamte Programm wird beendet, wenn du keinen try..catch-Block benutzt).

NumericUpDown verwendet decimal für Value, weil man es auch mit Nachkommastellen (DecimalPlaces) konfigurieren kann.
Man kann aber einfach int value = (int)numericUpDown.Value schreiben (also eine explizite Umwandlung [Cast]).

Dies alles dient auch dazu UI und Logik (Datenmodellierung) voneinander zu trennen (eines der wichtigsten Konzepte in der Software-Entwicklung).

PS: Was meinst du eigentlich mit Leak [https://dict.leo.org/englisch-deutsch/leak] in LeakResult?

PPS: Ich habe deine Zitate jeweils editiert, damit kein unnötiger Zeilenumbruch dort angezeigt wird (der Text sollte direkt nach dem `Quote`-Tag stehen, nicht in einer neuen Zeile).


Ralf Jansen - Fr 20.05.22 12:47

Zitat:
PS: Was meinst du eigentlich mit Leak in LeakResult?


Da ich so gerne spekuliere. (Un)Dichteprüfung in einem Rohrwerk?


UserTom - Fr 20.05.22 13:58

Hallo an alle,

Vielen Dank für Eure Antworten.

Was Leak bedeutet, weiß ich auch nicht. Ich habe den Beispielcode von einer Internetseite. Siehe im Anhang das Bild.

Zitat:
Und anstatt eines einzelnen Wertes setzt du dann in einer Schleife alle Array-Werte:

Ich habe das jetzt folgendermaßen gelöst. Ich hoffe, es ist richtig.

In der Klasse.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
namespace PDE
{
    class UIValues
    {
        private string[] _sourcefilepathlocalapps;
        public string[] SourceFilePathLocalApps
        {
            get { return _sourcefilepathlocalapps; }
            set { _sourcefilepathlocalapps = value; }
        }

        private string[] _sourcefilepathnetapps;
        public string[] SourceFilePathNetApps
        {
            get { return _sourcefilepathnetapps; }
            set { _sourcefilepathnetapps = value; }
        }


Im Connect_Click:

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
      uivalues.FMBauteilByte = txtFMBauteilByte.Text;
      uivalues.FMBauteilBit = txtFMBauteilBit.Text;
      uivalues.HeartByte = txtHeartByte.Text;
      uivalues.HeartBit = txtHeartBit.Text;

      for (int i = 0; i < uivalues.SourceFilePathLocalApps.Length; i++)
        uivalues.SourceFilePathLocalApps[i] = tbSourceFilePathLocalApps[i].Text;

      for (int i = 0; i < uivalues.SourceFilePathNetApps.Length; i++)
        uivalues.SourceFilePathNetApps[i] = tbSourceFilePathNetApps[i].Text;

      ScanDBOffset.Enabled = true;
      EventLoop.Enabled = true;


In der Schleife im DoWork.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
for (int i = 0; i < cbActivateLocalApps.Length; i++)
{
    if (cbActivateLocalApps[i].Checked == true)
    {
        string praegeCodeNrFolderName = s7VariablenResult.FolderName;
        string praegeCodeNrFileName = s7VariablenResult.PraegeCodeNr;
        string praegeCode2NrFileName = s7VariablenResult.PraegeCode2Nr;
        string sourceFilePath = uivalues.SourceFilePathLocalApps[i];
        string targetPath = DEFAULTTARGETPATH;

        MoveFilesfromLocal(praegeCodeNrFolderName, praegeCodeNrFileName, sourceFilePath, targetPath);
        MoveFilesCode2fromLocal(praegeCodeNrFolderName, praegeCode2NrFileName, sourceFilePath, targetPath);
    }
}


Zitat:
Und bzgl. int:

Ich habe Folgendes probiert, aber ich bekomme es nicht hin.

In der Klasse UIValues.TryParse verstehe ich noch weniger, da muss ich noch einen Wert mehr übergeben??

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
private string _dbnr;
public int DBNr
{
    get { return int.Parse(_dbnr); }

    set { _dbnr = value.ToString(); }
}



C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
uivalues.IPNr = txtIPNr.Text;
uivalues.RackNr = txtRackNr.Text;
uivalues.SlotNr = txtSlotNr.Text;

#########################################################################
uivalues.DBNr = txtDBNr.Text;  << Ist rot unterstrichen
########################################################################

uivalues.Offset = txtDBOffsetNr.Text;
uivalues.DBSize = txtDBSizeNr.Text;


In der Methode Ready():

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
private void Ready()
{
    int S7AreaDB = 0x84;

    #####################################################
    int dbnumber = uivalues.DBNr;
    #####################################################

    int TrigSaveFilesByte = int.Parse(uivalues.TrigSaveFilesByte);


Vielen Dank für die Antworten.

Grüße Tom

Moderiert von user profile iconTh69: Leerzeile aus Zitaten entfernt
Moderiert von user profile iconTh69: C#-Tags hinzugefügt


Th69 - Fr 20.05.22 14:30

Bei deinen String-Arrays fehlt noch die Erzeugung des Arrays (mittels new[...]).
Ich habe doch extra folgendes geschrieben:

C#-Quelltext
1:
public string[] SourceFilePathLocalApps = new string[MaxLocalApps]; // die Konstante MaxLocalApps dafür als Klassenmember anlegen!                    


Für einfache Datenhaltungsklassen, wie eben UIValues, solltest du die automatischen Eigenschaften ({ get; set; }) benutzen (solange du keine eigene Logik im Getter oder Setter hast).

Auch bei DBNr. Dein umgeänderter Code kann so nicht funktionieren, da du jetzt einen String einem int-Wert zuweisen möchtest.

Ich meinte einfach

C#-Quelltext
1:
2:
3:
4:
class UIValues
{
  public int DBNr { get; set; }
}

Und dann in der Zuweisungsmethode

C#-Quelltext
1:
uivalues.DBNr = int.Parse(txtDBNr.Text);                    

bzw. mit TryParse

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
if (int.TryParse(txtDBNr.Text, out int dbnr))
{
  uivalues.DBNr = dbnr;
}
else
{
  // Fehlermeldung ausgeben
}


Noch einfacher ginge es mit einer reinen Membervariablen

C#-Quelltext
1:
2:
3:
4:
class UIValues
{
  public int DBNr;
}


C#-Quelltext
1:
2:
3:
4:
if (!int.TryParse(txtDBNr.Text, out uivalues.DBNr))
{
   // Fehlermeldung ausgeben
}
(out-Parameter müssen Variablen sein, keine Eigenschaften)


UserTom - So 22.05.22 09:25

Hallo Th69,

Vielen Dank für deine Antwort.

Zitat:
Bei deinen String-Arrays fehlt noch die Erzeugung des Arrays (mittels new[...]).

So ich habe jetzt eine neue Variante mit new[...]. Ich werde die Dateien wieder mit anhängen.

Zitat:
Für einfache Datenhaltungsklassen, wie eben UIValues, solltest du die automatischen Eigenschaften ({ get; set; }) benutzen

Ok habe ich angepasst. Vielen Dank für deine Hilfe.

Zitat:
Und dann in der Zuweisungsmethode

Wenn ich das besser mit TryParse machen sollte dann muss ich alle einzelne Zuweisungen die ich in Connect_Click habe in eine If Schleife packen???


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
private void Connect_Click(object sender, EventArgs e)
{
    uivalues.Error = txtError.Text;

    uivalues.IPNr = txtIPNr.Text;
    uivalues.RackNr = int.Parse(txtRackNr.Text);
    uivalues.SlotNr = int.Parse(txtSlotNr.Text);

    uivalues.DBNr = int.Parse(txtDBNr.Text);
    uivalues.DBOffset = int.Parse(txtDBOffsetNr.Text);
    uivalues.DBSize = int.Parse(txtDBSizeNr.Text);

    uivalues.PraegeCodeDBPos = int.Parse(txtPraegeCodeDBPos.Text);
    uivalues.FolderNameDBPos = int.Parse(txtFolderNameDBPos.Text);
    uivalues.PraegeCode2DBPos = int.Parse(txtPraegeCode2DBPos.Text);

    uivalues.TrigSaveFilesByte = int.Parse(txtTrigSaveFilesByte.Text);
    uivalues.TrigSaveFilesBit = int.Parse(txtTrigSaveFilesBit.Text);
    uivalues.TypLinksByte = int.Parse(txtTypLinksByte.Text);
    uivalues.TypLinksBit = int.Parse(txtTypLinksBit.Text);
    uivalues.TyprechtsByte = int.Parse(txtTypRechtsByte.Text);
    uivalues.TyprechtsBit = int.Parse(txtTypRechtsBit.Text);
    uivalues.RoboterDSLeerByte = int.Parse(txtRoboterDSLeerByte.Text);
    uivalues.RoboterDSLeerBit = int.Parse(txtRoboterDSLeerBit.Text);
    uivalues.FMBauteilByte = int.Parse(txtFMBauteilByte.Text);
    uivalues.FMBauteilBit = int.Parse(txtFMBauteilBit.Text);
    uivalues.HeartByte = int.Parse(txtHeartByte.Text);
    uivalues.HeartBit = int.Parse(txtHeartBit.Text);

    for (int i = 0; i < uivalues.SourceFilePathLocalApps.Length; i++)
        uivalues.SourceFilePathLocalApps[i] = tbSourceFilePathLocalApps[i].Text;

Das wird aber den Code in dieser Methode ganz schön vergrößern.
Wäre es nicht besser ich lagere das in eine Methode PassAllUIVariables() aus und rufe die dann in Connect_Click auf? Oder wie sollte ich es deiner Meinung nach machen?

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
if (int.TryParse(txtDBNr.Text, out int dbnr))
{
  uivalues.DBNr = dbnr;
}
else
{
  // Fehlermeldung ausgeben
}

Ich habe den Code getestet und er macht auch was er soll aber mit Nebeneffekten.

Meine Vermutung:
Am Ende von Connect_Click schalte ich den Timer EventLoop ein.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
else
{
    MessageBox.Show("Es müssen alle Felder ausgefüllt werden");
}

EnableTextBoxes(false);

ScanDBOffset.Enabled = true;
EventLoop.Enabled = true;


Im EventLoop_Tick schalte ich den Timer aus. Wenn in die Schleifen reingesprungen wird ist es gut und der Timer bleibt aus.
Wenn nicht wird über den else Zweig der Timer wieder eingeschaltet.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
        private void EventLoop_Tick(object sender, EventArgs e)
        {
            EventLoop.Enabled = false;

            if (BW_Event.IsBusy != true)
            {

                S7VariablenResult s7VariablenResult = LeakResult();

                if (s7VariablenResult.TrigSaveFiles && (s7VariablenResult.TypLinks || s7VariablenResult.TypRechts) && !s7VariablenResult.FMBauteil)
                {
                    BW_Event.RunWorkerAsync();                    
                }
                else
                {
                    if (EventLoop.Enabled == true)
                    {
                        return;
                    }
                    else
                    {
                        EventLoop.Enabled = true;
                    }

                }
            }
        }


Sollte der Timer aus bleiben, weil ein Kopiervorgang gestartet ist, bin ich nicht sicher wo ich den Timer nach dem kopieren wieder einschalten muss.
Der Timer wird in der BW_Event_ProgressChanged wieder eingeschalte. Ist das gut?

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
private void BW_Event_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
    S7VariablenResult s7VariablenResult = LeakResult();

    lblPraegeCode.Text = s7VariablenResult.PraegeCodeNr; // It’s already a string
    lblFolderName.Text = s7VariablenResult.FolderName;
    lblPraegeCode2.Text = s7VariablenResult.PraegeCode2Nr;

    EventLoop.Enabled = true;
}


Noch eine andere Vermutung ist das die beiden Timer daran Schuld sind.
Beide Timer rufen die S7VariablenResult s7VariablenResult = LeakResult() auf wäre das möglich?


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
private void ScanDBOffset_Tick(object sender, EventArgs e)
{
    ScanDBOffset.Enabled = false;
######################################################################
    S7VariablenResult s7VariablenResult = LeakResult();
######################################################################
}



C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
private void EventLoop_Tick(object sender, EventArgs e)
{
    EventLoop.Enabled = false;

    if (BW_Event.IsBusy != true)
    {
#######################################################################
        S7VariablenResult s7VariablenResult = LeakResult();
#######################################################################
    }
}


So und dann zuletzt haben wir noch das sporadische anzeigen von einer Fehlermeldung.
Ich denke das auch der Timer daran Schuld ist.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
for (int i = 0; i < cbActivateLocalApps.Length; i++)
{
    if (cbActivateLocalApps[i].Checked == true)
    {
        string praegeCodeNrFolderName = s7VariablenResult.FolderName;
        string praegeCodeNrFileName = s7VariablenResult.PraegeCodeNr;
        //string praegeCode2NrFileName = s7VariablenResult.PraegeCode2Nr;
        string sourceFilePath = uivalues.SourceFilePathLocalApps[i];
        string targetPath = DEFAULTTARGETPATH;

        MoveFilesfromLocal(praegeCodeNrFolderName, praegeCodeNrFileName, sourceFilePath, targetPath);
    }
}


Ich habe mir die Mühe gemacht und habe jede For-Schleife einzeln getestet um den Fehler einzugrenzen.
Da bekomme ich keine Fehlermeldung erst ab 2 For-Schleifen beginnt es und dann auch nicht immer.

Die Fehlermeldung lautet: Eine Datei kann nicht erstellt werden, wenn sie bereits vorhanden ist.
Ich kann das halt nicht nachvollziehen, weil ja alle Dateien kopiert und verschoben werden.

Könntest du dir das bitte mal anschauen? Vielen Dank nochmal für die bisherige Hilfe.

Grüße Tom

Moderiert von user profile iconTh69: C#-Tags hinzugefügt


UserTom - So 22.05.22 09:30

Hallo,

Ich habe die Dateien vergessen.

Grüße Tom


UserTom - So 22.05.22 11:59

Hallo,

Kleines Update meinerseits. Ich habe in Google mal nach Timer mit BackgroundWorker gesucht.
Und bin darauf gestoßen das ich den RunWorkerCompleted nutzen sollte um den Timer zum Schluß wieder zu reaktivieren.

C#-Quelltext
1:
2:
3:
4:
5:
6:
private void BW_Event_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
    Ready();

    EventLoop.Enabled = true;
}


Die Ready Methode im ProgressChanged war glaube ich auch falsch und habe sie auch verschoben.
Es sah ganz gut aus und ohne Fehler aber es war auch erst ein Versuch.

Was denkt Ihr ist das gut so?

Vielen Danke

Grüße Tom

Moderiert von user profile iconTh69: C#-Tags hinzugefügt


Ralf Jansen - So 22.05.22 12:52

RunWorkerCompleted ist hilfreich. Insbesondere wenn man an Fehlerhandling denkt. In den EventArgs steckt z.b. die Exception drin wenn DoWork durch eine abgebrochen wurde.
Im Moment hast du ja für den Fehlerfall eine MessageBox und die würde alles dauerhaft stoppen bis die einer wegklickt. Üblicherweise nicht das was man bei einem Background Thread will.

Ansonsten solltest du so jetzt den Tick Event vereinfachen können.
Etwa


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
private void EventLoop_Tick(object sender, EventArgs e)
{
    S7VariablenResult s7VariablenResult = LeakResult();

    if (s7VariablenResult.TrigSaveFiles && (s7VariablenResult.TypLinks || s7VariablenResult.TypRechts) && !s7VariablenResult.FMBauteil)
    {
        EventLoop.Enabled = false;
        BW_Event.RunWorkerAsync();
    }
}


Th69 - So 22.05.22 13:25

UserTom hat folgendes geschrieben:
Wäre es nicht besser ich lagere das in eine Methode PassAllUIVariables() aus und rufe die dann in Connect_Click auf? Oder wie sollte ich es deiner Meinung nach machen?

Ja, das wäre wohl gut.
Und laß ersteinmal die Parse-Aufrufe drin (damit du nicht noch mehr Baustellen aufmachst). Dann baue aber wenigstens einen try...catch-Block um den gesamten Methoden-Aufruf und gebe dem Anwender eine Fehlermeldung (à la "Bitte nur Zahlenwerte eingeben") aus.

Und bgzl. der Timer: du solltest auf jeden Fall sicherstellen, daß jeder Timer nur seine eigenen Aufgaben durchführt und nicht beide dasselbe ausführen.

Auch das mit dem RunWorkerCompleted sieht jetzt sinnvoller aus.

Ansonsten hilft hier nur, viel Testen und debuggen bzw. loggen.


UserTom - So 22.05.22 23:34

Servus,

Vielen Dank für Eure Antworten.

@ Ralf

Zitat:
Insbesondere wenn man an Fehlerhandling denkt.


Ist die Fehlerbehandlung so richtig?

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
private void BW_Event_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
        {
            if (e.Cancelled == true)
            {
                lblEventResult.Text = "Canceled!";
            }
            else if (e.Error != null)
            {
                lblEventResult.Text = "Error: " + e.Error.Message;
            }
            else
            {
                lblEventResult.Text = "Done";
            }

            Ready();

            EventLoop.Enabled = true;
        }


Zitat:
Im Moment hast du ja für den Fehlerfall eine MessageBox

Die Messagebox habe ich gegen eine Klassenmethode SaveErrorMessage ausgetauscht.
Damit werden Logfiles erstellt. Habe ich aus dem I-Net.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
using System;
using System.IO;
using System.Windows.Forms;

namespace PDE
{
    internal static class ErrorExtensions
    {
        //[DebuggerStepThrough()]
        public static void SaveErrorMessage(this Exception ex)
        {
            var dir = Path.Combine(Application.StartupPath, "ErrorLogging");
            var di = new DirectoryInfo(dir);
            if (!di.Exists)
                di.Create();
            string dateTime = DateTime.Now.ToString("dd.MM.yy");
            FileInfo fi = new FileInfo(Path.Combine(dir, string.Concat("Error-file-from_", dateTime, ".log"))); ;
            //FileInfo fi = new FileInfo(Path.Combine(dir, string.Concat("Error-", DateTime.Now.ToFileTime().ToString(), ".log")));
            using (StreamWriter swLogFile = new StreamWriter(fi.FullName))
            {
                swLogFile.Write(ex.ToString());
            }
        }
    }
}


Zitat:
Ansonsten solltest du so jetzt den Tick Event vereinfachen können.

Du hast IsBusy weggelasen war das absicht? Soll ich die if Abfrage drin lassen?

@ Th69

Zitat:
Dann baue aber wenigstens einen try...catch-Block


Da ich die NumericUpDown Elemente optisch nicht mag habe ich mir überlegt wie ich verhindern kann das man nur Zahlen eingeben kann.
Mit dem KeyPressEvent bin ich mir sicher das man nichts falsches eingeben kann.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
private void OnlyNumber_KeyPress(object sender, KeyPressEventArgs e)
{
    //Prüfung, ob eine Zahl oder BackSpace ausgelöst wurde
    if ("1234567890\b".IndexOf(e.KeyChar.ToString()) < 0)
    {
        //Bei Abweichung Ereignis verwerfen
        e.Handled = true;
    }
}


Zitat:
Und bgzl. der Timer:


Beide Timer rufen die folgende Methode auf

C#-Quelltext
1:
S7VariablenResult s7VariablenResult = LeakResult();                    


ScanDBOffset_Tick ist für die UI-Elemente der SPS Variablen zuständig.
EventLoop_Tick ist für die IO Operationen zuständig.
Beide müssen Variablen aus dieser Methode abrufen. Ich denke bei 100ms wird das sehr schnell passieren
das die Timer gleichzeitig auf diese Methode zugreifen.

Ich sollte eine 2. s7VariablenResult (s7UIVariablenResult) erstellen nur für die UI-Elemente. Würde das was bringen?
Oder stören die sich gar nicht? Wie ist deine Meinung dazu.

Vielen Dank nochmal für Eure Antworten.

Grüße Tom


Th69 - Mo 23.05.22 08:42

UserTom hat folgendes geschrieben:
Mit dem KeyPressEvent bin ich mir sicher das man nichts falsches eingeben kann.

Doch, nämlich per Copy&Paste (Strg+V bzw. über das Kontextmenü) (wie auch bei meiner NumericTextBox)!

Und bzgl. s7VariablenResult:

Soll bzw. darf der Anwender denn die Werte in den TextBoxen ändern, während die beiden Timer laufen? Oder wäre es nicht besser, sie würden nur beim Start der Timer einmalig ausgelesen?

Denn bisher liest du ja jetzt die TextBox-Werte einmalig beim Connect_Click (bzw. in der ausgelagerten Methode) und in LeakResult (dort solltest du nur das Auslesen der S7-Werte durchführen - die TextBox-Werte stehen ja jetzt in UIValues).

Ist denn S7VariablenResult eine von dir erstellte Struktur? Dann könntest du diese auf die Rückgabewerte beschränken (da die Eingabewerte ja in UIValues jetzt stehen).
Die UIValues-Klasse hatte ich eigentlich nur für die Folder-Namen und CheckBoxen (für die Kopierroutinen gedacht), nicht für die S7-Werte. Vllt. solltest du dann zwei getrennte S7-Klassen (für Eingabe und Ausgabe) erzeugen?
Und die S7-Eingabewerte (IPNr, ... , HeartBit) nur direkt vor dem Starten der Timer aus den TextBoxen auslesen?

Wie du siehst, ist Datenmodellierung hier ersteinmal wichtig, um danach dann an den richtigen Stellen die Daten zu lesen und/oder zu schreiben.

PS: In Zeile 90 ff. kannst du auch einfach jetzt die neue Abfrage-Methode aufrufen: if (!AreAllTextBoxesFilled())


Ralf Jansen - Mo 23.05.22 12:14

Zitat:
Ist die Fehlerbehandlung so richtig?

Besser
Zitat:
Die Messagebox habe ich gegen eine Klassenmethode SaveErrorMessage ausgetauscht.
Damit werden Logfiles erstellt. Habe ich aus dem I-Net.

Bisher war im catch Handler nur die MessageBox und sonst nix. Heißt der Code danach würde ausgeführt werden was vermutlich immer falsch wenn etwas DoWork knallt. Vermutlich gehört da noch ein throw; rein.
Zitat:
Du hast IsBusy weggelasen war das absicht? Soll ich die if Abfrage drin lassen?

Absicht. Du hast dir einen so genannten One-Shot Timer gebaut. Da du den Timer abschaltest während der Backgroundworker läuft und erst danach wieder anschaltest kann Timer tick nie feuern während der Background Worker läuft. Der Test ist also überflüssig(auch wenn er nicht schaden würde).


UserTom - Mo 23.05.22 23:43

Servus,

@ Th69

Zitat:
Doch, nämlich per Copy&Paste

Ok muss ich schauen was ich da machen werde. Wenn ich den Ablauf mit dem kopieren sicher habe kann ich mich um schöner Wohnen kümmern.

Zitat:
Soll bzw. darf der Anwender denn die Werte in den TextBoxen ändern.

Es wird einmalig von mir eingestellt und nachdem man auf Connect geklickt hat sind alle Eingabefelder gesperrt.
Wenn die Timer laufen kann man nichts verändern. Es sei den jemand logt sich über die Login Form ein und entsperrt die Felder.

Zitat:
Oder wäre es nicht besser, sie würden nur beim Start der Timer einmalig ausgelesen?

Darüber habe ich auch nachgedacht. Vielen Dank für deinen Hinweis. Ich habe jetzt zu Testzwecke einen Timer deaktiviert.
Also mit dem Connect_Click lese ich die Textboxen ein. Das passt für mich da ja einmal lesen reicht. Wenn es mal fertig
wird soll dieses kleine Tool ja vom Prinzip her immer laufen solange wie diese Anlage produziert.

Zitat:
und in LeakResult (dort solltest du nur das Auslesen der S7-Werte durchführen


Meinst du so???

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
private S7VariablenResult LeakResult()
{
    S7VariablenResult s7VariablenResult = new S7VariablenResult();
    // Reads the buffer.            
    S7VariablenResult s7VariablenResult = new S7VariablenResult();
    // Reads the buffer.

    byte[] dbbuffersize = new byte[uivalues.DBSize];

    s7Client.DBRead(uivalues.DBNr, uivalues.DBOffset, uivalues.DBSize, dbbuffersize);

    s7VariablenResult.PraegeCodeNr = S7.GetStringAt(dbbuffersize, uivalues.PraegeCodeDBPos);
    s7VariablenResult.FolderName = S7.GetStringAt(dbbuffersize, uivalues.FolderNameDBPos);
    s7VariablenResult.PraegeCode2Nr = S7.GetStringAt(dbbuffersize, uivalues.PraegeCode2DBPos);
    s7VariablenResult.TrigSaveFiles = S7.GetBitAt(dbbuffersize, uivalues.TrigSaveFilesByte, uivalues.TrigSaveFilesBit);
    s7VariablenResult.TypLinks = S7.GetBitAt(dbbuffersize, uivalues.TypLinksByte, uivalues.TypLinksBit);
    s7VariablenResult.TypRechts = S7.GetBitAt(dbbuffersize, uivalues.TyprechtsByte, uivalues.TyprechtsBit);
    s7VariablenResult.RoboterDSLeer = S7.GetBitAt(dbbuffersize, uivalues.RoboterDSLeerByte, uivalues.RoboterDSLeerBit);
    s7VariablenResult.FMBauteil = S7.GetBitAt(dbbuffersize, uivalues.FMBauteilByte, uivalues.FMBauteilBit);
    s7VariablenResult.HeartBeat = S7.GetBitAt(dbbuffersize, uivalues.HeartByte, uivalues.HeartBit);

    return s7VariablenResult;


Das habe ich probiert. Da bekomme ich dann Probleme mit dem kopieren. Es werden keine Datei kopiert in CopyFilesfromNetwork.
Das verschieben mit Move funktioniert. Wenn ich wieder auf die orginale s7VariablenResult zurück gehe dann wird wieder alles kopiert.

Zitat:
Ist denn S7VariablenResult eine von dir erstellte Struktur?

Nein von einer Vorlage.

Zitat:
Dann könntest du diese auf die Rückgabewerte beschränken

Das verstehe ich nicht. Kannst du das besser erklären.

Zitat:
PS: In Zeile 90 ff. kannst du auch einfach jetzt die neue Abfrage-Methode aufrufen: if (!AreAllTextBoxesFilled())

Ich habe diese Abfrage gelöscht, weil sie meiner Meinung nach überflüssig ist.

@ Ralf

Vermutlich gehört da noch ein throw; rein. Diese throw kannst du mir den genauer erklären?

Vielen Dank nochmal.

Grüße Tom

Moderiert von user profile iconTh69: C#-Tags hinzugefügt


Th69 - Di 24.05.22 09:07

UserTom hat folgendes geschrieben:
Es wird einmalig von mir eingestellt und nachdem man auf Connect geklickt hat sind alle Eingabefelder gesperrt.

Dann ist es ja jetzt gut so, daß nicht jedesmal die (gleichen) Werte wieder und wieder per Parse ausgelesen werden.

UserTom hat folgendes geschrieben:
Meinst du so???

Ja, bzw. fast so (bis auf doppelte Zeilen 5+6).

UserTom hat folgendes geschrieben:
Das habe ich probiert. Da bekomme ich dann Probleme mit dem kopieren. Es werden keine Datei kopiert in CopyFilesfromNetwork.

Das muß dann ein anderer Fehler sein (evtl. hast du vergessen eine bestimmte Variable zu setzen, die du bisher in LeakResult ausgelesen hast). Am besten mal mit dem Debugger (Stichwort: Haltepunkt/breakpoint setzen) dann da durchgehen (s. z.B. [Artikel] Debugger: Wie verwende ich den von Visual Studio? [https://mycsharp.de/forum/threads/109261/artikel-debugger-wie-verwende-ich-den-von-visual-studio]).

UserTom hat folgendes geschrieben:
Nein von einer Vorlage.

Wenn du diese nicht ändern kannst/darfst, dann mußt du eben klar im Code unterscheiden, welche Daten davon Eingabe und welche Ausgabe sind (oder du verzichtest auf diese Klasse und erstellst dir selber 2 getrennte Klassen - ist aber jetzt ersteinmal nicht zwingend).

UserTom hat folgendes geschrieben:
Das verstehe ich nicht. Kannst du das besser erklären.

Die gerade beschriebene Unterteilung bzgl. Eingabe und Ausgabe, so daß LeakResult (bzw. besser wohl ReadResults benannt) nur die Eigenschaften der Ausgabe-Klasse setzt (anstatt nur bestimmte Eigenschaften der Riesenklasse S7VariablenResult).

UserTom hat folgendes geschrieben:
Diese throw kannst du mir den genauer erklären?

throw wirft die gerade ausgelöste Exception im catch-Handler weiter, so daß dieser Fehler-Status dann über RunWorkerCompletedEventArgs in RunWorkerCompleted abgefragt werden kann (dies würde aber DoWork, d.h. den Kopiervorgang abbrechen), s.a. throw (C#-Referenz) [https://docs.microsoft.com/de-de/dotnet/csharp/language-reference/keywords/throw].


Ralf Jansen - Di 24.05.22 11:55

Zitat:
Diese throw kannst du mir den genauer erklären?


Hat user profile iconTh69 schon. Dein erster try..catch z.b. ist wenn das anlegen des ZielOrdners schief geht. Im Moment scheint es so zu sein das du den Fehler anzeigst/logst aber dann weiter machst mit dem kopieren der Files. Dein Zielordner konnte aber nicht angelegt werden. Da macht weitermachen keinen Sinn und du solltest da irgendwie abbrechen. Eine Lösung ist da halt dann ein throw; am Ende des Catch Blocks einzufügen damit die Exception weitergeworfen wird um irgendwo anders dann behandelt zu werden(z.b. in WorkerCompleted wo dann im Zweifel nicht mehr viel passieren muß)


UserTom - So 29.05.22 09:04

Servus,

Vielen Dank für Eure Antworten.

Zitat:
Das muß dann ein anderer Fehler sein.

Es lag an einer Variable. Jetzt funktioniert es.

Zitat:
Wenn du diese nicht ändern kannst/darfst,

Doch, die kann man ändern. Aber das brauche ich nicht mehr, da ich mich dazu entschlossen habe nur noch mit einem Timer zu arbeiten. Ich habe es auch in ReadResult umgetauft.

An Euch beide vielen Dank für die Erklärung über throw.
Zitat:
Dein erster try..catch z.b. ist wenn das anlegen des ZielOrdners schief geht.

Zitat:
(dies würde aber DoWork, d.h. den Kopiervorgang abbrechen)

Eine Frage hier zu. Könnte man über CancellationPending verhindern das wenn der Ordner nicht erstellt wird das man bevor das kopieren beginnt rausspringt? So als hätte man einen Cancel Button.
Wenn ja wie müsste ich das umbauen.
Es ist eines klar, wenn das Ordner erstellen nicht funktionieren sollte bin ich mir fast sicher, das dass Problem von der SPS kommt. Der Pfad "D:\S7_Export\AppData" ist ein Freigegebener Ordner der ohne Adminrechte nicht gelöscht werden kann. Ich hätte an der Stelle, wenn es funktionieren würde, die Möglichkeit an die SPS ein Signal zu schicken damit die Anlage stoppt. Fehler analysieren, Problem beheben usw...


An dieser Stelle würde ich gerne Euch nochmal auf 2 Sachen ansprechen die ihr erwähnt hattet.

@TH69
Zitat:
Eigentlich würde ich dafür sogar empfehlen, den gesamten BackgroundWorker-Code in eine eigene Klasse auszulagern

Würdest du mir das erklären wie das in meinem Fall aussehen würde? Vorausgesetzt mein Aufbau mit dem BackgroundWorker ist in Ordnung.

@ Ralf
Zitat:
Du hast dir einen so genannten One-Shot Timer gebaut.

Ist das falsch oder unnötig? Ist es besser wenn Timer tick feuert? Mir gefällt es gar nicht.
Ich dachte immer ich muss ihn ausschalten während das kopieren läuft, weil ich das aus einem Beispielprojekt von einer anderen Internetseite so übernommen habe.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
 if (s7VariablenResult.HeartBeat)
 {
     lblHeartbeat.BackColor = Color.Green;
 }
 else
 {
     lblHeartbeat.BackColor = Color.LightGray;
 }


Vor allem, weil mein Heartbeat deswegen nicht richtig funktioniert. Es sieht blöd aus wenn es aufhört zu blinken und nach dem kopieren blinkt es weiter. Wie wäre es besser umzusetzen?

Vielen Dank nochmal für Eure tolle Unterstützung.

Grüße Tom

Moderiert von user profile iconTh69: Beitragsformatierung überarbeitet.


Th69 - So 29.05.22 10:25

Guten Morgen,

ich hatte ja noch etwas mehr geschrieben:
Th69 hat folgendes geschrieben:
Eigentlich würde ich dafür sogar empfehlen, den gesamten BackgroundWorker-Code in eine eigene Klasse auszulagern, weil dann würde man zuerst Fehlermeldungen bei der Verwendung der Form-Controls erhalten (welche man dann sukzessive ersetzen würde), aber das mute ich dir jetzt (noch) nicht zu...

Da ging es mir in erster Linie darum, alle Zugriffe auf die Controls aus dem BackgroundWorker-Code zu eliminieren (und bei Auslagerung wären die Controls in der neuen Klasse ja nicht bekannt und würden ersteinmal Compilerfehler verursachen).

Es stimmt aber, daß man sich über die Auslagerung von Code beim Entwickeln Gedanken machen sollte ("Trennung von Zuständigkeiten").
Bis jetzt hat deine Klasse ja 3 Hauptaufgaben:

Nach der bisherigen Umstellung deines Codes solltest du wenigstens die reinen IO-Operationen (MoveFilesfromLocal, ..., CopyFilesfromNetwork) auslagern (da du dort bisher auch noch Zugriffe auf UI-Elemente machst, müßtest du nur den reinen IO-Code der Methoden auslagern):

C#-Quelltext
1:
2:
3:
4:
5:
... = Directory.EnumerateDirectories(...);
foreach (string dir in dirs)
{
  //...
}

Und die jeweilige äußere Schleife in den Form-Methoden lassen:

C#-Quelltext
1:
2:
3:
4:
5:
for (...)
  if (cbNetSubFolderUses[j].Checked)
  {
     IOOperations.MoveFilesfromLocal(...);
  }

M.E. kannst du die Klasse IOOperations (oder wie auch immer du sie nennst) ersteinmal static machen.

Du kannst sogar das Exception-Handling so umschreiben, daß der Code nur genau an einer Stelle steht (anstatt immer wieder in jeder Methode), indem du ein Delegate (wie z.B. Action) benutzt:

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
void TryAction(Action action)
{
  try
  {
    action();  
  }
  catch (DirectoryNotFoundException ex)
  {
    ex.SaveErrorMessage();
  }
  catch (UnauthorizedAccessException ex)
  {
    ex.SaveErrorMessage();
  }
}

(zu nahezu jeder Codeverdopplung gibt es [elegante] Wege zur Vermeidung davon.)

Und der Aufruf sieht dann z.B. einfach so aus:

C#-Quelltext
1:
TryAction(() => File.Move(filename, $"{Path.Combine(targetPath, praegeCodeNrFolderName, Path.GetFileName(file)}"));                    

(hier jetzt [kurz] als Lambda-Ausdruck, aber es gibt auch noch andere Möglichkeiten).

Wenn du bisher noch nicht mit (eigenen) Delegates gearbeitet hast: Delegaten (C#-Programmierhandbuch) [https://docs.microsoft.com/de-de/dotnet/csharp/programming-guide/delegates/] - oder du suchst dir weiteres Material dazu im Internet.

PS: Auch Code wie z.B.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
if (s7VariablenResult.TrigSaveFiles)
{
  lblSaveFiles.BackColor = Color.Green;
}
else
{
  lblSaveFiles.BackColor = Color.LightGray;
}
läßt sich einfach mit einer Hilfsmethode kürzer schreiben (Aufgabe für dich ;-)).

Und gerade auch noch in deinem Code entdeckt:

C#-Quelltext
1:
2:
3:
4:
if (!chkActivateLocalApp1.Checked
    && !chkActivateLocalApp2.Checked
    && //...
    && !chkActivateLocalApp8.Checked

(auch das sollte eine Schleife [als eigene Methode] sein, bzw. kurz mit LINQ.)


UserTom - So 29.05.22 19:46

Guten Abend,

Danke für deine Antwort und die Menge an Hilfe.

Zitat:
Nach der bisherigen Umstellung deines Codes solltest du wenigstens die reinen IO-Operationen (MoveFilesfromLocal, ..., CopyFilesfromNetwork) auslagern

Da hätte ich gedacht das ich das nicht umsetzen kann.


C#-Quelltext
1:
2:
3:
4:
5:
private IOFileOperations ioFileOperations = new IOFileOperations();

public MainForm()
{
    InitializeComponent();


Die Klasse. Ich habe es aber nicht statisch hinbekommen.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48:
49:
50:
51:
52:
53:
54:
55:
56:
57:
58:
59:
60:
61:
62:
63:
64:
65:
66:
67:
68:
69:
70:
71:
72:
73:
74:
75:
76:
77:
78:
79:
80:
81:
82:
83:
84:
85:
86:
87:
88:
89:
namespace PDE
{
    class IOFileOperations
    {
        void TryAction(Action action)
        {
            try
            {
                action();
            }
            catch (DirectoryNotFoundException ex)
            {
                ex.SaveErrorMessage();
            }
            //catch (UnauthorizedAccessException ex)
            //{
            //    ex.SaveErrorMessage();
            //}
        }

        public void MoveDirectoryfromLocal(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            var iOFileOperations = new IOFileOperations();

            IEnumerable<string> dirs = Directory.EnumerateDirectories(sourceFilePath, "*", System.IO.SearchOption.AllDirectories).Where(x => x.Contains(praegeCodeNrFileName));

            foreach (string dir in dirs)
            {
                iOFileOperations.TryAction(() => FileSystem.MoveDirectory(dir, dir.Replace(sourceFilePath, Path.Combine(targetPath, praegeCodeNrFolderName)), true));
            }
        }

        public void MoveFilesfromLocal(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            var iOFileOperations = new IOFileOperations();

            IEnumerable<string> files = Directory.EnumerateFiles(sourceFilePath, "*", System.IO.SearchOption.TopDirectoryOnly).Where(x => x.Contains(praegeCodeNrFileName));
            foreach (string file in files)
            {
                FileInfo fi = new FileInfo(file);
                string filename = fi.FullName;

                iOFileOperations.TryAction(() => File.Move(filename, $"{Path.Combine(targetPath, praegeCodeNrFolderName + "\\")}{Path.GetFileName(file) }"));
            }
        }

        public void MoveFilesCode2fromLocal(string praegeCodeNrFolderName, string praegeCode2NrFileName, string sourceFilePath, string targetPath)
        {
            var iOFileOperations = new IOFileOperations();

            IEnumerable<string> files = Directory.EnumerateFiles(sourceFilePath, "*", System.IO.SearchOption.TopDirectoryOnly).Where(x => x.Contains(praegeCode2NrFileName));
            foreach (string file in files)
            {
                FileInfo fi = new FileInfo(file);
                string filename = fi.FullName;

                iOFileOperations.TryAction(() => File.Move(filename, $"{Path.Combine(targetPath, praegeCodeNrFolderName + "\\")}{Path.GetFileName(file) }"));
            }
        }

        public void CopyDirectoryfromNetwork(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            var iOFileOperations = new IOFileOperations();

            IEnumerable<string> dirs = Directory.EnumerateDirectories(sourceFilePath, "*", System.IO.SearchOption.AllDirectories).Where(x => x.Contains(praegeCodeNrFileName));

            foreach (string dir in dirs)
            {
                iOFileOperations.TryAction(() => FileSystem.CopyDirectory(dir, dir.Replace(sourceFilePath, Path.Combine(targetPath, praegeCodeNrFolderName)), true));
                iOFileOperations.TryAction(() => FileSystem.DeleteDirectory(dir, DeleteDirectoryOption.DeleteAllContents));
            }
        }

        public void CopyFilesfromNetwork(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            var iOFileOperations = new IOFileOperations();

            IEnumerable<string> files = Directory.EnumerateFiles(sourceFilePath, "*", System.IO.SearchOption.TopDirectoryOnly).Where(x => x.Contains(praegeCodeNrFileName));
            foreach (string file in files)
            {
                FileInfo fi = new FileInfo(file);
                string filename = fi.FullName;

                iOFileOperations.TryAction(() => File.Copy(file, $"{Path.Combine(targetPath, praegeCodeNrFolderName + "\\")}{Path.GetFileName(file) }"));
                iOFileOperations.TryAction(() => File.Delete(file));
            }
        }
    }
}


Die Methoden

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
        private void MoveFilesfromLocal(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            for (int i = 0; i < cbLocalSubFolderUses.Length; i++)
            {
                if (cbLocalSubFolderUses[i].Checked)
                {
                    ioFileOperations.MoveDirectoryfromLocal(praegeCodeNrFolderName, praegeCodeNrFileName, sourceFilePath, targetPath);
                }
                else
                {
                    ioFileOperations.MoveFilesfromLocal(praegeCodeNrFolderName, praegeCodeNrFileName, sourceFilePath, targetPath);
                }
            }
        }

        private void MoveFilesCode2fromLocal(string praegeCodeNrFolderName, string praegeCode2NrFileName, string sourceFilePath, string targetPath)
        {
            for (int i = 0; i < cbActivateCode2LocalApps.Length; i++)
            {
                if (cbActivateCode2LocalApps[i].Checked)
                {
                    ioFileOperations.MoveFilesCode2fromLocal(praegeCodeNrFolderName, praegeCode2NrFileName, sourceFilePath, targetPath);
                }
            }
        }

        private void CopyFilesfromNetwork(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            for (int j = 0; j < cbNetSubFolderUses.Length; j++)
            {
                if (cbNetSubFolderUses[j].Checked)
                {
                    ioFileOperations.CopyDirectoryfromNetwork(praegeCodeNrFolderName, praegeCodeNrFileName, sourceFilePath, targetPath);
                }
                else
                {
                    ioFileOperations.CopyFilesfromNetwork(praegeCodeNrFolderName, praegeCodeNrFileName, sourceFilePath, targetPath);
                }
            }
        }

Und das kopieren hat sofort funktioniert.

Zitat:
läßt sich einfach mit einer Hilfsmethode kürzer schreiben (Aufgabe für dich ;-)).

Das habe ich bestimmt nicht so umgesetzt wie du das dir gedacht hast.
Ich habe die Methode aber der Code ist nicht kürzer.
Jede Variable hat sein eigenes Label deswegen weiß ich nicht wie ich es einkürzen kann.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48:
49:
50:
51:
52:
53:
54:
55:
56:
57:
58:
59:
60:
61:
62:
63:
64:
65:
66:
        private void S7VariablenStatus(S7VariablenResult s7VariablenResult)
        {
            if (s7VariablenResult.TrigSaveFiles)
            {
                lblSaveFiles.BackColor = Color.Green;
            }
            else
            {
                lblSaveFiles.BackColor = Color.LightGray;
            }

            if (s7VariablenResult.TypLinks)
            {
                lblTypLinks.BackColor = Color.Green;
            }
            else
            {
                lblTypLinks.BackColor = Color.LightGray;
            }

            if (s7VariablenResult.TypRechts)
            {
                lblTypRechts.BackColor = Color.Green;
            }
            else
            {
                lblTypRechts.BackColor = Color.LightGray;
            }

            if (s7VariablenResult.RoboterDSLeer)
            {
                lblRobDSLeer.BackColor = Color.Green;
            }
            else
            {
                lblRobDSLeer.BackColor = Color.LightGray;
            }

            if (s7VariablenResult.FMBauteil)
            {
                lblFMBauteil.BackColor = Color.Green;
            }
            else
            {
                lblFMBauteil.BackColor = Color.LightGray;
            }

            if (s7VariablenResult.HeartBeat)
            {
                lblHeartbeat.BackColor = Color.Green;
            }
            else
            {
                lblHeartbeat.BackColor = Color.LightGray;
            }

            if (s7VariablenResult.RoboterDSLeer)
            {
                lblPraegeCode.Text = "";
                lblFolderName.Text = "";
                lblPraegeCode2.Text = "";

                lblCopyAllFilesResult.Text = "";
                progressBar1.Value = 0;
            }
        }



C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
            uivalues.HeartByte = int.Parse(txtHeartByte.Text);
            uivalues.HeartBit = int.Parse(txtHeartBit.Text);            

            if (BW_CopyAllFiles.IsBusy != true)
            {
                S7VariablenResult s7VariablenResult = ReadResult();

#############################################################################
                S7VariablenStatus(s7VariablenResult);
#############################################################################

                if (s7VariablenResult.TrigSaveFiles && (s7VariablenResult.TypLinks || s7VariablenResult.TypRechts) && !s7VariablenResult.FMBauteil)
                {
                    TimerCopyAllFiles.Enabled = false;
                    BW_CopyAllFiles.RunWorkerAsync();
                }
            }


Zitat:
Und gerade auch noch in deinem Code entdeckt:

Ich habe zwar eine Methode erstellt aber ich weiß nicht wie ich sie in die If else Abfrage integrieren soll.
Dadurch kann ich ich die Methode auch nicht testen.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
        private void OneAppAreChecked()
        {
            const int MAXLOCALAPPS = 8;
            for (int n = 0; n < MAXLOCALAPPS; n++)
            {
                cbActivateLocalApps[n].Checked = false;
                
            }
        }

        private void Connect_Click(object sender, EventArgs e)
        {
            // At least one checkbox must be activated.

###########################################################################
            if (  ??????? )
###########################################################################
            {
                MessageBox.Show("No app is activated!");
            }
            else
            {
                uivalues.Error = txtError.Text;


Etwas ärgerlich das ich nicht alles ohne hilfe umsetzen konnte.

Grüße Tom


UserTom - Mo 30.05.22 08:35

Guten Morgen,

Zitat:
Und gerade auch noch in deinem Code entdeckt:

Ich bin ja echt blöd gewesen.

Dafür hattest du mir ja schonmal die Hilfe mit enable gegeben, das Prinzip ist das gleiche.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
private bool OneAppIsChecked()
        {
            if (!chkActivateLocalApp1.Checked
                && !chkActivateLocalApp2.Checked
                && !chkActivateLocalApp3.Checked
                && !chkActivateLocalApp4.Checked
                && !chkActivateLocalApp5.Checked
                && !chkActivateLocalApp6.Checked
                && !chkActivateLocalApp7.Checked
                && !chkActivateLocalApp8.Checked)
            {
                return true;
            }

            return false;
        }

        private void Connect_Click(object sender, EventArgs e)
        {
            // At least one checkbox must be activated.
            if (OneAppIsChecked() == true)
            {
                MessageBox.Show("No app is activated!");
            }
            else


Zitat:
(auch das sollte eine Schleife [als eigene Methode] sein, bzw. kurz mit LINQ.)

Methode habe ich aber ohne Schleife. Das mit LINQ zu lösen kann ich noch nicht. Würdest du mir das bitte zeigen?

So, dann schaue ich mal weiter. Vielen Dank.

Grüße Tom


Th69 - Mo 30.05.22 08:44

Ja, so sieht der Code doch schon viel übersichtlicher (und damit wartbarer) aus.

UserTom hat folgendes geschrieben:
Ich habe es aber nicht statisch hinbekommen.

Dafür dann jeweils noch static vor die Klasse sowie die Methoden schreiben:
static class IOOperations sowie z.B. public static void MoveDirectoryfromLocal(...)
Für TryAction solltest du auch eine Compilerwarnung CA1822: Member als statisch markieren. [https://docs.microsoft.com/de-de/dotnet/fundamentals/code-analysis/quality-rules/ca1822] erhalten (falls die Code Analyse in den Projekteinstellungen nicht explizit deaktiviert ist).

Du hast aber bei deiner nicht-statischen Klasse auch noch einen OOP-Fehler drin. Du brauchst nicht jedesmal innerhalb derselben Klasse ein neues Objekt mit new IOFileOperations() erstellen. Die Methode tryAction ist doch in der selben Klasse (wahrscheinlich hattest du vorher einen anderen Compilerfehler und hast es deswegen so gelöst?).

Kleiner Hinweis noch: Vergleiche mal deine TryAction(...) Aufrufe mit meinem Code im letzten Beitrag (ich hatte noch den Aufruf bzgl. Path.Combine verbessert).

Bei den Farbzuweisungen hatte ich es so ähnlich gedacht:

C#-Quelltext
1:
SetColor(Control control, bool condition) { ... }                    

Und Aufruf dann je Label kurz in einer Zeile, z.B.

C#-Quelltext
1:
SetColor(lblSaveFiles, s7VariablenResult.TrigSaveFiles);                    

Die Gesamt-Auslagerung in die Methode S7VariablenStatus ist aber auch eine gute Idee (ich würde die Methode dann nur z.B. ShowS7VariablenStatus nennen, damit klarer ist, was sie macht).

Und bei OneAppAreChecked() fehlt noch bool als Rückgabetyp, so daß du dort dann return false oder return true benutzen kannst.
Und dann funktioniert auch die if (OneAppAreChecked())-Abfrage.

Dies hilft dir hoffentlich alles beim Verständnis für mehr OOP (ich will dich damit nicht quälen ; -).

Edit: Jetzt haben sich unsere Antworten überschnitten. Du hattest ja schon den Code von OneAppIsChecked() als Schleife hingeschrieben. Jetzt nur noch ein bißchen logisch anpassen (bes. statt Checked zu setzen eine Abfrage daraus machen).
Du solltest aber true und false vertauschen (bezogen auf den Methodennamen).

Edit2: LINQ [https://docs.microsoft.com/de-de/dotnet/csharp/programming-guide/concepts/linq/] ist ein (eigenes) Thema, das du mal in einem kleinen Testprojekt ausprobieren solltest.
Hier nur kurz, wie ich es gedacht habe: if (!cbActivateLocalApps.Any(cb => cb.Checked)) (bzw. innerhalb der Methode OneAppIsChecked).


UserTom - Mo 30.05.22 10:33

Servus,

Vielen Dank für deine Antwort.

Zitat:
Dafür dann jeweils noch static vor die Klasse sowie die Methoden schreiben:

Habe ich geändert. Muss ich noch testen.

Zitat:
Kleiner Hinweis noch: Vergleiche mal deine TryAction(...) Aufrufe mit meinem Code im letzten Beitrag


Habe ich probiert, aber da ist noch ein Klammerfehler drin. Siehe Bild.

Zitat:
Bei den Farbzuweisungen hatte ich es so ähnlich gedacht:

Da muss ich mich erstmal mit SetColor beschäftigen.

Zitat:
Die Gesamt-Auslagerung in die Methode S7VariablenStatus ist aber auch eine gute Idee


Und ich konnte sie so einkürzen.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
 private void ShowS7VariablenStatus(S7VariablenResult s7VariablenResult)
        {
            lblSaveFiles.BackColor = (s7VariablenResult.TrigSaveFiles) ? Color.Green : Color.LightGray;

            lblTypLinks.BackColor = (s7VariablenResult.TypLinks) ? Color.Green : Color.LightGray;

            lblTypRechts.BackColor = (s7VariablenResult.TypRechts) ? Color.Green : Color.LightGray;

            lblRobDSLeer.BackColor = (s7VariablenResult.RoboterDSLeer) ? Color.Green : Color.LightGray;

            lblFMBauteil.BackColor = (s7VariablenResult.FMBauteil) ? Color.Green : Color.LightGray;

            lblHeartbeat.BackColor = (s7VariablenResult.HeartBeat) ? Color.Green : Color.LightGray;

            if (s7VariablenResult.RoboterDSLeer)
            {
                lblPraegeCode.Text = "";
                lblFolderName.Text = "";
                lblPraegeCode2.Text = "";

                lblCopyAllFilesResult.Text = "";
                progressBar1.Value = 0;
            }
        }


Zitat:
Dies hilft dir hoffentlich alles beim Verständnis für mehr OOP (ich will dich damit nicht quälen ; -).

Hoffe ich auch und es ist auch schon etwas besser geworden mit dem Verständnis, aber da fehlt noch sehr viel. Vielen Dank für die ganze Hilfe.
Es gibt so viele mögliche Lösungswege und wenn man nur einen kennt, weil man noch keine Erfahrung hat entsteht Spaghetticode.

So jetzt noch eine Frage über den Code wie er aussieht. Wir haben viele neu Methoden erstellt. Gibt es eine Vorgabe wo an welcher Stelle
die Methoden aufgerufen werden sollten? Gibt es auch einen logischen Hintergrund, wo ich in einer Methode eine andere Methode aufrufen sollte?

Als Beispiel???


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
 void ShowPlcStatus()

  private void TimerCopyAllFiles_Tick(object sender, EventArgs e)
        {
#####################           
 ShowPlcStatus();
#####################  

            uivalues.TrigSaveFilesByte = int.Parse(txtTrigSaveFilesByte.Text);


So vielen Dank nochmal für die tolle Hilfe.

Grüße Tom


Th69 - Mo 30.05.22 11:04

Die Fehlermeldung sagt doch, was fehlt: eine runde Klammer ;-)
Die Codeänderung ist hauptsächlich, damit man nicht selber im Code die Verzeichnistrenner (Slashes bzw. Backslashes) drin hat (hilft auch für eine mögliche Portierung auf andere Systeme) - und wenn dann sollte man Path.PathSeparator [https://docs.microsoft.com/de-de/dotnet/api/system.io.path.pathseparator] verwenden.

UserTom hat folgendes geschrieben:
Da muss ich mich erstmal mit SetColor beschäftigen.

Und ich konnte sie so einkürzen.

Mit dem Tertiäroperator ? : kann man den Code auch gut einkürzen (auch wenn manche Entwickler das unleserlicher als if ... else ... sehen - ich selber benutze es aber auch überwiegend, wenn es nicht zu sehr verschachtelt ist).

Trotzdem wäre es eine gute Übung, wenn du es noch schaffst SetColor zu implementieren (die vermeidet, daß du die Farben jeweils explizit hinschreiben mußt).
Ich selber würde diese Methode sogar noch mit Defaultparametern für die Farben ausstatten, um sie individuell doch noch ändern zu könnnen:

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
SetColor(Control control, bool condition, Color colorTrue = default, Color colorFalse = default)
{
  if (colorTrue == default)
    colorTrue = Color.Green;

  if (colorFalse == default)
    colorFalse = Color.LightGray;

  // todo: ...
}
(Color-Werte können nicht direkt als Defaultparameter benutzt werden, da sie nicht konstant, sondern nur static readonly sind - ähnlich wie bei String.Empty :-( ).

UserTom hat folgendes geschrieben:
So jetzt noch eine Frage über den Code wie er aussieht. Wir haben viele neu Methoden erstellt. Gibt es eine Vorgabe wo an welcher Stelle
die Methoden aufgerufen werden sollten? Gibt es auch einen logischen Hintergrund, wo ich in einer Methode eine andere Methode aufrufen sollte?

So eine ähnliche Frage gab es schon mal: Aufrufen von Methoden [https://entwickler-ecke.de/viewtopic.php?t=118870] (diese bezieht sich auf die vorherige Frage vom selben User: Rückgabewerte - wofür brauche ich sie? [https://entwickler-ecke.de/viewtopic.php?t=118862]).


UserTom - Mo 30.05.22 13:00

Hallo,

Vielen Dank für deine Antwort.

Ja habe ich noch herausgefunden, ich habe die Klammer vorher an der falschen Stelle ausprobiert und war nicht akribisch genug noch mehr auszuprobieren.
Was mich wieder ärgert. Probieren nicht wissen.

Zitat:
Trotzdem wäre es eine gute Übung

So ich habe eine Variante ich kann sie aber leider nicht testen. Was ist deine Meinung?


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
 private void SetColor(Control control, bool condition, Color colorTrue = default, Color colorFalse = default)
        {
            if (colorTrue == default)
                colorTrue = Color.Green;

            if (colorFalse == default)
                colorFalse = Color.LightGray;

            if (condition)
            {
                control.BackColor = colorTrue;
            }
            else
            {
                control.BackColor = colorFalse;
            }
        }

        private void ShowS7VariablenStatus(S7VariablenResult s7VariablenResult)
        {
            if (s7VariablenResult.RoboterDSLeer)
            {
                SetColor(lblSaveFiles, true);
            }
            else
            {
                SetColor(lblSaveFiles, false);
            }

                //lblSaveFiles.BackColor = (s7VariablenResult.TrigSaveFiles) ? Color.Green : Color.LightGray;


Falls es richtig ist, dann ist es gut für die Wartbarkeit. Habe ich verstanden, ich brauche nur an einer Stelle die Farbe ändern
und kann sogar die Methode für andere Situationen hernehmen. Für die Anzahl der Codezeilen wieder ein Rückschritt, was aber für den
Vorteil der Wartbarkeit zu vernachlässigen ist.

Wenn ich es richtig verstanden habe, ist das hier jetzt ein gutes Beispiel für die OOP gewesen?

Zitat:
So eine ähnliche Frage gab es schon mal:

Habe ich mir durchgelesen. Muss ich erstmal darüber nachdenken und vielleicht nochmal lesen.

Vielen Dank für deine tolle Unterstützung.

Grüße Tom


Th69 - Mo 30.05.22 13:47

Fast gut! ;-)

Ich hatte doch schon den (kurzen, einzeiligen) Aufruf angegeben:

C#-Quelltext
1:
SetColor(lblSaveFiles, s7VariablenResult.TrigSaveFiles);                    

So hast du keine mehrzeilige if ... else ...-Anweisung mehr beim Aufruf (TrigSaveFiles ist ja ein bool-Wert, also true oder false).

(Ist RoboterDSLeer jetzt ein Schreibfehler bei dir, statt TrigSaveFiles?)

Wenn du noch eine konkrete Frage bzgl. der Methodenaufrufe hast, dann stelle sie ruhig (ist besser als nur allgemein darüber zu diskutieren).


Ralf Jansen - Mo 30.05.22 14:03

Zitat:
Was ist deine Meinung?

Ok. Andere Variante einfach weil es immer mehrere gibt. Wartbarkeit, Lesbarkeit, Testbarkeit hängen immer zusammen und sind keine absoluten Größen. Es hängt leider immer auch vom Publikum ab.


C#-Quelltext
1:
2:
3:
private void SetColor(Control control, bool condition) => SetColor(control, condition, Color.Green, Color.LightGray);

private void SetColor(Control control, bool condition, Color colorTrue, Color colorFalse) => control.BackColor = condition ? colorTrue : colorFalse;


Zitat:
Was mich wieder ärgert. Probieren nicht wissen.

In der IT heißt es auch "Ich habe nur bewiesen das es geht aber noch nicht getestet." Nenne es nicht probieren sondern testen und schon ist der negative Klang weg.


UserTom - Mo 30.05.22 22:50

Servus,

Vielen Dank für Eure Antworten.

Zitat:
(Ist RoboterDSLeer jetzt ein Schreibfehler bei dir, statt TrigSaveFiles?)

Jupp das war ein Schreibfehler.

Zitat:
Wenn du noch eine konkrete Frage bzgl. der Methodenaufrufe hast, dann stelle sie ruhig

Im Moment gerade nicht aber ich werde mir den Code nochmal anschauen und darüber nachdenken.

Aber ich hätte eine andere Frage. Und zwar gibt es ja die 3 Schichten Architektur.
Und wenn ich das richtig verstanden habe dann müsste ich alles was mit der reinen SPS zu tun hat in eine Klasse z.B. SPS auslagern. Das wollte ich schon immer machen und hatte auch mal angefangen. Das ist aber völlig in die Hose gegangen und ich habe es dann sein gelassen.
Es ist halt SPS Logik deswegen dachte ich das gehört ausgelagert. Ich muss aber auch dazu sagen das das für mich zu schwer ist und ich Eure Hilfe bräuchte. Aber was meint den Ihr dazu? Sinnvoll?
Von einer anderen Seite gesehen wäre es aber auch wieder doppelte Arbeit weil wir zum Beispiel erst vor kurzem die UIValues in der Methode Connect_Click untergebracht haben wenn ich die Methode jetzt in die Klasse SPS verschiebe dann entsteht ein sehr großer Rattenschwanz. Nicht Wahr?
Wenn Ihr der Meinung seit es ist auch für mich zu schaffen dann ok.

@ Ralf

Hattest du meinen Beitrag gelesen wo ich dich wegen dem OneShot-Timer was gefragt hatte?
Ich wollte halt nur wissen ob das so ok ist das ich den Timer stoppe oder gar nicht nötig ist.

So vielen Dank nochmal für die ganze Hilfe und Erklärungen.

Grüße Tom

Moderiert von user profile iconTh69: Beitragsformatierung überarbeitet.


Th69 - Di 31.05.22 09:49

Zuersteinmal zum One-Shot-Timer:
Es kommt darauf an, ob du die im Timer aufgerufenen Methoden während des Kopiervorgangs benötigst oder nicht (z.B. ShowPlcStatus()).
Ob der Kopiervorgang läuft, d.h. der BackgroundWorker, das fragst du ja ab.

Was ich jetzt aber noch im Code gesehen habe: Warum werden im Timer-Event TimerCopyAllFiles_Tick (statt einmalig im Connect) die UI-Werte ausgelesen, d.h. die Parse-Aufrufe, und dann noch in uiValues geschrieben (statt in s7VariablenResult)?
Die uiValues sollen ja nur die UI-Elemente abdecken, welche für den asynchronen Vorgang benötigt werden (und nicht schon in der anderen Klasse S7VariablenResult stehen - so hast du sie ja doppelt zu verwalten!?!).

Und dazu solltest du ein Objekt von S7VariablenResult als Membervariable anlegen und dessen Werte setzen (anstatt immer wieder lokale Objekte in ReadResult zu erzeugen - daher ja auch mein Vorschlag bzgl. zweier S7-Klassen, um die Trennung von Eingabe- und Ausgabewerten klarer zu machen).

Eigentlich habe ich angefangen, folgendes zu schreiben:
Zitat:
Und nun ist daher genau der richtige Zeitpunkt, um diese SPS-Funktionalität in eine eigene Klasse auszulagern.
Erzeuge dazu unabhängig vom bisherigen Code eine neue Klasse (in einer neuen Datei) und füge dort als Member ein Objekt von S7VariablenResult hinzu.
Und nun überlege dir, welche öffentlichen (public) Methoden du benötigst und erstellst dann die Signatur (Methodenkopf) dafür.

Aber dann ist mir anhand des Codes klargeworden, daß eigentlich nur die Methode ReadResult ausgelagert werden kann. Da diese aber eben noch die aus den UI-Controls gelesenen Werte benötigt, müßtest du dafür auch wieder eine Klasse (wie UIValues) als Parameter benutzen (oder aber alle Werte einzeln übergeben), und das macht den Code nicht einfacher.

Vom Design bzw. Architektur ist das aber selbstverständlich das Ziel.

Du solltest dir also überlegen, ob du nicht doch zwei getrennte Klassen für die S7-Eingabe und Ausgabe erzeugst (und auf die Gesamtklasse S7VariablenResult verzichtest). Alternativ könntest du deine SPS-Klasse mit den einzelnen Daten als Eigenschaften ausstatten (also statt getrennter Daten- und Codeklasse).

PS: Mir ist auch noch eingefallen, daß du für z.B. EnableTextBoxes und AreAllTextBoxesFilled auch noch ein Array anlegen könntest und diese dann per Schleife (oder LINQ) die Werte setzt bzw. abfragst. Dies ginge dann sogar auch für die Settings (wenn du die Namen der Controls identisch zu den Settingswerten machst, abgesehen vom Präfix txt).


Ralf Jansen - Di 31.05.22 09:58

Zitat:
Hattest du meinen Beitrag gelesen wo ich dich wegen dem OneShot-Timer was gefragt hatte?
Ich wollte halt nur wissen ob das so ok ist das ich den Timer stoppe oder gar nicht nötig ist.


Tatsächlich übersehen. Welche Sorte Timer man nimmt hängt natürlich von den Anforderungen ab.
Jetzt hast du einen "Mach was regelmäßig mit einer Pause dazwischen" Ding. Der Abstand hängt davon ab wie lange das kopieren dauert die einstellbare Dauer des Timers steuert jetzt die Pausenlänge nicht genau in welchem Abstand das kopieren initiiert werden soll. Wenn das ok ist ist das ok.

Wenn deine Anforderung wäre es muß wenn möglich haargenau alle 5 Minuten passieren oder so dann nicht. Wenn die Anforderung wäre kopiere immer wenn sich was geändert hat dann bräuchte man wieder was ganz anderes.

Kann die Frage ob das nötig ist oder nicht nicht wirklich beantworten ohne genauere Anforderungen zu kennen. Möglicherweise macht es für deinen Anwendungsfall keinen Unterschied ob du stoppst oder nicht.


UserTom - Mi 01.06.22 13:21

Hallo,

Vielen Dank für die Antworten.

Zitat:
Zuerst einmal zum One-Shot-Timer:

Ich habe es getestet, ob ich den Timer laufen lassen kann. Kann ich nicht.
Es wird zwar kopiert, aber nicht so wie es soll und auf der Visu flackert alles.

Zitat:
Was ich jetzt aber noch im Code gesehen habe: Warum werden im Timer-Event TimerCopyAllFiles_Tick

Hattest du auch in der aktuellen Datei geschaut? Das ist nicht mehr so. Ich hänge noch eine neue Datei dran.

Zitat:
Die uiValues sollen ja nur die UI-Elemente abdecken, welche für den asynchronen Vorgang benötigt werden


Ich habe nur eine Klasse mit dem Namen S7VariablenResult, die enthält alle Variablen die aus dem Datenbaustein gelesen werden müssen.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
namespace PDE
{
    // Variablen von der SPS
    class S7VariablenResult
    {
        public string PraegeCodeNr;     // Prägecode Hauptteil
        public string FolderName;       // Ordnername
        public string PraegeCode2Nr;    // Prägecode Zusatzteil
        public bool TrigSaveFiles;      // Daten kopieren
        public bool TypLinks;           // Typ Links
        public bool TypRechts;          // Typ Rechts
        public bool RoboterDSLeer;      // Roboter hat kein Teil im Greifer
        public bool FMBauteil;          // Fertigmeldung Bauteil
        public bool HeartBeat;          // Heartbeat
    }
}


Dann gibt es die Methode private S7VariablenResult ReadResult().
Diese Methode muss in einem Loop (Timer) an vielen Stellen im Code aufgerufen werden,
und zwar dann, wenn es notwendig ist, die Variablen im Datenbaustein abzufragen.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
 
        private S7VariablenResult ReadResult()
        {
            S7VariablenResult s7VariablenResult = new S7VariablenResult();
            // Reads the buffer.

            byte[] dbbuffersize = new byte[uivalues.DBSize];

            s7Client.DBRead(uivalues.DBNr, uivalues.DBOffset, uivalues.DBSize, dbbuffersize);

            s7VariablenResult.PraegeCodeNr = S7.GetStringAt(dbbuffersize, uivalues.PraegeCodeDBPos);
            s7VariablenResult.FolderName = S7.GetStringAt(dbbuffersize, uivalues.FolderNameDBPos);
            s7VariablenResult.PraegeCode2Nr = S7.GetStringAt(dbbuffersize, uivalues.PraegeCode2DBPos);
            s7VariablenResult.TrigSaveFiles = S7.GetBitAt(dbbuffersize, uivalues.TrigSaveFilesByte, uivalues.TrigSaveFilesBit);
            s7VariablenResult.TypLinks = S7.GetBitAt(dbbuffersize, uivalues.TypLinksByte, uivalues.TypLinksBit);
            s7VariablenResult.TypRechts = S7.GetBitAt(dbbuffersize, uivalues.TyprechtsByte, uivalues.TyprechtsBit);
            s7VariablenResult.RoboterDSLeer = S7.GetBitAt(dbbuffersize, uivalues.RoboterDSLeerByte, uivalues.RoboterDSLeerBit);
            s7VariablenResult.FMBauteil = S7.GetBitAt(dbbuffersize, uivalues.FMBauteilByte, uivalues.FMBauteilBit);
            s7VariablenResult.HeartBeat = S7.GetBitAt(dbbuffersize, uivalues.HeartByte, uivalues.HeartBit);

            return s7VariablenResult;
        }


Hier nochmal eine Erklärung, welche Variable wofür ist.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
        public string PraegeCodeNr;     // Variable mit dem Wert, um die Dateien zu "suchen". z. B. 22060112573
        public string FolderName;       // Variable mit dem Wert für den Ordnernamen z. B. 22060112573R
        public string PraegeCode2Nr;    // auch eine Variable um die Dateien zu "suchen".

        public bool TrigSaveFiles;      // Triggersignal von der SPS zum Daten kopieren

        public bool TypLinks;           // Typ Links - Der Ordnername bekommt so den Buchstaben (L)
        public bool TypRechts;          // Typ Rechts - Der Ordnername bekommt so den Buchstaben (R)

        public bool RoboterDSLeer;      // Das Teil wurde abgelegt, das ist die Variable um die Visu zu aktualisieren.

        public bool FMBauteil;          // Fertigmeldung Bauteil -  Ist eine wichtige Variable für die if-Bedingung.

        public bool HeartBeat;          // Heartbeat - Für visuelle Zwecke nicht unbedingt notwendig.


Zitat:
Und dazu solltest du ein Objekt von S7VariablenResult als Membervariable anlegen und dessen Werte setzen (anstatt immer wieder lokale Objekte in ReadResult zu erzeugen - daher ja auch mein Vorschlag bzgl. zweier S7-Klassen, um die Trennung von Eingabe- und Ausgabewerten klarer zu machen).

Das verstehe ich gerade nicht so. Kannst du das anhand eines Beispieles bitte erklären?

Zitat:
und das macht den Code nicht einfacher.

Ich denke, dass ich das alles in der Main lassen sollte.

Zitat:
Du solltest dir also überlegen, ob du nicht doch zwei getrennte Klassen für die S7-Eingabe und Ausgabe erzeugst

Was ist für dich die S7-Eingabe (die Textboxen)? Und was die Ausgabe? Ich denke mit Sicherheit anders und bevor ich doppelte Arbeit mache, frage ich besser nach.

Zitat:
Alternativ könntest du deine SPS-Klasse mit den einzelnen Daten als Eigenschaften ausstatten (also statt getrennter Daten- und Codeklasse).

So wie die UIValues?

Zitat:
PS: Mir ist auch noch eingefallen, daß du für z.B. EnableTextBoxes und AreAllTextBoxesFilled auch noch ein Array anlegen
könntest und diese dann per Schleife (oder LINQ)

Du meinst das so?


C#-Quelltext
1:
cboLocalApps = new ComboBox[] { cboLocalApp1, cboLocalApp2, cboLocalApp3, cboLocalApp4, cboLocalApp5, cboLocalApp6, cboLocalApp7, cboLocalApp8 };                    


Ok, kann ich machen.

Zitat:
Dies ginge dann sogar auch für die Settings (wenn du die Namen der Controls identisch zu den Settingswerten machst, abgesehen vom Präfix txt).


Das habe ich nicht so gut verstanden.

So vielen Dank nochmal.

Grüße Tom


Th69 - Mi 01.06.22 15:29

Hallo,

OK, die einmalige Initialisierung der UIValues im Connect ist gut so. :zustimm:

Und bzgl. Eingabe- und Ausgabevariablen meinte ich genau auf die ReadResult()-Methode bezogenen Daten (für s7Client.DBRead und S7.GetStringAt).
Ich war bisher davon ausgegangen, daß auch die Eingabevariablen (PraegeCodeDBPos, FolderNameDBPos, ...), welche du ja nach UIValues verschoben hast, auch noch in der S7VariablenResult-Klasse drin sind und auch dort zum weiteren Bearbeiten benötigt werden (du hattest ja geschrieben, du kannst diese Klasse nicht ändern).

Wenn du doch frei die Klassen bestimmen kannst, dann ist es ja jetzt kein Problem diese auszulagern (nur daß du dann statt dem Namen UIValues eine eigene Klasse anlegen solltest, damit die SPS-Klasse komplett unabhängig [auch vom Namen her] von einer UI ist).

Und deine Frage dazu:
UserTom hat folgendes geschrieben:
Das verstehe ich gerade nicht so. Kannst du das anhand eines Beispieles bitte erklären?

Da bin ich auch noch davon ausgegangen, daß in der S7VariablenResult-Klasse mehr Daten stehen, als du in ReadResult setzt (und dann hättest du ja immer ein neues Objekt in ReadResult erzeugt, ohne die alten Werte zu behalten. Und dies hättest du mit einem Klassenmember (wie du ja auch uiValues nutzt) vermieden.

Wenn dem nicht (mehr) so ist, dann kannst du deinen Code diesbzgl. jetzt so lassen.

Mir geht es vorallem darum, daß du dir Gedanken über die Daten (Klassen) und den Datenfluß machst.

Und bzgl. dem Array: ja, wie die anderen Arrays auch, nur eben jetzt für deine TextBox-Objekte txtIPNr, .., txtHeartBit.

Bei den Settings:
Zitat:
Das habe ich nicht so gut verstanden.

Das habe ich mir fast gedacht. Schau dir einfach mal die Definition der Settings-Eigenschaften an (Datei "Properties/Settings.Designer.cs"). Dann wirst du (hoffentlich) feststellen, daß intern immer dieselbe Methode (bzw. genauer gesagt der Index-Operator) benutzt wird.
Und diesen kann man auch selber ansprechen, wenn man die Settings-Eigenschaften als string hat (wie z.B. der Name der zug. TextBox).
Somit dann analog zu den anderen Arrays in einer Schleife die Settings lesen und schreiben.

Ich habe gesehen, daß du var settings = Settings.Default; (überwiegend) benutzt. Das finde ich gut, da es auch wieder redundanten Code reduziert (du solltest es dann aber konsequent überall für den Zugriff auf die Settings machen ;- ).

PS: Du hast ja für EnableLocalAppControls() sowie EnableNetAppControls() wieder zwei Methoden angelegt (True/False), anstatt nur eine mit einem bool enable-Parameter (wie bei EnableTextBoxes(bool enable)).
Das tiefe Verständnis scheint dir dafür wohl noch zu fehlen?


UserTom - Mi 01.06.22 22:52

Servus,

Zitat:
Ich war bisher davon ausgegangen, daß auch die Eingabevariablen (PraegeCodeDBPos, FolderNameDBPos, ...), welche du ja nach UIValues verschoben hast, auch noch in der S7VariablenResult-Klasse drin sind und auch dort zum weiteren Bearbeiten benötigt werden (du hattest ja geschrieben, du kannst diese Klasse nicht ändern).

Also mit nicht ändern meinte ich, wenn ich die S7VariablenResult Klassen-Datei ändere dann muss natürlich auch die dazugehörige
ReadResult()-Methode angepasst werden. Da aber die Methode so richtig ist wie sie ist (jetzt noch besser weil eingekürzt)
brauch nichts geändert werden. Ich denke wir reden zwecks meiner Idee eine SPS Klassen-Datei zu erstellen vielleicht ein bisschen aneinander vorbei.
Ich wollte eine SPS.cs Datei erstellen und alles was die SPS betrifft auslagern. Also z.B die Methode ConnectToPLC()weil damit die Verbindung
mit der CPU von der SPS hergestellt wird. Die Methode ShowPlcStatus() damit wird ein Status von der CPU abgerufen ob sie rennt oder gestoppt ist. Die Methode ShowS7VariablenStatus(S7VariablenResult s7VariablenResult) fragt die Bit Signale ab. Die Methode Ready() damit schreibe ich in den DB.
Und natürlich auch die Methode S7VariablenResult ReadResult() damit werden die DB-Daten von der SPS gelesen.

Ich hatte das ja schonmal probiert und als ich die Methode S7VariablenResult ReadResult() ausgelagert hatte habe ich nichts mehr zum laufen gebracht.
Und nur wegen der folgenden Methode.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
private S7VariablenResult ReadResult()

        {

warum steht das hier in der Methode? Verstehe ich nicht.
##########################################################################            
S7VariablenResult s7VariablenResult = new S7VariablenResult();
##########################################################################

            // Reads the buffer.

            byte[] dbbuffersize = new byte[uivalues.DBSize];

            s7Client.DBRead(uivalues.DBNr, uivalues.DBOffset, uivalues.DBSize, dbbuffersize);

            s7VariablenResult.PraegeCodeNr = S7.GetStringAt(dbbuffersize, uivalues.PraegeCodeDBPos);
            s7VariablenResult.FolderName = S7.GetStringAt(dbbuffersize, uivalues.FolderNameDBPos);
            s7VariablenResult.PraegeCode2Nr = S7.GetStringAt(dbbuffersize, uivalues.PraegeCode2DBPos);
            s7VariablenResult.TrigSaveFiles = S7.GetBitAt(dbbuffersize, uivalues.TrigSaveFilesByte, uivalues.TrigSaveFilesBit);
            s7VariablenResult.TypLinks = S7.GetBitAt(dbbuffersize, uivalues.TypLinksByte, uivalues.TypLinksBit);
            s7VariablenResult.TypRechts = S7.GetBitAt(dbbuffersize, uivalues.TyprechtsByte, uivalues.TyprechtsBit);
            s7VariablenResult.RoboterDSLeer = S7.GetBitAt(dbbuffersize, uivalues.RoboterDSLeerByte, uivalues.RoboterDSLeerBit);
            s7VariablenResult.FMBauteil = S7.GetBitAt(dbbuffersize, uivalues.FMBauteilByte, uivalues.FMBauteilBit);
            s7VariablenResult.HeartBeat = S7.GetBitAt(dbbuffersize, uivalues.HeartByte, uivalues.HeartBit);

            return s7VariablenResult;
        }


Zitat:
Wenn du doch frei die Klassen bestimmen kannst, dann ist es ja jetzt kein Problem diese auszulagern

Ich kann alles tun aber ich weiß leider nicht wie. Das mit dem "= new" ist am schlimmsten für mich.
Manchmal macht es Ahaaa und manchmal stehe ich da wie blöde.

Zitat:
Und dies hättest du mit einem Klassenmember (wie du ja auch uiValues nutzt) vermieden.

Zum Beispiel hier da weiß ich nicht auf welchen Klassenmeber du dich beziehst. Das beneide ich Euch sehr weil ihr diese Wörter nutzen könnt.
Ich weiß das ein Klassenmember vieles sein kann. Aber was du mit Klassenmeber jetzt meinst weiß ich nicht.

Zitat:
Mir geht es vorallem darum, daß du dir Gedanken über die Daten (Klassen) und den Datenfluß machst.

Das mache ich aber das ist nicht so einfach.

Zitat:
Und bzgl. dem Array: ja, wie die anderen Arrays auch, nur eben jetzt für deine TextBox-Objekte txtIPNr, .., txtHeartBit.

Ich hänge noch ein paar Dateien mit dran. Ansonsten müsste es jetzt noch besser sein.

Zitat:
Bei den Settings:

Die Datei habe ich mir angeschaut es wiederholt sich alles. Aber keine Ahnung was ich davon nutzen könnte.

Zitat:
(bzw. genauer gesagt der Index-Operator)

Was ist denn ein Index-Operator.

Zitat:
Ich habe gesehen, daß du var settings = Settings.Default;

Ich habe alle überarbeitet.

Zitat:
PS: Du hast ja für EnableLocalAppControls()

Habe ich auch angepasst.

Vielen Dank nochmal für deine Hilfe.

Grüße Tom


Th69 - Do 02.06.22 10:53

UserTom hat folgendes geschrieben:

C#-Quelltext
1:
2:
3:
4:
warum steht das hier in der Methode? Verstehe ich nicht.
##########################################################################
S7VariablenResult s7VariablenResult = new S7VariablenResult();
##########################################################################

Genau das meinte ich mit "anstatt immer wieder lokale Objekte in ReadResult zu erzeugen". Und mit Klassenmember meine ich Felder (fields) und Eigenschaften (properties) [ich benutze das Wort "Feld(er)" ungern in diesem Zusammenhang, da es in anderen Programmiersprachen, z.B. C oder C++, als Synonym für Array steht].

Hättest du also dieses Objekt außerhalb der Methode:

C#-Quelltext
1:
private S7VariablenResult s7VariablenResult = new S7VariablenResult();                    

so würde es nur einmalig angelegt werden und du könntest direkte dessen Werte ändern (dann könnte auch die Rückgabe entfallen und die aufgerufenen Methoden verwenden ebenfalls direkt diesen Klassenmember s7VariablenResult).

Das Auslagern aller (bisherigen) SPS-Methoden in eine eigene Klasse wird schwierig, da du in diesen ja auch auf die UI-Elemente der Form zugreifst.
Trotzdem würde man in einem größeren professionellem Projekt diese auslagern (Stichworte: Wrapper / Interface). Dies wäre erst einmal Mehraufwand, entkoppelt aber die Form- und die S7Client-Klasse.
Im Anhang mal ein Ansatz dafür:
- bei PlcGetStatus müßtest du die UI-relevanten Status-Abfragen trotzdem in der Form-Klasse durchführen, außer Text und BackColor würden über die SPS-Klasse gesetzt -> Hinzufügen zum Rückgabetyp (falls dir die Schreibweise (x, y, z) nichts sagt: Tupeltypen [https://docs.microsoft.com/de-de/dotnet/csharp/language-reference/builtin-types/value-tuples]) - alternativ kannst du auch eine Datenklasse bzw. Eigenschaften dafür erzeugen, s. nächster Punkt.
- für ReadResult habe ich dir 2 Versionen erstellt, einmal mit den "Eingabe"-Werten als Eigenschaften und einmal als extra Klasse S7Input.
Die Erzeugung und Rückgabe des lokalen Objekts s7VariablenResult habe ich jetzt mal so gelassen (sofern S7VariablenResult auch nur die Werte umfaßt, die von ReadResult gesetzt werden - so wie du es ja in deinem letzten Code hast). Die entsprechenden Eigenschaften in UIValues solltest du dann entfernen, und nur noch z.B. die Pfadnamen dort drin haben.

Du solltest dir auch noch mal die ShowResult-Methode anschauen, denn nach dem Code-Umbau macht sie jetzt so keinen Sinn mehr, da sie den ErrorText nicht mehr direkt ausgibt, sondern in uiValues.Error schreibt, welches nirgendwo benutzt wird.

Und nochmal wegen der Settings:
Der Indexoperator ist hier ApplicationSettingsBase.Item[String] [https://docs.microsoft.com/de-de/dotnet/api/system.configuration.applicationsettingsbase.item], also der Zugriff mittels der eckigen Klammern [...] (in der Doku wird der Index-Operator immer "Item[]" genannt).
Da dich das alles zurzeit eher verwirrt, laß das mal mit den Settings (ich wollte dir nur weitere Codeverkürzungen aufzeigen - diese wäre aber nicht so performant wegen den Stringmethoden).


UserTom - Do 02.06.22 19:47

Guten Abend TH69,

Vielen Dank für deine Antwort.

Man da habe ich was zu tun das zu alles durch zuarbeiten. Hat viel mit testen zu tun.

Aber ich hätte erstmal ein anderes Problem.

In der Methode ConnectBtn_Click werden ja die UIValues geladen. Mit AreAllSPSTextBoxesFilled() wird man erinnert wenn eine Textbox nicht
ausgefüllt ist. Und diese Methode, scheint von mir nicht richtig erstellt worden.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
 private void ConnectBtn_Click(object sender, EventArgs e)
        {
####################################################################################         
   if (AreAllSPSTextBoxesFilled() == true// All text boxes must be filled out.
#####################################################################################
   {
       uivalues.Error = txtError.Text;

       uivalues.IPNr = txtIPNr.Text;
       uivalues.RackNr = int.Parse(txtRackNr.Text);
       uivalues.SlotNr = int.Parse(txtSlotNr.Text);

       uivalues.DBNr = int.Parse(txtDBNr.Text);
       uivalues.DBOffset = int.Parse(txtDBOffsetNr.Text);
       uivalues.DBSize = int.Parse(txtDBSizeNr.Text);




C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
private bool AreAllSPSTextBoxesFilled()
        {
            for (int i = 0; i < tbSPSConectionsValues.Length; i++)
            {
                if (!string.IsNullOrEmpty(tbSPSConectionsValues[i].ToString()))
                {
                    return true;
                }
            }

            return false;
        }


Keine Ahnung wo der Fehler ist. Könntest du mal da drüber schauen.

Zitat:
Du solltest dir auch noch mal die ShowResult-Methode anschauen, denn nach dem Code-Umbau macht sie jetzt so keinen Sinn mehr,

Ich habe sie gerade getestet sie zeigt mir aber die Werte an die sie mir anzeigen soll. Deswegen weiß ich jetzt nicht wirklich was daran
nicht richtig ist.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
private void ShowResult(int Result)
        {
            // This function returns a text explanation of the error code.
            uivalues.Error = s7Client.ErrorText(Result);

            if (Result == 0)
            {
                uivalues.Error = uivalues.Error + " (" + s7Client.ExecutionTime.ToString() + " ms)";
            }
        }


Die Orginale sah so aus!


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
 private void ShowResult(int Result)
        {
            // Diese Funktion gibt eine Texterklärung des Fehlercodes zurück
            txtError.Text = s7Client.ErrorText(Result);
            if (Result == 0)
            {
                txtError.Text = txtError.Text + " (" + s7Client.ExecutionTime.ToString() + " ms)";
            }
        }


Vielen Dank nochmal für deine Hilfe und vor allem für die SPS.cs.

Schönen Abend noch.

Grüße Tom


Ralf Jansen - Do 02.06.22 21:53

Zitat:
Keine Ahnung wo der Fehler ist.


Logisch fast richtig nur falsch rum gedacht. Du brichst ab sobald ein Wert gesetzt ist und sagst alles in Ordnung(true). Du möchtest ab das sobald eine Wert NICHT gesetzt ist abbrechen und sagen es ist nicht in Ordnung(false).

Es sind übrigens TextBoxen und keine Values die enthalten vielleicht Text sind aber kein Text. Ein TextBox.ToString() liefert dir hier vermutlich den Typ Namen ala "System.Winforms.TextBox" bzw. den gesamten beschreibenden Context der TextBox aber nicht nur den TextInhalt eine TextBox kann halt viel mehr als den Text. Ein TextBox (wie eigentlich alle Controls) hat einfach eine Text Property.
Also eher

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
private bool AreAllSPSTextBoxesFilled()
{
    for (int i = 0; i < tbSPSConectionsValues.Length; i++)
        if (string.IsNullOrEmpty(tbSPSConectionsValues[i].Text))
            return false;
    return true;
}

oder wieder die Kurzform

C#-Quelltext
1:
private bool AreAllSPSTextBoxesFilled() => tbSPSConectionsValues.All(tb=> !string.IsNullOrEmpty(tb.Text));                    


Bei if (AreAllSPSTextBoxesFilled() == true) kann das == true weg. Aus der Methode kommt schon ein boolean raus und das vergleichen mit einem boolean ändert den kein bißchen.


UserTom - Fr 03.06.22 06:53

Guten Morgen,

@ Ralf

Vielen Dank für deine Antwort und die Erklärung.
Funktioniert natürlich und sehr ärgerlich, dass ich das nicht gefunden habe, weil es war ja nicht wirklich schwer.
Fast schon peinlich.

Zitat:
oder wieder die Kurzform

Könntest du mir die Kurzform erklären? Ich weiß, dass sie funktioniert, weil du mir das als Option hier mit reingeschrieben hast, aber in Wirklichkeit hätte ich damit nichts anfangen können. return true / false sind auch weg.
Es ist, glaube ich, mit LINQ geschrieben? Die Pfeile sind Lambdaausdrücke. Die größere Variante ist verständlicher, was da passiert.

Vielen Dank für deine ganzen Erklärungen.

@ TH69

Nochmal.
Zitat:
Du solltest dir auch noch mal die ShowResult-Methode anschauen,

In nehme meine Aussage zurück. Funktioniert nicht. Ich bin wieder auf das Original gegangen.
txtError.Text braucht über die UIValues auch nicht geladen werden.

Auch dir Danke für deine tolle Hilfe.

Einen schönen Tag.
Grüße Tom

Moderiert von user profile iconTh69: C#-Tags hinzugefügt


Th69 - Fr 03.06.22 09:35

Hallo UserTom,

wegen der Kurzschreibweise: wenn du englisch kannst, s. Help understanding this code snippet [https://www.reddit.com/r/csharp/comments/v1wx38/help_understanding_this_code_snippet/] (es sind also zwei verschiedene Bedeutungen von =>).

C# wird für Anfänger leider nicht einfacher, weil mit jeder Version (aktuell ja C#10, auf dem Weg ist schon C#11) neue Features und Syntax hinzukommt (da bin ich für mich froh, daß ich direkt mit C#1 [und .NET 1.1] angefangen hatte).


Ralf Jansen - Fr 03.06.22 13:44


C#-Quelltext
1:
private bool AreAllSPSTextBoxesFilled() => tbSPSConectionsValues.All(tb=> !string.IsNullOrEmpty(tb.Text));                    

Das entspricht dem hier wenn man mal den Expression Body (In deutsch Ausdruckskörpermember :puke:) Syntax wegläßt und einfach einen klassischen Methodenblock benutzt so wie er dir vermutlich geläufiger ist.

C#-Quelltext
1:
2:
3:
4:
private bool AreAllSPSTextBoxesFilled(List<string> tbSPSConectionsValues)
{
    return tbSPSConectionsValues.All(tb => !string.IsNullOrEmpty(tb));
}

All is dann tatsächlich eine Linq Methode. Die du dir in deinem Fall so implementiert vorstellen kannst. Ziemlich ähnlich zudem was du dir selbst mit einer Schleife gebaut hast.


C#-Quelltext
1:
2:
3:
4:
5:
6:
public bool All(TextBox[] source, Func<TextBox, bool> predicate) {
   foreach (TextBox textBox in source) 
       if (!predicate(textBox)) 
          return false;
   return true;
}

Heißt Linq versteckt meist nur die Schleifen (for, foreach, while was auch immer) ist aber keine Magie die passieren natürlich trotzdem.


Eigentlicher Code für All https://referencesource.microsoft.com/#System.Core/System/Linq/Enumerable.cs,be4bfd025bd2724c

user profile iconTh69 Für einen "ach du Sch***e Moment bezüglich neuen Features empfehle ich einen Blick auf Raw Strings https://github.com/dotnet/csharplang/issues/4304


Th69 - Fr 03.06.22 15:12

Ja, ich weiß - kenne ich auch schon von C++: Raw String Literal in C++ [https://www.geeksforgeeks.org/raw-string-literal-c/]
Ich frage mich generell dabei, wer benutzt festkodierte mehrzeilige String-Literale in seinem Code (die meisten dieser Strings sollten doch entweder z.B. für Übersetzungen aus Ressourcen stammen oder aus Dateien o.ä. eingelesen werden).


UserTom - Fr 03.06.22 23:42

Servus,


Zitat:
wegen der Kurzschreibweise: wenn du englisch kannst,

Also Spanisch kann ich besser. Aber Google wird mir schon dabei helfen.

@ Ralf

Vielen Dank für deine Erklärungen und die Beispiele. Wird nicht einfach sein das alles zu verstehen.
Aber es ist wichtiger das der Code so gut wie möglich wird.

Deshalb eine Bitte und mal sehen wie ihr darüber denkt.

@ TH69

Schon wieder.
Zitat:
Du solltest dir auch noch mal die ShowResult-Methode anschauen,

Als ich mich um diese Methode gekümmert habe, ist mir aufgefallen das sie oft gebraucht wird.
An einigen Stellen sind immer wieder ähnliche Abfragen für die Variablen der SPS.
Ich habe leider nur das Gefühl das man das zusammenfassen kann ich weiß aber leider nicht wie.

Also es geht um folgende Codes:


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
    private void ConnectToPLC()
        {
            int Result = s7Client.ConnectTo(uivalues.IPNr, uivalues.RackNr, uivalues.SlotNr);
            ShowResult(Result);
            if (Result == 0// connection successfull
            {
                EnableTextBoxes(false);



C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
    private void ShowPlcStatus()
        {
            int Status = 0;
            int Result = s7Client.PlcGetStatus(ref Status);
            ShowResult(Result);
            if (Result == 0)
            {
                switch (Status)
                {
                    case S7Consts.S7CPUSTATUSRUN



C#-Quelltext
1:
2:
3:
4:
        byte[] buffersize = new byte[1];

            int Result = s7Client.WriteArea(S7AreaDB, dbnumber, dboffset, 1, S7WLBit, buffersize);
            ShowResult(Result);


Und zum Schluß die Methode die immer aufgerufen wird.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
    private void ShowResult(int Result)
        {
            // Diese Funktion gibt eine Texterklärung des Fehlercodes zurück
            txtError.Text = s7Client.ErrorText(Result);
            if (Result == 0)
            {
                txtError.Text = txtError.Text + " (" + s7Client.ExecutionTime.ToString() + " ms)";
            }
        }

Ich würde gerne die ganzen Visu Sachen gleich in die ShowResult Methode mit integrieren.
Über die Argumentklammern wollte ich dann eine entsprechende Argumentliste mit übergeben.

Über mehrere If Bedingung wird dann die Ausgabe gesteuert.

z.B.

C#-Quelltext
1:
private void ShowResult(int Result, bool PlcGetStatus, bool connectTo,....)                    


So vielen Dank nochmal für Eure toole Hilfe.

Grüße Tom


Th69 - Sa 04.06.22 09:33

Das solltest du m.E. nicht so umsetzen, da diese Methode dann zuviele verschiedene Dinge macht (zuviele Abhängigkeiten).

Mir ist nur aufgefallen, daß du in ShowPlcStatus erst ShowResult() aufrufst und direkt danach noch jeweils txtError.Text direkt zuweist (diese Zuweisungen können doch dann gelöscht werden, oder?).

Und in ShowResult

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
if (Result == 0)
{
    // ...
}
else
{
    if (Result > 0)
    {
        // ..
    }
}

ist entweder die Abfrage if (Result > 0) überflüssig, oder es fehlt noch die Abfrage auf Result < 0 (je nachdem was negative Werte bedeuten).

Es gibt auch noch eine kleine Speicher- und Performanceverbesserung für ShowResult:

C#-Quelltext
1:
2:
3:
4:
if (Result == 0)
{
    txtError.AppendText(" (" + s7Client.ExecutionTime + " ms)"); // .ToString() ist hier auch überflüssig, da der + Operator automatisch diese aufruft
}
(es entfällt die aufwendigere + Operation, s.a. TextBoxBase.AppendText [https://docs.microsoft.com/de-de/dotnet/api/system.windows.forms.textboxbase.appendtext]).

Benutzt du denn jetzt die SPS-Klasse?


UserTom - So 05.06.22 19:50

Hallo TH69,

Vielen Dank für deine Antworten.


Zitat:
Das solltest du m.E. nicht so umsetzen,

Ok, deswegen habe ich gefragt.

Zitat:
Und in ShowResult


Result == 0 ist OK und alles über 0 sind Fehler.

Ein Ausschnitt:

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
  public string ErrorText(int Error)
        {
            switch (Error)
            {
                case 0return "OK";
                case S7Consts.ERRTCPSOCKETCREATION: return "SYS : Error creating the Socket";
                case S7Consts.ERRTCPCONNECTIONTIMEOUT: return "TCP : Connection Timeout";
                case S7Consts.ERRTCPCONNECTIONFAILED: return "TCP : Connection Error";
                case S7Consts.ERRTCPRECEIVETIMEOUT: return "TCP : Data receive Timeout";
                case S7Consts.ERRTCPDATARECEIVE: return "TCP : Error receiving Data";
                case S7Consts.ERRTCPSENDTIMEOUT: return "TCP : Data send Timeout";
                case S7Consts.ERRTCPDATASEND: return "TCP : Error sending Data";


Zitat:
Benutzt du denn jetzt die SPS-Klasse?


Ich habe mir eine Kopie von meinem Projekt erstellt und versuche die SPS.cs darin zu integrieren.
Zur Zeit ruht die Sache da ich verärgert bin weil ich einige Zusammenhänge nicht verstehe.

Zitat:

C#-Quelltext
1:
public S7VariablenResult ReadResult2(S7Input s7Input)                    

Mit ReadResult2 habe ich Probleme zu verstehen was ich damit machen soll.
Ich verstehe nicht warum ich "2 mal" aus dem Datenbaustein lesen soll.
Aber lass gut sein ich komme schon dahinter ich brauche halt meine Zeit dafür.

Also vielen Dank noch mal für die tolle Hilfe.

Schöne Pfingsten noch.

Grüße Tom


Th69 - So 05.06.22 20:17

ReadResult1 und ReadResult2 sind zwei verschiedene Möglichkeiten, die Methode ReadResult zu implementieren (einmal mit Setzen von Klasseneigenschaften und beim der zweiten mittels Übergabe eines Parameters der Klasse S7Input, für welche dann die Eigenschaften vor dem Aufruf gesetzt werden müssen).
Nimm die, welche du besser verstehst.


UserTom - Mo 06.06.22 23:40

Servus Th69,

Vielen Dank für deine Antwort.

Ok, jetzt weiß ich bescheid, echt blöde das ich das nicht selber erkannt habe.

Ich habe da eine Situation gehabt die ich optimieren möchte aber bevor ich da was mache frage ich Euch mal um Rat.
Also es wäre möglich das das Tool gestartet ist aber nicht verbunden. Ich muss also in der SPS ein Bit mit einbinden das sicherstellt das das Trigger Signal nur gesetzt werden kann, wenn eine Verbindung aufgebaut ist.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
 private void TimerCopyAllFiles_Tick(object sender, EventArgs e)
{
    S7VariablenResult s7VariablenResult = ReadResult();

    if (s7Client.Connected )
    {
        // set a Bit...?
    }
}

Meine erste Frage wäre ob ich den Backgroundworker auch mit einer Schleife und Thread.Sleep aufrufen könnte?
Es geht mir darum auf den Windows Forms Timer zu verzichten.
Der Intervall ist auf 500 eingestellt und welcher Wert da der Richtige ist weiß ich nicht. Das müsste man messen.
Meine erste Idee ist ein Heartbeat aufzubauen aber dafür bräuchte ich einen Timer in der S7 Schnittstelle.
Ich denke das würde sich mit dem Windows Forms Timer beißen. Ist dieser Timer eigentlich in Kombination mit dem Backgroundworker der richtige Timer?

Dann habe ich das hier gelesen: Muss es immer Timer sein? [https://www.vb-paradise.de/index.php/Thread/75675-muss-es-immer-timer-sein/]

Kann man mit INotifyPropertyChanged vielleicht eine Variable von der SPS überwachen?
Und wenn ja mit einem Event den Backgroundworker aufrufen?
Wenn ich einen Timer in der SPS habe und einen Timer im Programm dann kommt es zu komischen Reaktionen da die Schnittstelle nicht in Echtzeit überträgt.

Was sollte ich Eurer Meinung nach machen?

Vielen Dank für die bisherige Hilfe.

Grüße Tom

Moderiert von user profile iconTh69: C#-Tags hinzugefügt
Moderiert von user profile iconTh69: URL-Titel hinzugefügt.


Th69 - Di 07.06.22 08:30

Das hatte ich dir aber auch in meinem Beitrag vom 2.6. schon geschrieben:
Th hat folgendes geschrieben:
für ReadResult habe ich dir 2 Versionen erstellt, einmal mit den "Eingabe"-Werten als Eigenschaften und einmal als extra Klasse S7Input.


Leider verstehe ich dein Problem mit dem Trigger-Signal nicht so richtig. Es gibt also in der S7-Schnittstelle eine Methode zum Setzen dieses Bits, aber du mußt jetzt dieses in einem festen Intervall triggern (während das Programm damit verbunden ist).

Beim Windows-Timer ist zu wissen, daß dieser (auf den meisten Systemen, je nach Hardware) nur eine Präzision von ca. 15-16ms hat (d.h. max. 64x je Sekunde aufgerufen wird), s. z.B. PowerShell Sleep Duration Accuracy and Windows Timers [https://xkln.net/blog/powershell-sleep-duration-accuracy-and-windows-timers/] (laß dich von dem "PowerShell" nicht verwirren, dieser Artikel gilt allgemein für .NET bzw. Windows).
Es gibt aber genauere, z.B. Multimedia Timer [for .NET Core] [https://github.com/MikeCodesDotNET/Multimedia-Timer] (bedenke aber, daß dieser dann im Gegensatz zum Windows.Forms.Timer nicht direkt auf die UI zugreifen kann, du also genauso wie für den BackgroundWorker programmieren müßtest).

Ich sehe aber jetzt ersteinmal (bis auf evtl. höhere Präzision) nichts, was gegen deinen bisherigen Code spricht (also Windows.Forms.Timer und BackgroundWorker).

Und wenn die S7-Schnittstelle selbst kein Event bereitstellt, dann hilft nur ein regelmäßiges Setzen (bzw. Abfragen).


Ralf Jansen - Di 07.06.22 09:49

Deine ReadResult liest doch auch von der s7? Da müßte genauso die Prämisse gelten das du zu dieser verbunden sein mußt.

Zitat:
Meine erste Frage wäre ob ich den Backgroundworker auch mit einer Schleife und Thread.Sleep aufrufen könnte?

Eher nicht weil Thread.Sleep den Thread anhält. Da das im Moment der UI Thread wäre hälst du deine UI an.
Eine Option wäre die Schleife IM Backgroundworker zu haben den kannst du gefahrlos immer wieder pausieren weil der ja aufgabenspezifisch ist und nicht noch andere Aufgaben hat (wie z.B. die UI zu aktualisieren). Dann mußt du dir aber wieder Gedanken machen wie du synchronisierte Aufrufe aus dem Backgroundworker machst um auf die UI zuzugreifen. Das war gerade der Charme der Lösung Winforms Timer + Backgroundworker. Du konntest erst den UI zugriff machen und dann den Backgroundworker starten.

Übrigens dein Link ist wenig hilfreich bezügl Timer. Das ein anderes System wie z.b. die s7 regelmäßig Bescheid gibt (Signale sendet das sich was geändert hat) ist eher selten. Meist muß man doch selber regelmäßig prüfen. Ob dein System entsprechende Signale (wie auch immer) sendet und du damit teoretisch ohne regelmäßiges prüfen auskommst sollte dir die Dokumentation sagen.


UserTom - Mi 08.06.22 11:02

Servus,

Vielen Dank für Deine Antworten.

Zitat:
Leider verstehe ich dein Problem mit dem Trigger-Signal nicht so richtig

Also ich kann mit:


C#-Quelltext
1:
2:
 if (s7Client.Connected)
{


Überprüfen, ob das Tool sich mit der SPS verbunden hat.

Die SPS selbst weiß gar nicht, dass dieses Tool überhaupt existiert. Wird sie auch nie.
Das Tool ist dafür da, sich mit der SPS zu verbinden, Bits zu lesen, die IO-Operationen durchzuführen und dann Bits auf 0 oder 1 zu setzen.

Nun gut ich habe es geschafft ein Bit in der SPS zu setzen, wenn das Tool die Verbindung aufgebaut hat.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
 private void TimerCopyAllFiles_Tick(object sender, EventArgs e)
{
 S7VariablenResult s7VariablenResult = ReadResult();

 if (s7Client.Connected)
 {
   CPUIsConnected();
 }
 else
 {
   btnConnectToSPS.Enabled = true;
 }

   ShowS7VariablenStatus(s7VariablenResult);
   ShowPlcStatus();

if (BW_CopyAllFiles.IsBusy != true)
{
if (s7VariablenResult.TrigSaveFiles && (s7VariablenResult.TypLinks || s7VariablenResult.TypRechts) && !s7VariablenResult.FMBauteil)
{
  TimerCopyAllFiles.Enabled = false;
  BW_CopyAllFiles.RunWorkerAsync();
}
}
}



C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
 private void CPUIsConnected()
{
  byte[] buffersize = new byte[1];

  S7.SetBitAt(ref buffersize, 0, uivalues.ConnectToPLCBit, true);

  s7Client.DBWrite(uivalues.DBNr, uivalues.ConnectToPLCByte, buffersize.Length, buffersize);
}


Das ist schon ein Anfang, aber leider nicht ausreichend. Zum Beispiel, das Tool wird geschlossen. Dann ist das Bit aber trotzdem immer noch in der SPS gesetzt.
Die SPS macht weiterhin ihre Arbeit so wie die Bits gesetzt werden oder auch nicht.
Was ich jetzt brauche, ist eine watchdog timer (Sekundentakt) der vom Tool ausgesendet wird. Und dieses Signal 0 oder 1 kann ich dann in der SPS überwachen.
Ich habe jetzt in der S7-Schnittstelle (Sharp7.cs) auch einen Timer gefunden, mal sehen, ob das funktioniert.

Zitat:
Ich sehe aber jetzt ersteinmal (bis auf evtl. höhere Präzision) nichts, was gegen deinen bisherigen Code spricht

Ok, das freut mich, dass ich das so lassen kann. Wäre ohne Eure Hilfe gar nicht möglich gewesen, vielen Dank dafür.
Was meinst Du mit der höheren Präzision?

@Ralf

Vielen Dank auch an Dich.

Zitat:
Deine ReadResult liest doch auch von der s7?

Ich denke das, was ich oben beschrieben habe, müssten vielleicht Deine Frage beantworten.

Zitat:
Das war gerade der Charme der Lösung Winforms Timer + Backgroundworker.

Jetzt so, mit dem neuen Hintergrundwissen über Thread ist es so wohl am besten.
Ich hab halt keine Erfahrung und damit weiß ich nicht wann ich was benutzen kann.
Also viel lesen, testen und fragen, wenn es nicht weiter geht.

So, jetzt noch eine andere Sache, auf die ich gekommen bin, wo ich denke, dass sie zur Qualität beiträgt.
Bei mir ist alles auf einer Form (MainForm) untergebracht. Und alles, was zum Konfigurieren gehört, würde ich gerne auf eine andere Form (ConfigurationForm) auslagern. Ich habe den Button auf der MainForm erstellt.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
 private void Configuration_Click(object sender, EventArgs e)
{
  this.Hide();
  ConfigurationForm configurationForm = new ConfigurationForm();
  configurationForm.ShowDialog();
  if (configurationForm.DialogResult == DialogResult.OK)
  {
     configurationForm.Close();
  }

     this.Show();
}



C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
namespace PDE
{
    public partial class ConfigurationForm : Form
    {

        public ConfigurationForm()
        {
            InitializeComponent();
        }        

        private void ApplyConfiguration_Click(object sender, System.EventArgs e)
        {
            this.DialogResult = DialogResult.OK;
            this.Close();
        }

        private void CancelConfiguration_Click(object sender, System.EventArgs e)
        {
            this.DialogResult = DialogResult.Cancel;
            this.Close();
        }

        public string TextInTextBoxAufForm2
        {
            get { return this.txtAufForm2.Text; }
            set { this.txtAufForm2.Text = value; }
        }
    }
}



Die MainForm wird versteckt und die ConfigurationForm wird angezeigt.
Es werden alle Einträge gemacht und wenn fertig, dann die Form mit dem Button schließen.
Das einzige, was sich ändern soll, ist, dass die Werte von den UI-Elementen anstatt von der MainForm jetzt von der ConfigurationForm geholt werden.

Könnt Ihr mir anhand der Textbox "txtAufForm2" sagen, wie ich von der MainForm mit Property und/oder Methoden den Wert erhalte?
Oben ist ein Beispiel mit der Textbox. Die Form aufrufen funktioniert. Die Buttons funktionieren auch.

Was natürlich mein größter Alptraum ist, sind die ganzen Arrays, die ich in der MainForm habe.

@ TH69
PS: Du hast doch auch über Kommunikation mit 2 Forms geschrieben.
Wenn, man den Link auswählt, kommt immer die Meldung, die Seite kann nicht aufgerufen werden.

Grüße Tom


Th69 - Mi 08.06.22 11:22

Nur ersteinmal kurz (weil ich jetzt weg muß), der Artikel ist jetzt hier in den "C# & .NET Tutorials": Kommunikation von 2 Forms [https://entwickler-ecke.de/topic_Kommunikation+von+2+Forms_117789.html]

Edit:
OK, jetzt verstehe ich dein Trigger Signal Problem besser. Dann schau mal, ob du es mit dem S7-Timer hinbekommst.

UserTom hat folgendes geschrieben:
Was meinst Du mit der höheren Präzision?

Das von mir beschriebene (beschränkte) Timer-Intervall. Aber bei dem Trigger-Signal kommt es wohl nicht auf Millisekunden an, daher sollte es so funktionieren.

UserTom hat folgendes geschrieben:
Könnt Ihr mir anhand der Textbox "txtAufForm2" sagen, wie ich von der MainForm mit Property und/oder Methoden den Wert erhalte?

Da du die ConfigurationForm modal mit ShowDialog aufrufst:

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
private void Configuration_Click(object sender, EventArgs e)
{
  this.Hide();
  ConfigurationForm configurationForm = new ConfigurationForm();
  /*var result =*/ configurationForm.ShowDialog();
  if (configurationForm.DialogResult == DialogResult.OK) // alternativ: if (result == DialogResult.OK)
  {
     // configurationForm.Close(); // Dies ist bei ShowDialog überflüssig, da das Fenster automatisch bei OK oder Cancel geschlossen wird.

     var text = configurationForm.TextInTextBoxAufForm2; // Eigenschaft auslesen
  }

  this.Show();
}

Genau dabei sollte dir auch mein Artikel helfen, damit du die Übergabe von Daten zwischen verschiedenen Forms und/oder Klassen besser verstehst.

Bei den vielen Eigenschaften solltest du evtl. besser gleich ein Objekt einer Configuration-Klasse (die ich ja einfach UIValues genannt habe), an die ConfigurationForm übergeben, damit diese direkt (bei OK) befüllt wird.


UserTom - Mi 08.06.22 18:52

Hallo TH69,

Vielen Dank für den Beispielcode.

Ich denke den habe ich verstanden wie ich den Inhalt einer Textbox auf der ConfigurationForm in eine Variable in der MainForm bekomme.

Ich habe das jetzt mal richtig ausprobiert und habe die Textbox txtChooseTargetPath von der Mainform runtergenommen und auf die neue Form gesetzt.

Diesen Code angepasst:


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
namespace PDE
{
    public partial class ConfigurationForm : Form
    {        
        public ConfigurationForm()
        {
            InitializeComponent();
        }        

        private void ApplyConfiguration_Click(object sender, System.EventArgs e)
        {
            this.DialogResult = DialogResult.OK;
            this.Close();
        }

        private void CancelConfiguration_Click(object sender, System.EventArgs e)
        {
            this.DialogResult = DialogResult.Cancel;
            this.Close();
        }
############################################################
        public string TextChooseTargetPath
        {
            get { return this.txtChooseTargetPath.Text; }
            set { this.txtChooseTargetPath.Text = value; }
        }
############################################################
    }
}


In der MainForm auch:


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
 private void Configuration_Click(object sender, EventArgs e)
        {
            this.Hide();
            ConfigurationForm configurationForm = new ConfigurationForm();
            var result = configurationForm.ShowDialog(this);
            if (result == DialogResult.OK)
            {
                var textChooseTargetPath = configurationForm.TextChooseTargetPath; // Eigenschaft auslesen
                
            }

            this.Show();
        }


Dadurch habe ich in der MainForm einige Fehler gemeldet bekommen, da die Textbox jetzt nicht mehr vorhanden ist.
Ich habe txtChooseTargetPath.Text an einigen stellen genutzt.

Zum Beispiel hier:

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
 private void ApplyPathConfig_Click(object sender, EventArgs e)
        {
##########################################################################            
            if (string.IsNullOrEmpty(txtChooseTargetPath.Text)) <--- txtChooseTargetPath.Text ist rot unterstrichen
##########################################################################
            {
                DialogResult result = MessageBox.Show("Ziel-Pfad fehlt?""Confirmation", MessageBoxButtons.YesNo);
                if (result == DialogResult.Yes)
                {
                    SaveLocalPathConfigSettings();
                    SaveNetPathConfigSettings();

                    MessageBox.Show("Konfiguration gespeichert.");
                }
                else if (result == DialogResult.No)
                {
                    return;
                }
            }

            txtChooseTargetPath.Enabled = string.IsNullOrEmpty(txtChooseTargetPath.Text);

            SaveLocalPathConfigSettings();
            SaveNetPathConfigSettings();


Ich dachte da muss jetzt die neue Variable "textChooseTargetPath" rein aber da habe ich mich getäuscht.
Innerhalb der Methode wird diese Variable nicht erkannt, weil ich irgendetwas noch machen muss. Aber was?
Hier bin ich schon wieder aufgeschmissen.

Zitat:
Genau dabei sollte dir auch mein Artikel helfen,

Leider nicht so wie ich gehofft hatte.

Zitat:
Bei den vielen Eigenschaften solltest du evtl.

Also das bedeutet das ich die UIValues.cs noch mit diesen Textboxen, Comboboxen und Checkboxen entweder ergänzen muss oder ich mache eine neue Klasse und nenne sie UIPathValues. Weil diese UI-Elemente sind nur für den Kopierablauf. Ich sollte die UIValues.cs in UISPSValues.cs ändern.

Werden Objekte wie folgt instanziert?? Ein Beispielcode aus dem Internet und von mir ein bisschen angepasst.

Die Klasse mit den UI-Elementen...


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
public class UIPathValues
{
  public string ChooseTargetPath { get; set; }
....
....
....
usw.
  
}


Dann instanzieren in der MainForm?


C#-Quelltext
1:
2:
3:
UIPathValues myUIPathValues = new UIPathValues()
myUIPathValues.Data1 = "test";
myUIPathValues.Data2 ="fall";


In der ConfigurationForm muss man den Konstruktor entsprechend anpassen und setzt eine lokale Variable:


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
public partial class ConfigurationForm : Form
{
  private UIPathValues _uipathvalues = null;

  public ConfigurationForm(UIPathValues uipathvalues) 
  {
    _uipathvalues = uipathvalues;
  }
}


Dann gleich noch eine Sache und ich denke das ist die schwierigste Sache für mich.
Du weißt ja über den Button wird die ConfigurationForm geöffnet und man trägt alles ein.
Bevor natürlich die Form geschlossen wird muss noch der Button "In den Settings speichern" gedrückt werden.
Diese Click-Event von diesem Button ist in der MainForm und diese Event möchte ich natürlich in die neue Form verlagern. Was muss ich alles in der MainForm machen das dieser Button auf der neuen Form genauso funktioniert als ob er noch in der MainForm wäre???

Da habe ich mir aber etwas eingebrockt. Das das so kompliziert wird (für mich) hätte ich nicht gedacht.
Vielen Dank nochmal für deine tolle Unterstützung.

Grüße Tom


Th69 - Mi 08.06.22 20:32

So wie du die neue Klasse UIPathValues sowie den ConfigurationForm-Konstruktor angepaßt hast, genau so meinte ich es. :zustimm:

Jetzt mußt du nur noch in der MainForm davon ein Objekt als privaten Member erstellen:

C#-Quelltext
1:
private UIPathValues myUIPathValues = new UIPathValues();                    

Und wenn jetzt die Configuration-Klasse die Eigenschaften (bzw. Felder) der UIPathValues füllt, dann kannst du in der MainForm z.B mittels myUIPathValues.textChooseTargetPath darauf zugreifen (die lokale Variable var text = ... war nur Beispielcode, da ich ja nicht genau wußte, in welchem Objekt du die Daten speichern möchtest).

Bedenke, daß sich auch dann das ganze Auslesen und Schreiben der Settings ändert und du dann auf die UIPathValues-Werte zugreifen mußt (solange du diese Methoden dazu in der MainForm beläßt).

Macht denn so die Ereignismethode ApplyPathConfig_Click überhaupt noch einen Sinn, denn dies (also die Settings speichern) sollte doch genau dann nach dem ConfigurationForm.ShowDialog()-Aufruf passieren?
Oder sollen die Änderungen mittels des ConfigurationDialog nicht sofort gespeichert werden, sondern mittels eines zusätzlichen Buttons?

Meintest du dies mit deinen letzten Sätzen?

Du solltest dir am besten mal ein Datenfluß-Diagramm skizzieren, damit dir genau klar wird, welche Daten (Klassen-Objekte) du bei den einzelnen Aktionen benötigst.


UserTom - Do 09.06.22 12:36

Servus TH69,

Vielen Dank für deine Antworten.

Ich denke, hier hören wir auf. Ich habe gestern alle UIPathValues auf die neue Form verschoben und hatte ein Desaster.
Auf beiden Formen hatte ich logischerweise sehr viele Fehler, fast 100 jeweils.

Dann ist mir eingefallen, auf einer Internetseite gibt es einen Artikel, ob man wirklich eine SubForm braucht.
Aber das geht auch mit einem Tabcontrol und man kann die Tabpage mit Enabled = false sperren.
Es bleibt alles auf der MainForm.

Zitat:
Bedenke, dass sich auch dann das ganze Auslesen und Schreiben der Settings ändert

Das war einer der Auslöser, warum ich abgebrochen habe. Für mich war das nicht mehr handelbar.

Zitat:
Macht denn so die Ereignismethode ApplyPathConfig_Click überhaupt noch einen Sinn

Nein ich denke nicht, da hätte sich was bessere gefunden.

Zitat:
Du solltest dir am besten mal ein Datenfluß-Diagramm skizzieren

Kenne ich gar nicht, hast du vielleicht ein Beispiel auf Basis C#. Welche Software nimmst du?
Oder bist du noch sehr traditionell und erstellst so etwas mit Papier und Bleistift.

Eine letzte Frage, um Spaghetticode zu vermeiden.
Ich würde gerne alle Steuerelemente auf der Tabpage 2 prüfen, ob sie leer sind oder ob etwas aktiviert ist.

Du weißt ja selber, mit dem folgenden Code könnte ich das auch prüfen, aber ich würde für
jedes "Control/Array" wieder eine Kopie von der Methode erstellen.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
 private bool AnyAppIsActivated()
        {
            if (!cbActivateLocalApps.Any(cb => cb.Checked) && !cbActivateNetApps.Any(cb => cb.Checked))
            {
                return true;
            }

            return false;
        }


Ich denke, das muss nicht sein.
Aber wie frage ich in einer Schleife alle Controls ab, ob sie leer sind oder aktiviert?

So schonmal vorab vielen Dank für die ganze Hilfe, echt super von euch beiden.

Grüße Tom


Th69 - Do 09.06.22 13:26

UserTom hat folgendes geschrieben:
Kenne ich gar nicht, hast du vielleicht ein Beispiel auf Basis C#. Welche Software nimmst du?
Oder bist du noch sehr traditionell und erstellst so etwas mit Papier und Bleistift.

Das bleibt dir überlassen - ich meinte direkt auf Papier (oder mit einem Texteditor am Computer).
Wichtig ist nur zu verstehen, wie Daten herumgereicht werden, insb. wenn es mehr und mehr Daten werden und dann noch zwischen verschiedenen Forms (es sollte möglichst dann immer nur eine Codestelle geben, in welcher die Daten erzeugt und zugewiesen werden, nicht über mehrere Klassen verteilt, sondern stattdessen eben Referenzen herumreichen).
Bei größeren Projekten verwendet man dann sogar Schnittstellen (Interfaces), welche z.B. nur lesenden Zugriff erlauben, um das Schreiben von außerhalb der Klasse zu unterbinden.

Und wegen den vielen Fehlern beim Refaktorieren: das ist meistens normal so (gerade wenn man Codeteile aus einer Klasse entfernt bzw. in eine andere Klasse verschiebt).
Am besten dann größere Codeteile ersteinmal auskommentieren und sich an die Behebung der wichtigsten Fehler machen.

Generell würde man auch, statt alle Elemente auf die Form zu setzen, diese in passende UserControls (Benutzersteuerrelemente) [https://docs.microsoft.com/de-de/dotnet/api/system.windows.forms.usercontrol] aufteilen (so daß man nicht mehr den gesamten UI-Code in einer Klasse hat - ich hatte mal ein Projekt übernommen [für einen spezialisierten Editor], welcher aus über 10 TabPages bestand und der gesamte Code dazu war in der MainForm untergebracht: mehr als 30.000 Zeilen :hair:, so daß ich diesen auch entsprechend in passende UserControls (je TabPage mindestens 1) ausgelagert habe. Mein aktuelles Hobby-Projekt besteht auch nur aus 2 Forms, aber ~50 UserControls.).
Als Einsteig ist auch Arten von benutzerdefinierten Steuerelementen (Windows Forms .NET) [https://docs.microsoft.com/de-de/dotnet/desktop/winforms/controls/custom] lesenswert.

Und wegen den Control-Arrays - du kannst doch das Array (bzw. die Arrays) als Parameter der Methode hinzufügen:

C#-Quelltext
1:
2:
3:
4:
private bool IsAnyChecked(CheckBox[] checkboxes)
{
  return checkboxes.Any(cb => cb.Checked);
}

Wenn du mehrere Arrays verknüpfen willst, dann gibt es einen eleganten Weg mittels des Schlüsselworts params [https://docs.microsoft.com/de-de/dotnet/csharp/language-reference/keywords/params]:

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
private bool IsAnyChecked(params CheckBox[] checkboxes)
{
  foreach (var cbs in checkboxes)
    if (cbs.Any(cb => cb.Checked))
       return true;
  
  return false;
}

(ob die Logik jetzt 100%ig stimmt, habe ich nicht geprüft, da nur hier im Editor geschrieben)
Bei dem Aufruf kannst du dann beliebig viel Arrays übergeben, z.B.

C#-Quelltext
1:
IsAnyChecked(cbActivateLocalApps, cbActivateNetApps);                    

Je nach Abfrage, kann es aber erforderlich sein, verschiedene Hilfsmethoden zu entwickeln, z.B. wenn andere Eigenschaften abgefragt werden sollen. Dabei möglichst immer den allgemeinsten Typ nehmen, z.B. bei Text dann Control (anstatt z.B. nur für TextBox).

Edit: Man kann die Methode auch ganz kurz nur mit LINQ implementieren:

C#-Quelltext
1:
2:
3:
4:
private bool IsAnyChecked(params CheckBox[] checkboxes)
{
  return checkboxes.SelectMany(cbs => cbs).Any(cb => cb.Checked);
}


UserTom - Mi 15.06.22 12:36

Hallo Th69,

Vielen Dank für deine Antwort.
Ich hatte ein paar Tage Urlaub und melde mich deswegen jetzt erst.

Also ich bin mir Sicher das ich keine weiteren Formulare brauche.
Ich habe mir mal andere Professionelle Programme angeschaut die sich auch mit einer SPS Verbinden können. Oder Netzwerkprogramme. Oder das Programm einer unserer Applikationen riesig und hat sehr viel zum konfigurieren. Und alle sind mit dem Prinzip von TabControls aufgebaut öder ähnlichem.

Das mit dem Diagramm sollte ich mir noch anschauen. Ich habe keine Ahnung sowas zu erstellen. Also ich bin mir sicher ich habe noch nicht verstanden

Zitat:
wie Daten herumgereicht werden,

Sonst wüste ich sowas umzusetzen.

Zitat:
diese in passende UserControls (Benutzersteuerrelemente) aufteilen

Das wäre genial wenn ich das könnte, ich denke es erspart viel Code. Aber das wäre wieder die gleiche Situation wie mit den Formularen. Ich müsste alles umschreiben.
So ein User Control würde bei mir aus einer Combobox, einer Textbox, einem Panel und 3 Checkboxen bestehen und das Waagerecht angeordnet.
Klar das wäre echt super wenn ich nur noch 8 User Controls bräuchte. OK 16 weil ich 2 Tab Pages habe.
Aber wie bekomme ich die Daten darein?
Ich habe jeweils für alle 8 Steuerelement Typen (Combobox, Textbox,...) ein Array angelegt.
Und natürlich wieder das schlimmste die komplette Application.Settings würde so nicht mehr funktionieren.

Zitat:
mehr als 30.000 Zeilen

Wahnsinn bei so vielen Zeilen wird man ja irre werden. :nut:. Spitzen Leistung aber mit deinem Knowhow kein Problem.

Zitat:
Wenn du mehrere Arrays verknüpfen willst, dann gibt es einen eleganten Weg mittels des Schlüsselworts params:


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
private bool IsAnyChecked(params CheckBox[] checkboxes)
{
  foreach (var cbs in checkboxes)
    if (cbs.Any(cb => cb.Checked))
       return true;
  
  return false;
}


Bei diesem Code wird bei cbs.Any das Any rot unterstrichen.
Die Fehlermeldung sagt das Checkbox keine Definition für Any hat.

Wie könnte ich eine Abfrage erstellen ob irgendein Steuerelement auf einer TabPage verändert wurde?
Ich habe an so etwas gedacht?

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
foreach(Control c in currentTab.Controls)
{
    if(c is TextBox)
        // check for text change
    if(c is CheckBox)
        //check for check change
    etc...
}

Ich möchte aber nicht CurrentTab nutzen sondern ein TabPage bestimmen können!

Dann möchte ich mit Directory.Exists und einem Timer die Ordner überwachen ob sie existieren.
Mit einem Panel und der Farbe Grün für vorhanden und Rot nicht vorhanden. Ich denke das funktioniert aber ich denke das der neue Timer in einen separaten Thread gehört. Mit welchem Timer könnte ich das in einem separaten Thread laufen lassen?
Darauf hin habe ich den FileSystemWatcher entdeckt. Aber mit dem kann ich keine Ordner überwachen.
Was meint Ihr dazu? Irgendetwas was Sinn macht?

Vielen Dank nochmal für Eure Hilfe.

Grüße Tom

Moderiert von user profile iconTh69: Beitragsformatierung überarbeitet.
Moderiert von user profile iconTh69: C#-Tags hinzugefügt


Th69 - Mi 15.06.22 13:13

Da habe ich noch eine Array-Klammer vergessen (es soll ja ein Array von Arrays übergeben werden):

C#-Quelltext
1:
private static bool IsAnyChecked(params CheckBox[][] checkboxes)                    
(habe ich gerade selber ausprobiert und da noch ein fehlendes static vom Code Analyzer vorgeschlagen wurde, habe ich es auch noch hinzugefügt)

Und bzgl.
UserTom hat folgendes geschrieben:
Wie könnte ich eine Abfrage erstellen ob irgendein Steuerelement auf einer TabPage verändert wurde?
Ich habe an so etwas gedacht?

Das widerspricht der ereignisorientierten Programmierung bei WinForms - benutze daher die TextChanged bzw. CheckedChange o.ä.)-Ereignisse.

Und dann noch zu dem Umbau und den Settings:
Das ist eigentlich kein Problem - erzeuge dafür dann einfach in den UserControls passende Methoden zum Lesen und Schreiben der Settings und rufe diese dann von der Hauptform aus auf. Statt dann Get/SaveLocalControlTexts etc. für jeweils alle Arrays aufzurufen, übergebe die aktuelle Settings-Nummer als Parameter und lese/schreibe dann die einzelnen Werte des UserControls:

C#-Quelltext
1:
2:
3:
4:
5:
6:
public void ReadSettings(Settings settings, int nr)
{
  string name = "Setting" + nr; // <- hier richtigen Settings-Eigenschaftennamen eintragen
  checkBox.Checked = (bool)settings[name];
  // ...
}

(so ähnlich sind ja auch deine bisherigen Get/SaveLocalControlTexts-Methoden implementiert)

Oder du fügst zu der Settings-Klasse diese einzelnen Methoden (je Control-Typ) hinzu und rufst sie dann vom UserControl (bzw. den verschiedenen UserControls) auf.

Wenn du mit meiner Erklärung nicht klarkommst, dann zeige mal deine bisherige "Settings.cs"-Datei (am besten als Anhang).

Edit: Und bzgl. FileSystemWatcher habe ich noch den längeren (englischen) Artikel (+ Code) When files and folders do not exist, FileSystemWatcher cannot monitor file changes? It's fine if you listen recursively [https://blog.actorsfit.com/a?ID=00850-9537f515-307b-4e4d-b146-3b7f2b78e66d] gefunden.
Und beim FileSystemWatcher brauchst du selber keinen eigenen Timer zu benutzen, das macht die Klasse intern (nur wenn du in den Ereignismethoden dann wiederum UI-Zugriffe hast, so müßtest du dann Control.Invoke(...) benutzen).

Wenn du jedoch nur einen (oder mehrere) Verzeichnisse auf Vorhandensein überprüfen willst, dann ist wohl ein System.Timers.Timer [https://docs.microsoft.com/de-de/dotnet/api/system.timers.timer] die beste Wahl.


UserTom - Mi 15.06.22 17:06

Hallo Th69,

Vielen Dank für deine Antwort.

Zitat:
Code Analyzer vorgeschlagen wurde,

Nur wegen der Interesse welche Analyzer benutzt du? Bei mir wurde das mit static nicht vorgeschlagen.

Zitat:
Das widerspricht der ereignisorientierten Programmierung

OK passt.

Zitat:
Und dann noch zu dem Umbau und den Settings:

Ich habe mal um ganz klein anzufangen ein neues WinForm Projekt erstellt.
Darin habe ich ein User Control erstellt. Im Entwurf habe ich dann die Steuerelemente aus dem eigentlichen Projekt kopiert (siehe Bild) und eingefügt.
Das UC ist ja ein komplettes Steuerelement und die einzelnen Steuerelemente da drin sind ja nicht mehr anklickbar, z.B. die Combobox die bekommt ihre Daten von einer Textbox (Multiline).
Wie kann ich jetzt auf die Combobox im User Control zugreifen?
Das ist doch das gleiche Prinzip wie die Kommunikation zwischen 2 Forms?

Das mit dem FileSystemWatcher schaue ich mir mal an.

Die Settings.cs habe ich auch mit angehängt.

Vielen Dank für die ganzen Tipps.

Grüße Tom


Th69 - Mi 15.06.22 18:17

Ich benutze z.Z. VS 2019 und bei einem .NET 5-Projekt einfach den Standard-Code Analyzer (als Hilfe dazu wird mir Übersicht über die Analyse von .NET-Quellcode [https://docs.microsoft.com/de-de/dotnet/fundamentals/code-analysis/overview] angeboten).

Das UserControl ist im Designer-Modus innerhalb einer anderen Form (oder eines anderen UserControls) nicht editierbar, damit man nicht indirekt Änderungen vornehmen kann - zur Laufzeit ist dieses aber vom Anwender aus komplett bedienbar.

Und für die Übergabe von Daten habe ich extra in meinem Artikel Kommunikation von 2 Forms [https://entwickler-ecke.de/topic_Kommunikation+von+2+Forms_117789.html] die Abschnitte "Typische Anfängerfehler / 2. Zugriff auf private Controls einer anderen Form" sowie die Lösung dann mittels "1. Eigenschaften". Man erstellt also eine passende Schnittstelle für dieses UserControl.

Und bei deiner "Settings.cs": vergleiche mal den Code von GetLocalControlTexts und GetNetControlTexts (ebenso bei den anderen Methoden GetLocal/NetCheckBoxes sowie die SaveLocal/Net-Methoden)!

Du brauchst jetzt also nur jeweils zum Lesen und Schreiben je eine Methode für jedes Control (bzw. Control-Eigenschaft) zu erstellen (auch wenn es dann nur je eine Code-Zeile ist).
Probiere das einfach in deinem WinForms-Testprojekt aus.


UserTom - Do 16.06.22 20:18

Hallo Th69,

Vielen Dank für deine Antwort.

Zitat:
Ich benutze z.Z. VS 2019 und bei einem .NET 5-Projekt

Benutze ich auch aber mit Framework 4.8.

Das mit dem User Control muss ich mir in Ruhe anschauen.
Das mit der Schnittstelle habe ich noch nicht begriffen.
Und was auch ein Problem ist das ich nicht weiß ob ich die ganzen Arrays noch brauche für ein User Control.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
public partial class MainForm : Form
    {
        private const string DEFAULTTARGETPATH = @"D:\S7_Export\AppData";
        private S7Client s7Client;
        private ComboBox[] cboLocalApps;
        private TextBox[] tbSourceFilePathLocalApps;
        private CheckBox[] cbActivateLocalApps;
        private CheckBox[] cbLocalSubFolderUses;



C#-Quelltext
1:
2:
 cboLocalApps = new ComboBox[] { cboLocalApp1, cboLocalApp2, cboLocalApp3, cboLocalApp4, cboLocalApp5, cboLocalApp6, cboLocalApp7, cboLocalApp8 };
            tbSourceFilePathLocalApps = new TextBox[] { txtSourceFilePathLocalApp1, txtSourceFilePathLocalApp2, txtSourceFilePathLocalApp3, txtSourceFilePathLocalApp4, txtSourceFilePathLocalApp5, txtSourceFilePathLocalApp6, txtSourceFilePathLocalApp7, txtSourceFilePathLocalApp8 };


Ich denke nicht, weil ja die einzelnen Steuerelemente alle wegfallen. Das alles ist sehr verwirrend.

Ich habe da ein Youtube Video über User Control gefunden, mal sehen ob mir das hilft.

Zitat:
Und bei deiner "Settings.cs": vergleiche mal den Code

Unabhängig vom User Control eine Frage zu meinem aktuellen Projekt.
Ist es möglich das in meiner Settings.cs doppelter Code ist der nicht sein muss?

Z.B.
Zitat:
GetLocalControlTexts und GetNetControlTexts

Der Inhalt dieser Methoden ist identisch. Das heißt ich könnte eine Methode weglöschen
und die andere umbenennen in GetControlTexts. Und mit den anderen Methoden auch oder?


C#-Quelltext
1:
2:
3:
4:
 private void MainForm_Load(object sender, EventArgs e)
{
     GetLocalPathConfigSettings();
     GetNetPathConfigSettings();



C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
public void GetLocalPathConfigSettings()
{
    var settings = Settings.Default;

    txtChooseTargetPath.Text = settings.TargetPath;

###################################################################
    settings.GetControlTexts(cboLocalApps, "LocalApp");
###################################################################

    settings.GetControlTexts(tbSourceFilePathLocalApps, "LocalSourceFilePath");
    settings.GetCheckBoxes(cbActivateLocalApps, "LocalAppAktiv");
    settings.GetCheckBoxes(cbLocalSubFolderUses, "LocalSubFolderUse");
    settings.GetCheckBoxes(cbActivateCode2LocalApps, "ActivateCode2LocalApp");

    txtLocalApplication.Text = settings.LocalApplication;
        }


Vielen Dank für deine Hilfe.

Grüße Tom


Th69 - Do 16.06.22 20:49

Statt der Einzel-Arrays brauchst du jetzt nur noch ein Array für die UserControls.
Am besten sogar die UserControls dynamisch per Code erzeugen, aber du kannst auch, wie bisher, die UserControls direkt per Designer auf der Form platzieren und dann das Array daraus erzeugen.

Und nun benötigst du ja passenden Zugriff, um Daten zwischen der Form und dem UserControl auszutauschen - und dafür brauchst du eben Eigenschaften und/oder Methoden (und dies wird einfach als Schnittstelle bezeichnet).

Und bei den Settings meinte ich genau das von dir Beschriebene. :zustimm:


UserTom - So 19.06.22 23:21

Servus Th69,

Vielen Dank für deine Antwort.

Also das Übungsprojekt hat jetzt eine Form1 erstmal mit 2 UserControls (8 ist maximal)
Und für den Anfang eine Multiline Textbox. (Siehe Foto)
Die Daten aus der Textbox sollen in allen Combobox vom UserControl auswählbar sein.

Du hast gesagt:

Zitat:
Statt der Einzel-Arrays brauchst du jetzt nur noch ein Array für die UserControls.

Meinst du so:


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
namespace WindowsFormsApp11
{
    public partial class Form1 : Form
    {
        private UserControl[] userControls;

        public Form1()
        {
            InitializeComponent();

            userControls = new UserControl[] { userControl1, userControl2 };
        }

       
}
}


Das ist der Code aus meinem eigentlichen Projekt um die Comboboxen von der Multiline Texbox zu füllen:


C#-Quelltext
1:
2:
3:
4:
5:
const int MAXLOCALAPPS = 8;
            for (int n = 0; n < MAXLOCALAPPS; n++)
            {
                cboLocalApps[n].Items.AddRange(txtLocalApplication.Lines);
            }


Kommt der Code in die UserControl.cs:


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
namespace WindowsFormsApp11
{
    public partial class UserControl : System.Windows.Forms.UserControl
    {
        public UserControl()
        {
            InitializeComponent();
        }

        private void UserControl_Load(object sender, EventArgs e)
        {
####################################
Hier an diese Stelle
####################################
        }
    }
}


Zu viele Fragen, weil ich die Zusammenhänge nicht richtig begreife.
Vieles davon ist geraten und das bringt mir nichts.

Das gleiche Thema mit der Textbox die im UserControl ist, die bekommt die Daten vom folderBrowserDialog.

Kannst du mir bitte an Hand der Combobox und der Multiline Textbox zeigen wie das funktioniert.

Vielen Dank nochmal für alles.

Grüße Tom


Th69 - Mo 20.06.22 07:13

Ja, so meinte ich das Array für die UserControls.

Und schreibe dir eine Methode (oder Eigenschaft) in der UserControl-Klasse, um die ComboBox zu füllen:

C#-Quelltext
1:
2:
3:
4:
5:
public FillValues(string[] lines) // oder noch eleganter: IEnumerable<string> (so daß es für alle möglichen String-Collections funktioniert)
{
  // evtl. noch: comboBox.Items.Clear();
  comboBox.Items.AddRange(lines);
}

Und rufe sie dann von der Hauptform mittels userControl.FillValue(textBox.Lines) auf (bzw. in einer Schleife über alle UserControls).

Und den FolderBrowserDialog kannst du auch auf das UserControl packen und dort direkt die Methode zum Aufruf und Setzen der Pfade übernehmen.
Wenn du jedoch nur einen FolderBrowserDialog für die gesamte Anwendung haben möchtest (damit du nur einmalig die Eigenschaften dafür setzen mußt und diese dann zur Laufzeit immer gleich sind), dann übergebe nur den Pfad an das UserControl.
Wie in meinem Artikel sehen dann die Eigenschaften so aus:

C#-Quelltext
1:
2:
3:
4:
5:
public string Description
{
    get { return labelDescription.Text; }
    set { labelDescription.Text = value; }
}

(entsprechend Name, Control und Control-Eigenschaft anpassen).
Ist zwar ein bißchen mehr Source-Code als direkt die Controls zu benutzen, aber du hast eine klare Schnittstelle erzeugt.


UserTom - Di 21.06.22 17:55

Hallo Th69,

Vielen Dank für deine Antwort.
Da bin ich ja froh das ich wenigstens etwas richtig gemacht habe.

Du sagtest mal das der Code schon ganz ordentlich sei.

Wenn ich dieses Tool über die exe-Datei starte und auf dem PC mit der echten SPS verbinde dann funktioniert das Tool trotz Backgroundworker doch noch nicht.

Hier zu Hause nutze ich einen Simulator und der ist natürlich min. 50 mal schneller als die echte SPS.
Mit dem SPS Simulator funktioniert die Software so wie sie soll.

In Verbindung mit der echten SPS passiert nach dem Trigger Signal gar nichts.
Nach 1-2 Minuten taucht im Label "lblAllFilesCopied" 0% auf.


C#-Quelltext
1:
2:
3:
4:
5:
private void BW_CopyAllFiles_ProgressChanged(object sender, ProgressChangedEventArgs e)
{            
   progressBar1.Value = e.ProgressPercentage;
   lblAllFilesCopied.Text = "Processing......" + progressBar1.Value.ToString() + "%";
}


Ich habe in einem SPS Forum mal nachgefragt woran das liegen könnte.
Es scheint an der S7 400 CPU zu liegen, das die Leseanfragen vom Tool während des kopieren in der Netzwerkkommunikation, auch mitten im OB1 Zyklus eingebunden werden.

Das wurde bedeuten das das lesen vom: S7VariablenResult s7VariablenResult = ReadResult(); zu lange dauert. Vielleicht wegen der Progress Bar Schleife?

Das heißt der Backgroundworker verhindert nur das blockieren vom Tool.

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
        private void BW_CopyAllFiles_DoWork(object sender, DoWorkEventArgs e)
        {
            var bw = sender as BackgroundWorker;

            S7VariablenResult s7VariablenResult = ReadResult();

            //Ordner erstellen
            string createTargetPath = Path.Combine(DEFAULTTARGETPATH, s7VariablenResult.FolderName);
            Directory.CreateDirectory(createTargetPath);

            for (int l = 0; l < 100; l++)
            {
                if (bw.CancellationPending)
                {
                    // Set the e.Cancel flag so that the WorkerCompleted event
                    // knows that the process was cancelled.
                    e.Cancel = true;
                    bw.ReportProgress(0);
                    return;
                }
                else
                {
                    for (int i = 0; i < cbActivateLocalApps.Length; i++)
                    {
                        if (cbActivateLocalApps[i].Checked == true)
                        {
                            string praegeCodeNrFolderName = s7VariablenResult.FolderName;
                            string praegeCodeNrFileName = s7VariablenResult.PraegeCodeNr;
                            string sourceFilePath = uivalues.SourceFilePathLocalApps[i];
                            string targetPath = DEFAULTTARGETPATH;

                            MoveFilesfromLocal(praegeCodeNrFolderName, praegeCodeNrFileName, sourceFilePath, targetPath);

Was könnte man tun um das zu optimieren?
Gibt es hier vielleicht jemand der schon mal mit C# und SPS gearbeitet hat und kennt das Problem?

Grüße Tom

Moderiert von user profile iconTh69: Beitragsformatierung überarbeitet.


Th69 - Di 21.06.22 18:44

Ja, die aktuelle Version deines Codes halte ich für gut.

Du meinst, der s7Client.DBRead-Aufruf dauert 1-2 Minuten?
Wenn es keine asynchrone Lesemethode gibt, dann kannst du nur die Anzeige verbessern, d.h. z.B. vor dem ReadResult-Aufruf

C#-Quelltext
1:
bw.ReportProgress(-1);                    

aufrufen und in der zugehörigen Ereignismethode dann bei 0 einen anderen Text anzeigen ("Verbindung zur SPS wird hergestellt" bzw. "Lese Daten von der SPS" o.ä.).

Solange du keinen Aufruf von ReadResult() außerhalb des BackgroundWorkers hast, sollte die UI dann auch nicht blockieren.

Edit: Falls (laut Doku) kein negativer Wert (sondern nur 0-100 bzw. im deutschen steht 1-100) erlaubt sind, dann verwende 0 als Parameter dafür. Oder du zeigst den Label.Text vor dem Aufruf des BackgroundWorkers an und erst beim Kopieren wird mit ReportProgress(...) der Kopierfortschritt angezeigt.

PS: Wie groß ist denn DBSize beim s7Client.DBRead-Aufruf?


UserTom - Di 21.06.22 21:22

Servus,

Zitat:
Du meinst, der s7Client.DBRead-Aufruf dauert 1-2 Minuten?


Ich meine:
Mit dem SPS Simulator ist, wenn das Trigger Signal kommt, innerhalb von 5s alles kopiert
und die GUI hat auch alle angezeigt.

Mit der echten CPU Verbindung ist die GUI erstmal nicht das Problem.

Meine Vermutung ist das = ReadResult() zu lange brauch von der SPS die Variablen zu lesen.



C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
 private void BW_CopyAllFiles_DoWork(object sender, DoWorkEventArgs e)
{
     var bw = sender as BackgroundWorker;
###############################################################
     S7VariablenResult s7VariablenResult = ReadResult();
##################################################################
     //Ordner erstellen
     string createTargetPath = Path.Combine(DEFAULTTARGETPATH, s7VariablenResult.FolderName);
     Directory.CreateDirectory(createTargetPath);


Bei meinem ersten Versuch mit der echten SPS kam es noch nicht mal dazu das ein Ordner erstellt wurde.
Dann irgendwann 1-2 Minuten später taucht die 0% im Label auf. Da das ja Nutzlos ist, habe ich abgebrochen.
Ich habe eine Vermutung. ReadResult liest den ganzen Datenbaustein ein. Buffersize ist 64.
Das ist auch die Größe von dem DB. Über Return s7VariablenResult werden alle Werte übergeben.
Ich denke das das viel zu lange dauert. Außerdem sind auch viele andere Werte mit drin die
hierfür nicht gebraucht werden. Ich muss den DB umschreiben passend nur für dieses Tool.
Außerdem wird dieses "new S7VariablenResult()" mit dran schuld sein.
Immer ein neues Objekt ist bestimmt nicht hilfreich. Hier kommt deine SPS.cs vielleicht in Frage die ich aber nicht integriert bekomme.
Mir würde es reichen das die ReadResult so umprogrammiert wird das wir auf das new verzichten können und auch den
return nicht mehr brauchen. Wenn es möglich ist ohne SPS.cs. Aber nur falls du auch der Meinung bist das
das sinnvoll ist und das auslesen der Variablen verschnellert.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
 private S7VariablenResult ReadResult()
{
     S7VariablenResult s7VariablenResult = new S7VariablenResult();
     // Reads the buffer.

     byte[] dbbuffersize = new byte[uivalues.DBSize];

     s7Client.DBRead(uivalues.DBNr, uivalues.DBOffset, uivalues.DBSize, dbbuffersize);

     s7VariablenResult.PraegeCodeNr = S7.GetStringAt(dbbuffersize, uivalues.PraegeCodeDBPos);
     s7VariablenResult.FolderName = S7.GetStringAt(dbbuffersize, uivalues.FolderNameDBPos);
     s7VariablenResult.PraegeCode2Nr = S7.GetStringAt(dbbuffersize, uivalues.PraegeCode2DBPos);
     s7VariablenResult.TrigSaveFiles = S7.GetBitAt(dbbuffersize, uivalues.TrigSaveFilesByte, uivalues.TrigSaveFilesBit);
     s7VariablenResult.TypLeft = S7.GetBitAt(dbbuffersize, uivalues.TypLeftByte, uivalues.TypLeftBit);
     s7VariablenResult.TypRight = S7.GetBitAt(dbbuffersize, uivalues.TypRightByte, uivalues.TypRightBit);
     s7VariablenResult.RobotIsEmpty = S7.GetBitAt(dbbuffersize, uivalues.RobotIsEmptyByte, uivalues.RobotIsEmptyBit);
     s7VariablenResult.FilesAreCopied = S7.GetBitAt(dbbuffersize, uivalues.FilesAreCopiedByte, uivalues.FilesAreCopiedBit);
     s7VariablenResult.ConnectedToPLC = S7.GetBitAt(dbbuffersize, uivalues.ConnectedToPLCByte, uivalues.ConnectedToPLCBit);

     return s7VariablenResult;
}


Ich hoffe du verstehst das alles was ich da geschrieben habe.
Ich werde mal den Datenbaustein überdenken.

Vielen Dank für dein Hilfe.

Grüße Tom


Th69 - Mi 22.06.22 08:24

UserTom hat folgendes geschrieben:
Meine Vermutung ist das = ReadResult() zu lange brauch von der SPS die Variablen zu lesen.

Und die einzige Methode dadrin, die das verursachen kann ist der s7Client.DBRead-Aufruf (alles andere ist erstmal zweitrangig).
Trotzdem sind 1-2 Minuten für 64 Bytes viel zu lang, da muß noch etwas anderes daran Schuld sein (falsche Verbindung o.ä.).

Hast du denn ein anderes Testprogramm? Ansonsten schau mal in Snap7 [http://snap7.sourceforge.net/], dort gibt es laut Demo Images [http://snap7.sourceforge.net/sharp7.html#images] ein C#-Testprogramm, das im Download Sharp7 [https://sourceforge.net/projects/snap7/files/Sharp7/] enthalten sein sollte (bzw. die zu kompilierende Solution). Dort dann sharp7-full-1.0.0.7z [https://sourceforge.net/projects/snap7/files/Sharp7/sharp7-full-1.0.0.7z/download] auswählen, nicht den grünen Download-Button (der nur eine VB6 Datei herunterlädt).

PS: Du solltest die Variable dbbuffersize in dbbuffer umbenennen.


UserTom - Mi 22.06.22 09:58

Hallo Th69,

Vielen Dank für deine Antwort.

Zitat:
Und die einzige Methode dadrin, die das verursachen kann ist der s7Client.DBRead-Aufruf

Ja und das wiederrum steht alles in der Schnittstellen Datei Sharp7.cs
Irgendwo sitzt der Fehler nur wo genau.

Zitat:
Dort dann sharp7-full-1.0.0.7z auswählen

Das kenne ich schon. Da ist mir was aufgefallen, dass der Wert für Buffer sehr hoch gesetzt wird.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
namespace CSClient
{

    public partial class MainForm : Form
    {
        private S7Client Client;
        private byte[] Buffer = new byte[65536];


Ich werde das mal mit dem hohen Wert ausprobieren.
Im Bild DBRead sieht man die Werte vom DBRead im Simulator gibt es keine Probleme wenn der Wert so hoch ist.
Im angehängten Bild "Buffer_Beispiel" sieht man aber, dass der Buffer die DB-Größe ist.
Im Bild Buffer_Beispiel gibt es einen Hinweis. http://snap7.sourceforge.net/sharp7.html

Zitat:
Every time a component is tested this struct is filled by the PLC and we want to read it into a .NET struct, picking the fields and adjusting their byte-order (the PLC is Big-Endian, the PC is Little-Endian, so the bytes must be reversed).

Weißt du, wo ich die Bytes vertauschen müsste, wenn überhaupt?

Zitat:
PS: Du solltest die Variable dbbuffersize in dbbuffer umbenennen.

Habe ich.

Vielen Dank für deine Hilfe.

Grüße Tom


Th69 - Mi 22.06.22 14:09

UserTom hat folgendes geschrieben:
Weißt du, wo ich die Bytes vertauschen müsste, wenn überhaupt?

Das machen wohl die Methoden in der "Sharp7.cs", z.B. für GetIntAt (die du aber nicht nutzt).

Funktioniert denn das Testprogramm bei der realen SPS?

Ansonsten schreibe einfach selber ein Testprogramm (ohne BackgroudnWorker etc), daß einfach nur einen Connect durchführt und danach dann die Daten liest (und messe z.B. mit Stopwatch [https://docs.microsoft.com/de-de/dotnet/api/system.diagnostics.stopwatch] die Zeiten) - kann ja auch ein einfaches Konsolenprogramm sein.


UserTom - Mi 22.06.22 17:25

Servus,

Zitat:
Funktioniert denn das Testprogramm bei der realen SPS?


Ich habe mit dem echten Programm getestet. Es funktioniert das Ordner erstellen und auch das kopieren.
Die GUI friert nicht ein. Die Progressbar Anzeige passt nicht zu meinem kopieren.
Das kopieren dauert für die Größe der Dateien sehr lange. 29 Dateien und 4 Ordner. 1,18MB.

Die Progressbar Anzeige habe ich wie folgt umgebaut. Das hatte auch Ralf in einem Post so ähnlich dargestellt.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48:
49:
50:
51:
52:
53:
54:
55:
56:
57:
58:
59:
60:
61:
private void BW_CopyAllFiles_DoWork(object sender, DoWorkEventArgs e)
        {
            var bw = sender as BackgroundWorker;

            bw.ReportProgress(0);

            S7VariablenResult s7VariablenResult = ReadResult();

            //Ordner erstellen
            string createTargetPath = Path.Combine(DEFAULTTARGETPATH, s7VariablenResult.FolderName);
            Directory.CreateDirectory(createTargetPath);

            bw.ReportProgress(25);


            for (int i = 0; i < cbActivateLocalApps.Length; i++)
            {
                if (cbActivateLocalApps[i].Checked == true)
                {
                    string praegeCodeNrFolderName = s7VariablenResult.FolderName;
                    string praegeCodeNrFileName = s7VariablenResult.PraegeCodeNr;
                    string sourceFilePath = uivalues.SourceFilePathLocalApps[i];
                    string targetPath = DEFAULTTARGETPATH;

                    MoveFilesfromLocal(praegeCodeNrFolderName, praegeCodeNrFileName, sourceFilePath, targetPath);
                }
            }

            bw.ReportProgress(50);

            for (int j = 0; j < cbActivateLocalApps.Length; j++)
            {
                if (cbActivateLocalApps[j].Checked == true)
                {
                    string praegeCodeNrFolderName = s7VariablenResult.FolderName;
                    string praegeCode2NrFileName = s7VariablenResult.PraegeCode2Nr;
                    string sourceFilePath = uivalues.SourceFilePathLocalApps[j];
                    string targetPath = DEFAULTTARGETPATH;

                    MoveFilesCode2fromLocal(praegeCodeNrFolderName, praegeCode2NrFileName, sourceFilePath, targetPath);
                }
            }

            bw.ReportProgress(75);

            for (int k = 0; k < cbActivateNetApps.Length; k++)
            {
                if (cbActivateNetApps[k].Checked == true)
                {
                    string praegeCodeNrFolderName = s7VariablenResult.FolderName;
                    string praegeCodeNrFileName = s7VariablenResult.PraegeCodeNr;
                    string sourceFilePath = uivalues.SourceFilePathNetApps[k];
                    string targetPath = DEFAULTTARGETPATH;

                    CopyFilesfromNetwork(praegeCodeNrFolderName, praegeCodeNrFileName, sourceFilePath, targetPath);
                }
            }

            //Report 100% completion on operation completed
            bw.ReportProgress(100);
        }


So sieht die Prozessbar Anzeige besser aus und passt zum Ablauf. Bei 75% stockt es, weil da die Dateien vom Netzwerk kopiert werden.
Und das wundert mich, weil auf 3 von den 5 PCs nur jeweils eine 2kB csv-Datei liegt, auf dem 4. PC liegen 2 Dateien zusammen mit 300kB und auf dem 5. PC liegt ein Ordner mit 3 Dateien mit 647kB.
Ich vermute das die Rechte nicht so schnell freigegeben werden. Sie sind da und sie werden kopiert.
Firewall ist überall ausgeschaltet. Es sind autarke Anlagen die keine Verbindung nach draußen haben.
Mal eine Frage, ich habe nicht den gleichen Benutzer auf den PCs angelegt wie auf dem Quell-PC.
Das sind Applikations-PCs und die haben Ihre Benutzer die von den Firmen eingerichtet wurden.
Würde das was bringen, wenn auf den PCs der gleiche Benutzer wäre? Es sind alles Windows 10 PCs.
Ich kann bei den Firmen anfragen, ob ich das ändern darf. Es könnte sich auf die Applikationssoftware auswirken.

So ich werde weiter an dem DB arbeiten.


C#-Quelltext
1:
 s7VariablenResult.PraegeCodeNr = S7.GetStringAt(dbbuffer, uivalues.PraegeCodeDBPos);                    


S7.GetStringAt möchte ich austauschen gegen Char wie nach der Vorlage


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
private ComponentResult LeakResult()

{    ComponentResult Result = new ComponentResult();

    byte[] Buffer = new byte[26];

    // Reads the buffer.

    Client.DBRead(100026, Buffer);

    // Extracts the fields and inserts them into the struct

    Result.SerialNumber = S7.GetCharsAt(Buffer, 012);


Alles was nicht gebraucht wird muss raus.

Also vielen Dank nochmal für deine Antwort und Hilfe.

Grüße Tom


Ralf Jansen - Mi 22.06.22 19:21

Zitat:
Mal eine Frage, ich habe nicht den gleichen Benutzer auf den PCs angelegt wie auf dem Quell-PC.
Das sind Applikations-PCs und die haben Ihre Benutzer die von den Firmen eingerichtet wurden.
Würde das was bringen, wenn auf den PCs der gleiche Benutzer wäre?

Eher nein. Aber mußt du dich den im Moment irgendwo in deinem Programm für die Freigabe authentifizieren?

Zitat:
S7.GetStringAt möchte ich austauschen gegen Char wie nach der Vorlage

Ob char oder string glaube ich auch nicht das das einen Unterschied macht. Das lesen der Daten von der Quelle macht doch DBRead oder? Der potentiel teure IO Zugriff passiert also schon da.
Die S7 Klasse holt nur aus dem Byte Array die passenden Bytes und macht einen anderen Datentyp draus aus (Hier String oder Char Array)?

Was genau Zeit kostet solltest du aber messen und nicht raten.


UserTom - Do 23.06.22 07:22

Hallo Ralf,

Vielen Dank für deine Antwort.

Zitat:
Aber mußt du dich den im Moment irgendwo in deinem Programm für die Freigabe authentifizieren?

Das nicht, aber dann ist es noch unverständlicher, warum das Programm dafür so lange braucht.
Ich habe das Programm mal die Nachtschicht laufen lassen. Es hat nur jedes 2. Bauteil geschafft, die Daten zu kopieren.
Das Programm bleibt an dieser Stelle für 40sek. hängen.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
bw.ReportProgress(75);

            for (int k = 0; k < cbActivateNetApps.Length; k++)
            {
                if (cbActivateNetApps[k].Checked == true)
                {
                    string praegeCodeNrFolderName = s7VariablenResult.FolderName;
                    string praegeCodeNrFileName = s7VariablenResult.PraegeCodeNr;
                    string sourceFilePath = uivalues.SourceFilePathNetApps[k];
                    string targetPath = DEFAULTTARGETPATH;

                    CopyFilesfromNetwork(praegeCodeNrFolderName, praegeCodeNrFileName, sourceFilePath, targetPath);
                }
            }


Das ist natürlich viel zu lang. Die Frage ist, warum er da hängen bleibt.
Vorher haben wir das von der Visualisierung Zenon (Copa Data) in der Runtime mit VBA erledigen lassen.
Alle Dateien wurden in max. 8 sek. kopiert. Der Nachteil war, dass die Runtime dann für diese Zeit blockiert war.
Wenn man in diesem Moment eine Störung hat, dann zählt jede Sekunde. Taktzeit bestimmt die Anzahl der produzierten Teile.

Das neue Tool sollte dieses Handicap mit der Runtime beheben. Das war halt eine Empfehlung von der Firma Copa Data.

Gibt es einen großen Unterschied zwischen C#:

C#-Quelltext
1:
using Microsoft.VisualBasic.FileIO;                    


und VBA 7.1:


Quelltext
1:
2:
'Windows Dateioperationen zur Verfügung stellen
Set oFSO = CreateObject("Scripting.FileSystemObject") '


Ich bin sehr enttäuscht, dass meine ganze investierte Zeit wahrscheinlich für die Katz war.
Ich greife natürlich nach jedem Strohhalm.
Gibt es bei C# Befehle zum Kopieren oder verschieben, die nicht auf VisualBasic beruhen?
Wenn, der alte VBA Code das in kurzer Zeit schafft, dann müsste doch das neue Tool, das doch auch schaffen können.
Die Quellen und die Ziele sind in beiden Programmierungen gleich.

Habt ihr vielleicht noch eine Idee, wie das Verbessern könnte?

Grüße Tom


Th69 - Do 23.06.22 09:17

Hallo,

bei dir lokal läuft das doch schnell genug, d.h. es wird an der Umgebung auf dem Produktionsrechner liegen.
Sowohl die VB-Dateioperationen in Microsoft.VisualBasic.FileIO.FileSystem als auch die in der Klasse System.IO.File beruhen alle auf den WinAPI-Dateiverwaltungsfunktionen [https://docs.microsoft.com/de-de/windows/win32/fileio/file-management-functions] (die auch sonst von allen Windows-Programmen, wie z.B. Windows Explorer intern benutzt werden).

Es gibt ja in deinem Programm eine Unterscheidung zwischen "Local" und "Net" - worauf bezieht sich das genau?
Und wenn du manuell (oder per Batch-Script o.ä.) diese Dateien kopierst, ist das dann schnell genug?

Hast du auch schon mal in den Task-Manager (bzw. den Ressourcenmonitor) geschaut, wie hoch die generelle Auslastung auf dem Rechner ist?

Frühzeitige (bzw. regelmäßige) Integrationstests sowie Ausführung auf dem Zielsystem sind ein wesentlicher Bestandteil moderner (agiler) Entwicklung.


UserTom - Do 23.06.22 17:52

Hallo Th69,

Zitat:
bei dir lokal läuft das doch schnell genug, d.h. es wird an der Umgebung auf dem Produktionsrechner liegen.
Ja genau und ich denke ich habe da was gefunden. Zum Beispiel bei dem PC wo das Programm bei 75% stehen bleibt und es so lange dauert da habe ich mich gefragt warum. Ich habe mir dann mal den PC genauer angeschaut und in dem Quellordner fast 30000 Ordner gefunden. Ich habe dann mal unsere IT gefragt ob es möglich ist das das Programm deswegen so lange braucht. Die meinten ja, weil es so viele Ordner prüfen muss bis es den richtigen Ordner gefunden hat.

Zitat:
Es gibt ja in deinem Programm eine Unterscheidung zwischen "Local" und "Net" - worauf bezieht sich das genau?
Es gibt ein TabControl wo die ganzen Quelle Pfade eingetragen werden.

Eine TabPage ist für Lokale Pfade:

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
        private void ChooseSourcePathLocalApp_Click(object sender, EventArgs e)
        {
            if (sender is TextBox textBox)
            {
                if (folderBrowserDialog1.ShowDialog() == DialogResult.OK)
                {
                    textBox.Text = folderBrowserDialog1.SelectedPath;
                }
            }
        }


die andere TabPage für UNC Pfade:

C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
 
        private void ChooseSourcePathNetApp_Click(object sender, EventArgs e)
        {
            if (sender is TextBox textBox)
            {
                textBox.Text = GetNetworkFolders(new FolderBrowserDialog());
            }
        }

        private string GetNetworkFolders(FolderBrowserDialog oFolderBrowserDialog)
        {
            Type type = oFolderBrowserDialog.GetType();
            FieldInfo fieldInfo = type.GetField("rootFolder", BindingFlags.NonPublic | BindingFlags.Instance);
            fieldInfo.SetValue(oFolderBrowserDialog, 18);
            if (oFolderBrowserDialog.ShowDialog() == DialogResult.OK)
            {
                return oFolderBrowserDialog.SelectedPath.ToString();
            }
            else
            {
                return "";
            }
        }

Das Auswählen mache ich über FolderBrowserDialog. Wenn man nach UNC Pfade schaut dann dauert es ein bisschen aber ne praktische Sache.

Zitat:
Hast du auch schon mal in den Task-Manager

Bis jetzt noch nicht da ich heute mich um andere Dinge kümmern musste.
Aber das werde ich dann mal machen und gleichzeitig werde ich mal testen wie es sich verhält wenn im Quellordner nur die Datei vorhanden ist die aktuell benötigt wird.

So vielen Dank für deine Unterstützung.

Grüße Tom

Moderiert von user profile iconTh69: Beitragsformatierung überarbeitet.


Th69 - Fr 24.06.22 09:21

30000 Unterordner - das ist ja genauso schlimm wie bei meinem ehemaligen Projekt die ca. 30000 Code-Zeilen!

Wie kannst du denn da über den FolderBrowserDialog den passenden Ordner vernünftig auswählen?

Ich hatte auch deswegen nach "Local" und "Net" gefragt, ob der Zugriff darauf einen Unterschied macht, aber so wie ich deinen Beitrag verstehe ist es schon bei "Local", eben wegen der vielen Unterordner, langsam.

Benötigst du denn nur Zugriff auf einige wenige dieser Unterordner? Dann könntest du evtl. einen eigenen Laufwerksbuchstaben dafür anlegen und darüber dann zugreifen, s.a. Windows 10: Lokale Ordner als Laufwerk in Windows einbinden [https://www.tecchannel.de/a/lokale-ordner-als-laufwerk-in-windows-einbinden,3277952] (s. Bilder-Galerie).


UserTom - Fr 24.06.22 12:38

Hallo Th69,


Vielen Dank für deine Antwort. Das hast du jetzt leider falsch verstanden.
Im Anhang siehst du ein Screenshot vom Dialog wo die Quellpfade ausgewählt werden.
Zum Beispiel \\computer\PDE\kle15r02\AppData. In dem AppData Ordner sind die 30000 Unterordner.
Ich brauche keinen Unterordner auswählen.

Der Programmcode für "CopyDirectory" sucht die 30000 Unterordner nach dem richtigen Ordnernamen ab der kopiert werden soll.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
public static void CopyDirectoryfromNetwork(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            IEnumerable<string> dirs = Directory.EnumerateDirectories(sourceFilePath, "*", System.IO.SearchOption.AllDirectories).Where(x => x.Contains(praegeCodeNrFileName));

            foreach (string dir in dirs)
            {
                TryAction(() => FileSystem.CopyDirectory(dir, dir.Replace(sourceFilePath, Path.Combine(targetPath, praegeCodeNrFolderName)), true));
                TryAction(() => FileSystem.DeleteDirectory(dir, DeleteDirectoryOption.DeleteAllContents));
            }
        }


Und das dauert so lange oder? Mein Fehler ist das da so viele Unterordner drin sind.

Ich hatte ja erzählt das die Visu Zenon mit VBA das kopieren macht.
Im VBA Code gibt es aber keine lösch befehle. So summieren sich die Ordner und Dateien.
Daran habe ich nicht mehr gedacht. Und in meinem Programm sind Lösch Befehle mit drin.
Aber weil die GUI blockierte ist mein Programm bis heute noch nicht zum Einsatz gekommen.
Mit Eurer Hilfe ist der Backgroundworker reingekommen und jetzt sieht es besser aus.

Bei dem ersten Test hat es nicht so gut funktioniert, weil alle Quell-Ordner auf den 5 PCs
voll sind mit Unterordner oder Dateien. Ich habe zum Beispiel auf den einem PC über 200000
Dateien gefunden. Das Programm brauch eine "Ewigkeit" bis es die richtige Datei gefunden hat.

Oder siehst du das anders?

Noch eine andere Frage.

Mit dem folgenden Code über FolderBrowsingdialog wähle ich die Netzwerkordner aus.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
private void ChooseSourcePathNetApp_Click(object sender, EventArgs e)
        {
            if (sender is TextBox textBox)
            {
                textBox.Text = GetNetworkFolders(new FolderBrowserDialog());
            }
        }

        private string GetNetworkFolders(FolderBrowserDialog oFolderBrowserDialog)
        {
            Type type = oFolderBrowserDialog.GetType();
            FieldInfo fieldInfo = type.GetField("rootFolder", BindingFlags.NonPublic | BindingFlags.Instance);
            fieldInfo.SetValue(oFolderBrowserDialog, 18);
            if (oFolderBrowserDialog.ShowDialog() == DialogResult.OK)
            {
                return oFolderBrowserDialog.SelectedPath.ToString();
            }
            else
            {
                return "";
            }
        }


Der Dialog brauch recht lange das Netzwerk einzulesen ca.30 sek.
Gibt es eine bessere Variante?


Vielen Dank für deine Ideen.

Grüße Tom


Th69 - Fr 24.06.22 13:29

Das ist ja noch schlimmer!

Bedenke mal deinen Code dafür:

C#-Quelltext
1:
Directory.EnumerateDirectories(sourceFilePath, "*", System.IO.SearchOption.AllDirectories)                    

Damit werden alle Unterordner (+ Unter-Unterordner etc.) durchlaufen - und du wunderst dich, daß das lange dauert?

Benötigst du wirklich SearchOption.AllDirectories [https://docs.microsoft.com/de-de/dotnet/api/system.io.searchoption] (s.a. Anmerkungen dazu im Snippet-Link unten)?

Und wäre ein searchPattern wie

C#-Quelltext
1:
"*" + praegeCodeNrFileName + "*"                    

nicht viel eleganter (anstatt für jeden Eintrag Where(x => x.Contains(praegeCodeNrFileName) aufzurufen)?!!!

Und kannst du die Datei- und Ordnernamen nicht noch weiter einschränken, d.h. kann der praegeCodeNrFileName irgendwo im Dateinamen auftauchen oder evtl. nur am Anfang?

PS: Warum überhaupt praegeCodeNrFileName für die Suche nach Ordnern? Du hast doch bei den Kopiermethoden auch noch praegeCodeNrFolderName als Parameter?
Bis jetzt hatte ich mir die Logik deiner Kopiermethoden nicht näher angeschaut gehabt, aber mir kommt es jetzt so vor, als hättest du einfach ein bißchen zu viel Copy&Paste da betrieben, anstatt genau zu überlegen.
Wußtest du denn nicht von Anfang an, daß dort soviele Unterordner vorhanden sind? Das vorherige Programm mußte doch ebenso damit umgehen.

Kannst du auch mal ein paar Beispiele für praegeCodeNrFolderName und praegeCodeNrFileName nennen?

Edit:
Ich habe mir jetzt noch mal deine Methoden MoveFilesfromLocal sowie CopyFilesfromNetwork angesehen.
Kann es sein, daß im Fall Local/NetSubFolderUses[i].Checked zwar alle Unterordner durchsucht werden sollen, aber nur vom praegeCodeNrFolderName (und nicht über alle sourceFilePath)?! Oder ist das nur der Name des Zielordners?

PPS: Hier noch Link, um die Dateisuche selber (d.h. mit eigenen Anforderungen daran) zu implementieren: [Snippet] Verzeichnisse und Dateien rekursiv durchlaufen [https://mycsharp.de/forum/threads/58665/snippet-verzeichnisse-und-dateien-rekursiv-durchlaufen]


UserTom - Fr 24.06.22 18:29

Hallo Th69,

Das hat sehr lange gedauert bis ich mir das zusammen gebastelt hatte das alles so kopiert wird wie ich es benötige. Wenn das natürlich schlecht ist und deswegen alles so lange dauert wäre ich dir sehr dankbar wenn du mir helfen könntest das zu verbessern.

Zitat:
Benötigst du wirklich SearchOption.AllDirectories

Wahrscheinlich nicht aber so funktioniert es.

Also Zielpfad weißt du ja schon. D:\S7_Export\AppData\ + der Ordner der jedesmal erstellt wird.
So ein Ordnername sieht wie folgt aus: 22062415443R oder L

Jahr 22 / Monat 06 / Tag 24 / St. 15 / Min. 47 / Sek. 3 (nur die 1. Stelle) / Buchstabe L oder R


C#-Quelltext
1:
2:
3:
//Ordner erstellen
string createTargetPath = Path.Combine(DEFAULTTARGETPATH, s7VariablenResult.FolderName);
Directory.CreateDirectory(createTargetPath);

praegeCodeNrFileName -> 22062415473
praegeCodeNrFolderName -> 22062415473R oder L
praegeCode2NrFileName -> 22062414432 (separates Bauteil was vorher produziert wird)

Diese Werte werden von der SPS vorgegeben, da habe ich keinen Einfluss drauf.

So der
1. Lokale Sourcepath ist D:\S7_Export\Bosch - Die Datei sieht so aus: Ch_22062415473.txt

2. D:\S7_Export\Sensopart\x015id02xa1
Die möglichen Dateien:
22062415473_x015id02xa1_Job1_Pass.jpg (bis Job6)
oder
22062415473_x015id02xa1_Job1_Fail.jpg (bis Job6)
Pass und Fail können beide zusammen auftauchen. Es sollte natürlich nur Pass sein.

3. D:\S7_Export\Sensopart\x020id01xa1
22062414432_x020id02xa1_Job1_Pass.jpg (bis Job3) und das gleiche mit Fail.
Die werden hiermit verschoben "public static void MoveFilesCode2fromLocal()" Wegen 2. Code Nr. 22062414432

4. D:\S7_Export\Sensopart\x060id02xa1
22062415473_x060id02xa1_Job1_Pass.jpg
22062415473_x060id02xa1_Job3_Pass.jpg
22062415473_x060id02xa1_Job5_Pass.jpg
oder
22062415473_x060id02xa1_Job2_Pass.jpg
22062415473_x060id02xa1_Job4_Pass.jpg
22062415473_x060id02xa1_Job6_Pass.jpg und / oder das gleiche mit Fail.

5. D:\S7_Export\Tox - TOX_015R01_22062415473_20220624_154734 <-- Ordnerbezeichnung
Hier wird der Ordner kopiert mit Unterordner.
Wahrscheinlich wegen dieser Ordnerbezeichnung habe ich nicht mit * gearbeitet.


C#-Quelltext
1:
x.Contains(praegeCodeNrFileName)                    

Ist doch praktisch. Aber wahrscheinlich schlecht bedacht.

So nun die Netzwerkpfade.

1. \\tox10r01\AppData - Die Datei 22062415473_10r01_s_2022_06_24.csv
2. \\tox15r01\AppData - Die Datei 22062415473_15r01_s_2022_06_24.csv
3. \\tox15r02\AppData - Die Datei 22062415473_15r02_s_2022_06_24.csv
4. \\kle15r02\AppData - Der Ordner 22062415473 wird kopiert.
5. \\XPegasus\AppData - 2 Dateien 22062415473_HWH.xarch und 22062415473_SB_HWH.xarch

Auf dem Screenshot Gespeichert.jpg siehst du den Ordner 22050500000R mit seinem Inhalt.
Auf dem Screenshot Tabpage_Lokal ist ein Beispiel von der Konfiguration, was alles angeklickt werden muss.
Z.B. SubFolder wenn man einen Ordner kopieren oder verschieben muss.
Activate 2nd Code wenn es Dateien gibt mit einer anderen Prägung.

Zitat:
Und wäre ein searchPattern wie

Ich habe mit Wildcards probiert, aber wie du an Hand der Dateien und Ordnerbezeichnungen siehst und an meinem Code habe ich das nicht hinbekommen.

Zitat:
PS: Warum überhaupt praegeCodeNrFileName für die Suche nach Ordnern? Du hast doch bei den Kopiermethoden auch noch praegeCodeNrFolderName als Parameter?

praegeCodeNrFolderName ist nur zum erstellen des Ordner im ZielPfad. Buchstabe R oder L.
praegeCodeNrFileName ist zum suchen und kopieren der Ordner und Dateien in den Quellpfaden.

Zitat:
Wußtest du denn nicht von Anfang an, daß dort soviele Unterordner vorhanden sind?

Normalerweise sollte sich pro zyklus nur eine Datei oder Ordner in den Quellen befinden.
Wie ich schon mal sagte das Programm (VBA) auf der Visu Zenon löscht die Dateien nicht.
Über die letzten Wochen oder Monate haben sich so viele alte Daten gesammelt woran ich nicht mehr gedacht habe.
Das neue Programm löscht ja auch gleich das kopierte.

Zitat:
Das vorherige Programm mußte doch ebenso damit umgehen.

Mittlerweile hat sich herausgestellt das auch das alte Programm länger braucht.
Aber die Schnittstelle zwischen der SPS und der Visu Zenon ist natürlich eine professionelle Schnittstelle. (Siemens) Da hat man natürlich keinen Verlust bei der Abfrage der Variablen. Das ist halt Echtzeit.

Zitat:
Kann es sein, daß im Fall Local/NetSubFolderUses[i].Checked

Wie du auf dem Bild Tabpage_Lokal gesehen hast ist da ja eine Checkbox. Die muss angeklickt werden wenn ein Ordner kopiert werden muss.
Es gibt Applikationen die erstellen Ordner mit den Dateien. Andere Applikationen erstellen nur Dateien. Am besten siehst du das auf dem Bild Gespeichert.jpg.

Vielen Dank für deine Hilfe.

Grüße Tom

Moderiert von user profile iconTh69: Beitragsformatierung überarbeitet.


Th69 - Fr 24.06.22 20:16

OK, das mit den Ordner- und Dateinamen habe ich jetzt wohl verstanden.

Bitte probiere mal direkt als searchPattern: "*" + praegeCodeNrFileName + "*" (damit schon das Dateisystem die Filterung durchführt und nicht alle 30.000 Ordner in deinem Code durchlaufen werden - ist analog wie beim Datenbankzugriff). Dies ist logisch äquivalent zu deinem bisherigen Code, nur eben viel performanter.


Ralf Jansen - Fr 24.06.22 20:28

Zitat:
Directory.EnumerateDirectories(sourceFilePath, "*", System.IO.SearchOption.AllDirectories)

Die Frage war auch ob es AllDirectories sein muss und nicht TopDirectoryOnly reicht. Liegen deine, wie von dir erklärt formatierten, Verzeichnisse nicht direkt in sourceFilePath?


UserTom - Fr 24.06.22 22:45

Hallo,

Vielen Dank für Eure Antworten.

Zitat:
Bitte probiere mal direkt als searchPattern: "*" + praegeCodeNrFileName + "*"


Das habe ich. Ich konnte es in der ganzen Klasse IOFileOperations einbauen.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48:
49:
50:
51:
52:
53:
54:
55:
56:
57:
58:
59:
60:
61:
62:
63:
64:
65:
66:
67:
68:
69:
70:
71:
72:
73:
74:
75:
76:
77:
78:
namespace PDE
{
    internal static class IOFileOperations
    {
        public static void TryAction(Action action)
        {
            try
            {
                action();
            }
            catch (DirectoryNotFoundException ex)
            {
                ex.SaveErrorMessage();
            }
        }

        public static void MoveDirectoryfromLocal(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {            
            IEnumerable<string> dirs = Directory.EnumerateDirectories(sourceFilePath, "*" + praegeCodeNrFileName + "*");

            foreach (string dir in dirs)
            {
                TryAction(() => FileSystem.MoveDirectory(dir, dir.Replace(sourceFilePath, Path.Combine(targetPath, praegeCodeNrFolderName)), true));
            }
        }

        public static void MoveFilesfromLocal(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            IEnumerable<string> files = Directory.EnumerateFiles(sourceFilePath, "*" + praegeCodeNrFileName + "*");

            foreach (string file in files)
            {
                FileInfo fi = new FileInfo(file);
                string filename = fi.FullName;
                
                TryAction(() => File.Move(filename, $"{Path.Combine(targetPath, praegeCodeNrFolderName, Path.GetFileName(file))}"));
            }
        }

        public static void MoveFilesCode2fromLocal(string praegeCodeNrFolderName, string praegeCode2NrFileName, string sourceFilePath, string targetPath)
        {
            IEnumerable<string> files = Directory.EnumerateFiles(sourceFilePath, "*" + praegeCode2NrFileName + "*");

            foreach (string file in files)
            {
                FileInfo fi = new FileInfo(file);
                string filename = fi.FullName;
                
                TryAction(() => File.Move(filename, $"{Path.Combine(targetPath, praegeCodeNrFolderName, Path.GetFileName(file))}"));
            }
        }

        public static void CopyDirectoryfromNetwork(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            IEnumerable<string> dirs = Directory.EnumerateDirectories(sourceFilePath, "*" + praegeCodeNrFileName + "*");

            foreach (string dir in dirs)
            {
                TryAction(() => FileSystem.CopyDirectory(dir, dir.Replace(sourceFilePath, Path.Combine(targetPath, praegeCodeNrFolderName)), true));
                TryAction(() => FileSystem.DeleteDirectory(dir, DeleteDirectoryOption.DeleteAllContents));
            }
        }

        public static void CopyFilesfromNetwork(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            IEnumerable<string> files = Directory.EnumerateFiles(sourceFilePath, "*" + praegeCodeNrFileName + "*");

            foreach (string file in files)
            {
                FileInfo fi = new FileInfo(file);
                string filename = fi.FullName;
                
                TryAction(() => File.Copy(filename, $"{Path.Combine(targetPath, praegeCodeNrFolderName, Path.GetFileName(file))}"));
                TryAction(() => File.Delete(file));
            }
        }
    }
}


Das Programm tut alles so wie vorher. Perfekt.
Und wenn es dadurch jetzt noch schneller ist dann hast du mir sehr geholfen.

@Ralf

Zitat:
Die Frage war auch ob es AllDirectories sein muss

Ich denke nicht.
Aber durch die Änderung die mir Th69 vorgeschlagen hat erübrigt sich das ja.

Vielen Dank für alles.

Grüße Tom


Th69 - Sa 25.06.22 08:26

Jetzt verwendest du ja auch kein SearchOption.AllDirectories mehr, so daß jetzt (als Default) TopDirectoryOnly verwendet wird (und das sollte jetzt ja auch richtig sein, da du nur die Hauptordner durchsuchen solltest).

Gibt es denn jetzt einen Plan bei euch, wie ihr die 30000 Ordner wieder reduziert (da du ja geschrieben hast, daß sich diese "alten Daten angesammelt haben")? Für einen stabilen (und performanten) Dauerbetrieb deines Programms sollte das auf jeden Fall angegangen werden.

PS: Noch etwas ist mir aufgefallen:
Warum benutzt du

C#-Quelltext
1:
2:
3:
4:
5:
6:
foreach (string file in files)
{
    FileInfo fi = new FileInfo(file);
    string filename = fi.FullName;
    // ...
}
?
In file steht doch schon der vollständige Pfadname!?! Du kannst also auf das FileInfo (Ressource-)Objekt komplett verzichten.

Und auf die String-Interpolation ($"{...}") bei
$"{Path.Combine(targetPath, praegeCodeNrFolderName, Path.GetFileName(file))}" kannst du auch verzichten.

Das dir.Replace(...) ist auch nicht ganz optimal, da ja eigentlich nur der Pfadname am Anfang ersetzt werden soll (nicht auch noch innerhalb des Pfades).
Erzeuge am besten eine eigene Methode dafür:

C#-Quelltext
1:
2:
3:
4:
5:
string ReplacePath(string path, string sourcePath, string destPath)
{
  // evtl. vorher noch mittels if (path.StartsWith(sourcePath)) überprüfen, sonst String.Empty oder Exception
   return destPath + path.Substring(sourcePath.Length);
}

(oder als Erweiterungsmethode [https://docs.microsoft.com/de-de/dotnet/csharp/programming-guide/classes-and-structs/extension-methods] in einer eigenen statischen Klasse, dann beim ersten Parameter noch this angeben).


UserTom - So 26.06.22 00:12

Hallo Th69,

Vielen Dank für deine Antwort,

Zitat:
Gibt es denn jetzt einen Plan bei euch, wie ihr die 30000 Ordner

Wie ich schon geschrieben hatte, wir das neue Programm, wenn es mal an den "Start geht" die Dateien und Ordner die kopiert werden danach gleich gelöscht.

Zitat:
PS: Noch etwas ist mir aufgefallen:
Warum benutzt du

Ich habe das so im Internet gefunden. Das so etwas nicht richtig ist, das erkenne ich nicht.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48:
49:
50:
51:
52:
53:
54:
55:
56:
57:
58:
59:
60:
61:
62:
63:
64:
65:
66:
67:
68:
69:
namespace PDE
{
    internal static class IOFileOperations
    {
        public static void TryAction(Action action)
        {
            try
            {
                action();
            }
            catch (DirectoryNotFoundException ex)
            {
                ex.SaveErrorMessage();
            }
        }

        public static void MoveDirectoryfromLocal(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            IEnumerable<string> dirs = Directory.EnumerateDirectories(sourceFilePath, "*" + praegeCodeNrFileName + "*");

            foreach (string dir in dirs)
            {
                TryAction(() => FileSystem.MoveDirectory(dir, dir.Replace(sourceFilePath, Path.Combine(targetPath, praegeCodeNrFolderName)), true));
            }
        }

        public static void MoveFilesfromLocal(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            IEnumerable<string> files = Directory.EnumerateFiles(sourceFilePath, "*" + praegeCodeNrFileName + "*");

            foreach (string filename in files)
            {                
                TryAction(() => File.Move(filename, Path.Combine(targetPath, praegeCodeNrFolderName, Path.GetFileName(filename))));
            }
        }

        public static void MoveFilesCode2fromLocal(string praegeCodeNrFolderName, string praegeCode2NrFileName, string sourceFilePath, string targetPath)
        {
            IEnumerable<string> files = Directory.EnumerateFiles(sourceFilePath, "*" + praegeCode2NrFileName + "*");

            foreach (string filename in files)
            {               
                TryAction(() => File.Move(filename, Path.Combine(targetPath, praegeCodeNrFolderName, Path.GetFileName(filename))));
            }
        }

        public static void CopyDirectoryfromNetwork(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            IEnumerable<string> dirs = Directory.EnumerateDirectories(sourceFilePath, "*" + praegeCodeNrFileName + "*");

            foreach (string dir in dirs)
            {
                TryAction(() => FileSystem.CopyDirectory(dir, dir.Replace(sourceFilePath, Path.Combine(targetPath, praegeCodeNrFolderName)), true));
                TryAction(() => FileSystem.DeleteDirectory(dir, DeleteDirectoryOption.DeleteAllContents));
            }
        }

        public static void CopyFilesfromNetwork(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            IEnumerable<string> files = Directory.EnumerateFiles(sourceFilePath, "*" + praegeCodeNrFileName + "*");

            foreach (string filename in files)
            {                
                TryAction(() => File.Copy(filename, Path.Combine(targetPath, praegeCodeNrFolderName, Path.GetFileName(filename))));
                TryAction(() => File.Delete(filename));
            }
        }
    }
}


Zitat:
Das dir.Replace(...) ist auch nicht ganz optimal,


Zitat:
Erzeuge am besten eine eigene Methode dafür:


Warum noch eine Methode?

Grüße Tom


Th69 - So 26.06.22 09:00

Ja, so sieht der Code doch schon viel aufgeräumter und lesbarer aus. Du mußt natürlich bei dir lokal dieses intensiv testen bzw. wenn du Tests auf dem Produktionsrechner machst, das Löschen ersteinmal auskommentieren. Ich hoffe, es gibt Sicherheitskopien der Daten? Ansonsten zieh dir lokal die Daten (oder wenigstens einige Tausend Unterordner davon) und teste damit.

Aber eine wichtige Änderung mußt du noch vornehmen (sonst verlierst du evtl. Daten!).
Und zwar bzgl. TryAction - beim Kopieren und anschließendem Löschen bei beiden Copy...fromNetwork-Methoden fängst du jetzt eine Exception einzeln ab (anstatt bei einer Exception beim Kopieren das Löschen nicht mehr durchzuführen). Daher mußt du beide Methoden-Aufrufe in einen Block packen (so wie auch dein Originalcode aussah):

C#-Quelltext
1:
2:
3:
4:
5:
TryAction(() => 
          {
             File/-System.Copy...(...);
             File/-System.Delete...(...);
          });

(wie du das am besten formatierst mußt du entscheiden bzw. entsprechend deiner VS-Einstellung).

Und eine extra Methode kostet nichts, sie macht den Code aber lesbarer, damit man später anhand des Namens noch erkennt, was diese Methode macht (und auch wegen der zusätzlichen Abfrage).

Dann wünsche ich dir jetzt, daß sich die Mühe ausgezahlt hat und das Programm dann seinen gewünschten Dienst leistet.


UserTom - Mo 27.06.22 08:28

Guten Morgen Th69,

Vielen Dank für deine Antwort.

Funktioniert ist aber noch ein Fehler drin, leider ohne Fehlermeldung, weil eine Datei nicht kopiert wird.

Die Datei "Ch_22062415473.txt"(Beispiel) sollte mit diesem Code kopiert werden.


C#-Quelltext
1:
2:
3:
4:
5:
6:
7:
8:
9:
 public static void MoveFilesfromLocal(string praegeCodeNrFolderName, string praegeCodeNrFileName, string sourceFilePath, string targetPath)
        {
            IEnumerable<string> files = Directory.EnumerateFiles(sourceFilePath, "*" + praegeCodeNrFileName + "*");

            foreach (string filename in files)
            {
                TryAction(() => File.Move(filename, Path.Combine(targetPath, praegeCodeNrFolderName, Path.GetFileName(filename))));
            }
        }


Es gibt 2 Unterschiede, dass bei dieser Datei "Ch_22062415473.txt" die Pfadtiefe 4 ist.
D:\S7_Export\Bosch\CH00\Prog_1 und das CH_ vor der Zahl steht.

Das sollte doch mit "*" + praegeCodeNrFileName + "*" kein Problem sein, oder?

Was ich auch komisch finde ist, dass bevor diese Datei "Ch_22062415473.txt" kopiert wird,
vorher die 6 Dateien aus diesem Ordner "D:\S7_Export\Sensopart\x015id02xa1" kopiert werden.
Da ich nichts anderes erkennen kann, denke ich, dass es die Pfadtiefe ist.
Ach ja und die Dateien, die dann nach der Datei "Ch_22062415473.txt" dran sind, werden kopiert.
Und was ganz wichtig ist, die Datei ist auch vorhanden, die kopiert werden soll. Ich habe es extra ein paar Zyklen laufen lassen.

Es werden alle Dateien und Ordner kopiert, bis auf diese eine. Es ist wie verhext.
Ich habe schon das GUI-Element überprüft. Die Arrays kontrolliert, ob ich vielleicht einen Eintrag vergessen habe.

Was denkst du, woran kann das liegen?

Zitat:
Aber eine wichtige Änderung mußt du noch vornehmen

Danke, habe ich geändert.

Zitat:
Dann wünsche ich dir jetzt, daß sich die Mühe ausgezahlt hat und das Programm dann seinen gewünschten Dienst leistet.

Vielen Dank hätte ich ohne euch auch nicht geschafft.

Grüße Tom


Th69 - Mo 27.06.22 09:41

Wird die Datei schon nicht von Directory.EnumerateFiles gefunden oder gibt es eine Exception beim Verschieben?
Bei ersterem überprüfe mal in der Konsole mit dir *22062415473*, ob diese aufgelistet wird (vorher ins Verzeichnis mit cd wechseln).

Und als sourceFilePath wird auch der volle Pfad "D:\S7_Export\Bosch\CH00\Prog_1" übergeben (nicht, daß es hier um ein Unterverzeichnis geht, da du - auch schon vorher in deinem Code - nur TopDirectoryOnly benutzt)?

Am besten, du debuggst mal den Code (oder läßt dir alle Pfade und Dateien mal ausgeben, s. Ablaufverfolgung und Debuggen in Visual C# [https://docs.microsoft.com/de-de/troubleshoot/developer/visualstudio/csharp/language-compilers/trace-and-debug] - diese werden dann im VS-Ausgabefenster angezeigt).


Ralf Jansen - Mo 27.06.22 10:40

Nebenbei zum ausprobieren. Man kann auch fast alle Framework Funktionen aus der Powershell benutzen und schauen was passiert wenn einem die Konsole in Visual Studio zu unsympatisch ist aber ansonsten gut mit der Shell unterwegs ist. In deinem Fall


C#-Quelltext
1:
[IO.Directory]::EnumerateFiles("C:\MyLovelyPath""*MyLovelyFilter*")                    


UserTom - Mo 27.06.22 10:51

Hallo Th69,

Danke für deine Antwort.

Also auf meinem Testsystem funktioniert es, also kann es nur noch an dem anderen System liegen.

Anbei 3 Dateien vom debuggen.

Vielleicht siehst du etwas, was falsch sein könnte.

"D:\S7_Export\Bosch\CH00\Prog_1"

TopDirectoryOnly ist Prog_1, oder? Wenn ja dann kann es nicht daran liegen.

Vorher wurde von hier kopiert "D:\S7_Export\Sensopart\x015id02xa1"

TopDirectoryOnly wäre x015id02xa1 ?


Grüße Tom


Th69 - Mo 27.06.22 11:16

Ich hatte mich wegen dem Pfad nur gewundert, weil du in deinem Screenshot Tabpage_Local.jpg [https://entwickler-ecke.de/download.php?id=19126] nur den Hauptpfad "D:\S7_Export\Bosch" stehen hattest (aber du testest sicherlich verschiedene Pfade).

Auf deinen angehängten Screenshots kann ich jetzt anhand der Daten auch nichts fehlerhaftes erkennen. Wenn es bei dir lokal funktioniert, aber nicht auf dem Produktionsrechner, dann ist dort etwas mit der Datei (oder dem Ordner) falsch.
Daher probiere mal direkt die Datei dort zu finden (mit Powershell, wie Ralf vorgeschlagen hat).

PS: Noch als Rückfrage, da du nicht direkt dazu was geschrieben hast: Ist die Performance durch das Umschreiben des Filters denn jetzt akzeptabel?


UserTom - Mo 27.06.22 13:33

Hallo Th69,

Ja, die Performance ist ok. Das Tool braucht ca. 6 sek.
Bei 75 % stockt es ein bisschen, weil über Netzwerk kopiert wird.

Zitat:
Ich hatte mich wegen des Pfades nur gewundert,


Ja richtig, bei mir zu Hause habe ich nur mit "D:\S7_Export\Bosch" getestet.
Hier im Büro mit der langen Variante "D:\S7_Export\Bosch\CH_00_1\PRG_01".
Entspricht auch der Variante auf dem Produktionsrechner.

Ich habe mal mit PowerShell getestet. Die Datei wird gefunden. Siehe Bild.

Da muss ich weiter schauen. Ist bestimmt ein ganz banaler Fehler.

Vielen Dank, für Eure Hilfe.

Grüße Tom


Th69 - Di 28.06.22 14:38

Die Performance ist doch schon mal ganz gut (zumindenstens gegenüber den vorherigen 30 bzw. 40 sek.).

Bist du denn jetzt mit dem Fehler bei der Datei weitergekommen?
Wird die Datei denn überhaupt nicht ins Zielverzeichnis kopiert oder wird sie nur nicht im Quellverzeichnis gelöscht? Letzteres kann daran liegen, wenn noch ein Prozess (exklusiven) Zugriff auf diese Datei hat.
Ansonsten mal die Datei selber von Hand kopieren, dann löschen und danach wieder neu zurückkopieren (und dann, wenn keine Fehlermeldung dabei erschienen ist, dein Programm nochmals darüber laufen lassen).


Ralf Jansen - Di 28.06.22 15:37

Zitat:
Letzteres kann daran liegen, wenn noch ein Prozess (exklusiven) Zugriff auf diese Datei hat.

War das nicht eine der wenigen Stellen wo sowas wie Logging eingebaut war :gruebel: Der Faden hier ist schon so lang da kann man schonmal den Faden verlieren :roll:

Sowas wie noch gesperrt oder fehlende Zugriffsrechte auf die Datei oder den Ordner sollte doch im Log stehen.


Th69 - Di 28.06.22 16:06

Ich meinte bei File.Move [https://docs.microsoft.com/en-us/dotnet/api/system.io.file.move]:
Zitat:
Moving the file across disk volumes is equivalent to copying the file and deleting it from the source if the copying was successful.

If you try to move a file across disk volumes and that file is in use, the file is copied to the destination, but it is not deleted from the source.

Und dann kommt aber eben keine Exception (und daher auch kein Log-Eintrag) - wobei ich nicht weiß, ob auf dem Produktionsrechner unterschiedliche Laufwerke verwendet werden (beim lokalen Testen wohl nicht).


UserTom - Do 21.07.22 07:16

Guten Morgen,

Vielen Dank für Eure Antworten und Sorry das ich jetzt erst Antworte.
Ich musste unerwartet auf Dienstreise in ein anderes Werk.

Nun ich habe den "Fehler" gefunden.

@ Ralf

Es hat tatsächlich an den freigegebenen Ordner gelegen.
"D:\S7_Export\Bosch\CH_00_1\PRG_01".
Ich habe die Freigabe für PRG_01 eingerichtet und danach wurde auch die letzte Datei verschoben.

@ TH69

Richtig ich habe keine Exception gehabt und somit auch keinen Log-Eintrag.
Jetzt wo alle Rechten vorhanden sind kopiert das Tool zwischen 3 und 6 sek.

Ich denke das wir hier jetzt abschließen können. Ich habe noch eine Frage auf die Zukunft bezogen.
In meiner Abwesenheit wurde unsere Windows 10 Version 1607 auf 1809 mit einem Setup hochgezogen.
Warum nur auf 1809 kann ich nicht beantworten das hat unsere IT so entschieden.
Jetzt ist natürlich kein .Net Framework 4.8 mehr vorhanden. Das muss ich jetzt nach installieren.
Kann ich das vielleicht in das Tool mit einarbeiten so das, wenn jemand das Programm startet
und es ist kein .Net 4.8 vorhanden, das als Erstes das .Net installiert wird?
Es kann ja auch in der Zukunft noch passieren das auf Windows 11 hochgerüstet wird. Nur als Beispiel.
Wenn das Tool solche Probleme gleich selbst erkennt, wäre das von Vorteil.

Oder wäre ein wechsel auf eine der Core Versionen besser?
Was aber bedeuten würde das wieder alles umgeschrieben werden muss Oder?
Ehrlich gesagt würde ich das lieber vermeiden aber ich schätze Eure professionelle Meinung.

Vielen Dank für Eure tolle Unterstützung.

Grüße Tom


Ralf Jansen - Do 21.07.22 09:43

Wenn man über die normalen Windows Update Mechansimen einen PC aktualisiert verschwinden die installierten Frameworks eigentlich nicht.
Das klingt eher nach mehr oder weniger neu aufgesetzt.


Das nachinstallieren des Frameworks ist tatsächlich nicht übermäßig schwer aber gehört eher in eine Setup Anwendung. Deine Anwendung kann sich nicht wirklich selbst aus diesem Sumpf ziehen. Deine Anwendung ist ja bereits in .Net geschrieben. Das Framework muß aber "vorher" da sein bevor deine Anwendung irgendwas tut.

Wenn auch eine Setup zu viel ist solltest du mal mit deiner IT Truppe sprechen. Wenn die ein Deployment Tool zum PC aufsetzen verwenden an Windows Update vorbei sollten die dann auch das 4.8 Framework mit deployen.


UserTom - Do 21.07.22 10:00

Servus Ralf,

Vielen Dank für deine Antwort.

Ich habe nachgefragt. Das Update von Win 10 wurde über eine Setup.exe gemacht.
Es wurde nicht neu aufgesetzt, da noch alle Programme vorhanden war.

Es liegt an der Windows 10 Version 1809. Erst mit Update vom Oktober 2018 (Version 1809) kann man 4.8 installieren.
Erst mit dem Update vom Mai 2019 wurde 4.8 mit integriert.

https://docs.microsoft.com/de-de/dotnet/framework/migration-guide/versions-and-dependencies#net-framework-48

Dann werde ich das installieren müssen.

Danke

Grüße Tom