Du gestattest ein paar Anmerkungen
vermische nicht neue ISR Schreibweise mit den alten Namen der Interrupt
Vektoren. Die neuen Namen der Interrupt Vektoren enden allesamt immer
auf _vect
Ob diese Variable aus dem Modul herausgeführt werden muss oder nicht,
ist debatierbar. Defaultmässig würde ich sie im Modul verstecken mit dem
Hintergedanken, dass es ausserhalb niemanden etwas angeht, welcher ADC
jetzt gerade im Moment ausgelesen wird. Selbiges für das _adcdata Array.
Da du sowieso schöne Zugriffsfunktionen hast, willst du eigentlich
nicht, dass sich jeder dahergelaufene Code an diesen Variablen zu
schaffen macht.
Variablennamen die mit _ beginnen sind ausschliesslich für den Compiler
bzw. die Runtime-Library reserviert. Sinn der Sache ist es, dass es
keine Namenskollisionen von selbstdefinierten Namen und Dingen aus der
Runtime Library gibt. Man könnte jetzt argumentieren, dass dieselbe
Strategie auch hier sinnvoll ist, aber so richtig koscher ist es
trotzdem noch nicht. Machst du deine jetzigen Variablen modulglobal
(also static auf File-Scope), dann stellt sich dieses Problem allerdings
gar nicht mehr :-)
Halte dich bitte an die Konvention, dass Makros komplett in
Grossbuchstaben geschrieben werden. Das ist zwar nur eine Konvention,
allerdings ist es eine der wenigen, an die sich tatsächlich so gut wie
alle C-Programmierer halten. Sinn der Sache ist es, dass man im Code den
Unterschied zwischen
und
sofort erkennen kann. Zweiteres ist ein Makro, und daher muss man das
Makro überprüfen, ob diese Operation nicht in die Hose gehen wird.
Ersteres ist eine Funktion und daher brauche ich mir keine Gedanken
machen. Die Argumente werde einmal ausgewertet und an die Funktion
übergeben. Da kann es per Definition nicht passieren, dass i++ zweimal
ausgewertet wird.
Auch ist bei
1 | extern volatile unsigned int _adcdata[ADC_ADCS];
|
jedem sofort klar, das die Anzahl der _adcdata Einträge mittels eines
Makros festgelegt wird und es daher irgendwo einen #define dafür gibt.
1 | (_adcdata[_adc_current_pos]==ADC_DO_NOT_MEASURE
|
ADC_DO_NOT_MEASURE (schau dir auch die Schreibweise an: measure, nicht
messure) ist ein Makro, welches einen fixen Wert verkörpert. Mit der
Schreibweise seh ich das, mit deiner ist nicht klar ob es sich da nicht
etwa um eine Variable handelt, die ihren Wert auf dubiose Weise bekommt.
1 | #define ADCSRA_CONFIG (1<<ADEN)+(1<<ADSC)+(1<<ADIE)+(1<<ADPS2)+(1<<ADPS1)+(0<<ADPS0)
|
Wozu gibt es dieses Makro, wenn es dann sowieso nicht benutzt wird?
1 | void adc_remove(unsigned char channel) {
|
2 | unsigned char i=ADC_adcs;
|
3 | while(i--){
|
4 | if(i==channel) continue;
|
5 | if(_adcdata[i]==ADC_do_not_messure){
|
6 | _adcdata[channel]=ADC_do_not_messure;
|
7 | return;
|
8 | }
|
9 | }
|
10 | }
|
Sorry, aber diese Funktion verstehe ich einfach nicht. IMHO ist die
völlig falsch: Ein Kanal wird nur dann aus der Messung ausgeschlossen,
wenn es noch einen anderen Kanal gibt, der bereits aus der Messung
ausgeschlossen war? Irgendwie ergibt das nicht viel Sinn (oder aber ich
verstehe den Sinn nicht)