Hab mir gedacht, dass diese Symbole nur zu Versuchszwecken da waren, eben weil sie auskommentiert sind. Das mit der Arbitration kann man ja einfach so lassen, der Code stört ja nicht
----------
Bin grad wieder beim Review und hab da etwas bemerkt. Es geht um die Datei edl-hw.c in Zeile 45. Dort setzt Du ADIF auf 1, was aber der Interruptflag ist (der auch hardwareseitig zurückgesetzt wird). Ich denke, hier wolltest Du ADIE, also den Interruptenable, auf 1 setzen.
----------
Datei edl-panel.c
Zeile 181: Der hintere Ausdruck müsste
Code: Alles auswählen
"Modify == ModifyVolt"Eher unwichtig:
Zeile 85: Der Kommentar müsste "3" heißen.
Zeile 258: Hier könnte man auch die nicht auskommentierte if- Funktion auskommentieren und somit einfach direkt
Code: Alles auswählen
return 0x00Zeile 362 wird ModifyVoltagePos mit -1 initialisiert und dann wird in Zeile 659 abgefragt, ob es -1 ist, um es dann auf 5 zu ändern. Gleiches gilt für ModifyCurrentPos in Zeile 633 und 664 auf den Wert 3. Man könnte die Initialisierungswerte dann auch gleich auf "5" bzw "3" ändern und somit die beiden Änderungen, die ja in jedem Fall beim ersten Aufruf dieser Funktion stattfinden, überspringen.
Außerdem: Die Funktionsheader sollte man, zwecks Übersichtlichkeit und Informationstreue, entweder anpassen oder komplett entfernen. Viele passen nicht (mehr) auf die Funktion, die sie beschreiben. Da in den anderen Dateien auch keine sind, würde ich vorschlagen, sie komplett zu entfernen. So wie es aktuell ist, verwirrt es manchmal schon. Aber das kann ja dann in der "allgemeinen Aufräumaktion" passieren
----------
Datei edl.c
In der Funktion RecallDefaultParamSet (Zeile 275) würde ich auch noch
Code: Alles auswählen
wModeRange = Params.ModeRange;Ansonsten könnte man sich überlegen, die beiden Funktionen RecallUserParamSet und RecallDefaultParamSet zusammenzuführen, um Speicher zu sparen. Bei Gelegenheit kann ich da mal einen Versuch wagen, jetzt mach ich aber erst mal das Review
Die Pins sind in der C- FW anders initialisiert als in der Pascal, aber ich vermute, das wird Absicht sein. Zum Vergleich:
Code: Alles auswählen
//CFW PASCAL
DDRA = 0x00; // =0x00
PORTA = 0x03; // =0x00
DDRB = 0xBB; // =0xBB
PORTB = 0xD7; // =0xFD
DDRC = 0xFC; // =0xFC
PORTC = 0x0F; // =0xC3
DDRD = 0x0C; // =0x0C
PORTD = 0xEC; // =0xFF
PB1, PC2,3: in C als Output (init high), in Pascal als Output (init low)
PB3,5, PC7: in C als Output (init low), in Pascal als Output (init high)
PA0,1: in C als Input mit internem Pullup, in Pascal als Input mit Tristate
PD0,1: in C als Input mit Tristate, in Pascal als Input mit internem Pullup
andere Unterschiede betreffen die nicht benutzten Pins PC6 und PD4
In Zeile 1241 wird der StartTimer überprüft. Die letzte Aktion findet statt, wenn er 40 ist. Hier könnte man dann die entweder Abfrage dahingehend verändern, dass der StartTimer nur bis 41 oder etwa 50 läuft, oder ihn beim erreichen von 40 auf 254 ändern ( nach dem jobFaultCheck() ), um somit die unnötigen weiteren Einsprünge in die if- Anweisungen zu unterbinden. Könnte dann so aussehen:
Code: Alles auswählen
if (StartTimer < 255)
{
if (StartTimer == 4)
{
// ...
}
else if (StartTimer == 40)
{
jobFaultCheck();
StartTimer = 254;
}
StartTimer++;
}
Einige Sachen sind hier eher unwichtiger Natur. Diese hab ich nur notiert, weil sie mir aufgefallen sind, eben der Vollständigkeit halber
Hab das Review hiermit beendet, was aber keinesfalls heißen soll, dass nun alle Fehler verbannt sind
Gruß, Allack
Wird bald etwas weniger unschön. 
