Hui, andere kritisieren, dass macht immer Spaß.
Aber jetzt mal Ernst :)
- die deutschen Namen sind nicht besonders gut. Dadurch, dass der Rest
auf Englisch ist, wird das Lesen schwerer. Besonders "schlimm" sind die
Kombinationen wie "tb_SeriellOut" und "seriellWriteLine"
- "//Hexfile auslesen, Bytegröße berechnen, Fehlerabsicherung (MsgBox
Datei zu groß)". Frei nach Scott Meyers Effective C++(kann man auch gut
für C# verwenden :). Du machst zu viel in einer Methode. Besser, du
teilst das in kleine Häppchen auf. Das macht den Quellcode
übersichtlicher, leichter zu ändern und zu warten.
- Das ist an der Stelle nicht Wichtig, aber für die Zukunft:
1 | // pseudocode
|
2 | string complete = "";
|
3 | string hilfsstring = "";
|
4 | for (1 bis 1000){
|
5 | hilfsstring = irgendwas();
|
6 | complete += hilfsstring;
|
7 | }
|
Das wird schnell seeeeehr langsam, da complete jedes Mal komplett in
einen neuen Speicherbereich kopiert und der alte vom GC freigegeben
werden muss. Dafür gibt’s in C# den StringBuilder.
1 | // wieder pseudocode
|
2 | StringBuilder complete = new StringBuilder();
|
3 | string hilfsstring = "";
|
4 | for (1 bis 1000){
|
5 | hilfsstring = irgendwas();
|
6 | complete.Add(hilfsstring);
|
7 | }
|
8 | daten = complete.ToString();
|
- ich hab das jetzt nicht wirklich ausprobiert. Du speicherst den Index
der der ComboBoxen? Das würde bei mir nicht gut gehen. Durch diverse
USB-Seriell Wandler steht da dann ein Zufälliger Port drin.
- weiter zu den Boxen. Das mit den Schaltflächen für default ist
unnötig. Speicher beim Beenden des Programms einfach den aktuellen
Inhalt als String ab und lade den beim Neustart (inklusive Port
existiert und wenn nicht, dann ersten Port anzeigen). Das ist in anderen
Programmen gängig und entspricht einer gewissen Erwartungshaltung.
- ReadConvertHexFile gibt bei dir ein Byte Array zurück. Das kannst du
dir sparen. gib an der Stelle einfach ein string Array zurück, in dem
die eingelesenen Zeilen stehen. Da tust du dich leichter.
1 | // einlesen
|
2 | string []allLines = File.ReadLines(pfad);
|
3 |
|
4 | // später auf den Port ausgeben
|
5 | foreach(var oneLine in allLines){
|
6 | serialPort.Write(oneLine);
|
7 | }
|
- Noch mal zu den Namen. Überlege Dir bessere :) "bytes" ist etwas
ungenau da könnten auch die daten in einem Array drin stehen. byteCount
wäre da schon besser. "Nebenthread" ist auch nichtsssagend. Da musst du
dir noch etwas besseres ausdenken. Du kannst auch am Anfang den
erstbesten Namen, solltest doch später noch mal drüber nachdenken. Das
ist mal wichtig, wenn du ein halbes Jahr den Code nicht siehst, dann
stolperst du über so was.
- So weit ich den Artikel überflogen habe, ist der Bootloader ja
ziemlich geschwätzig. Du solltest diese Sachen auswerten. Ich habe
Testweise mal ein hexfile an einen Port gesendet, an dem nichts
angeschlossen war. Lief gut durch :)
- Wozu sind die Textbox und der Senden button gut? Der Ansatz ist gut.
Statt dem Senden Button kannst du einen wähle Datei Button machen, der
deinen OpenFile Dialog aufmacht. Das Ergebnis vom dialog schreibst du
dann in die Textbox(die du beim Verlassen des Programmes gerne mit
abspeichern kannst). der Starte Flash Vorgang liest nun die Datei aus
dem Textfeld ein und bemängelt maximal, dass er die Datei nicht finden
konnte, oder dass nichts drin steht. Dann brichst du wieder ab und der
Anwender kann sich die Datei raussuchen.
- Das mit dem Port extra öffnen kannst du dir auch sparen. Mach ihn auf,
wenn du flashen willst und danach wieder zu. Beachte, dass der Port auch
gerne mal von einem andern Programm belegt sein kann. Stichwort
try/catch/finally.
So, das wars fürs erste. Mir fällt zwar noch mehr auf, aber mir bluten
schon die Fingerkuppen ^^
Grüße,
