Hallo Forum,
ich habe hier ein kleines Verständnisproblem zur 32-Bit Division.
Ab und zu, in vieleicht 5% der Fälle, kommt es zu fehlerhaften
Berechnungen wenn ich 2 UINT32_T Zahlen miteinander dividiere. Hier mein
Auszug aus Code und Log, vieleicht fällt jemanden was dazu ein.
Dankeschön.
CODE:
declaration:
1
// ADC
2
staticuint32_tAdcSamples[]={0,0,0,0,0,0};
3
staticuint32_tAdcSum[]={0,0,0,0,0,0};
4
staticboolCalculationInProgress=false;
interrupt:
1
ISR(TIMER0_OVF_vect){
2
// dont count on calculation
3
if(!CalculationInProgress){
4
// read ADC for each sensor input
5
// and sumarize ADC values
6
// PINCx = ADCx
7
for(intt=0;t<6;t++){
8
if(SwitchLayout[t]==3){
9
AdcSamples[t]++;
10
AdcSum[t]=AdcSum[t]+(uint32_t)ReadADC(t);
11
}
12
}
13
}
14
}
main:
1
// every minute (64x60)
2
if(AdcSamples[t]>=(64*60)){
3
4
CalculationInProgress=true;
5
6
uint32_troundedVal=AdcSum[t]/AdcSamples[t];
7
sendGenericUint32Message(9634,AdcSum[t]);
8
sendGenericUint32Message(9635,AdcSamples[t]);
9
sendGenericUint32Message(9636,roundedVal);
10
11
AdcSamples[t]=0;
12
AdcSum[t]=0;
13
14
CalculationInProgress=false;
15
16
}
ERKLÄRUNG:
Ein Timer führt alle paar millisekunden die ISR durch und summiert Werte
u.
Anzahl Zählungen um am ende einen Mittelwert bilden zu können.
RICHTIG:
9634 with len 4 0x00 0x19 0x8f 0x02
9635 with len 4 0x00 0x00 0x0f 0x00
9636 with len 4 0x00 0x00 0x01 0xb4
1675010 / 3840 = 436,2005 => 436
FALSCH:
9634 with len 4 0x00 0x19 0x90 0x4c
9635 with len 4 0x00 0x00 0x0f 0x00
9636 with len 4 0x00 0x00 0x01 0x99
1675340 / 3840 = 436,2864 => 436
aber berechnet wird 409
Christian G. schrieb:> Hier mein> Auszug aus Code und Log, vieleicht fällt jemanden was dazu ein.
Wenn bei einer Division ein unerwartetes Ergebnis herauskommt, ist es in
90% der Fälle so, dass nicht die Zahlen durcheinander verrechnet werden,
von denen man es glaubt. In den restlichen 10% ist das Ergebnis, das man
sich ansieht, nicht das Ergebnis der genannten Operation.
Sprich: Guck, ob wirklich im Array in dem Moment das drin steht, was
erwartet wird.
Franz schrieb:> "CalculationInProgress" sollte "volatile" sein.
Und mindestens AdcSamples[] oder AdcSum[], ansonsten finden die
Operationen auf den Arrays nicht unbedingt zwischen den beiden
Operationen auf das bool statt.
Ansonsten ist der Code äquivalent zu:
Christian G. schrieb:> AdcSum[t] = AdcSum[t] + (uint32_t) ReadADC(t);
ReadADC () ist hier nicht zu sehen, könnte bisschen länger
dauern als das was man in der ISR tun sollte.
War mir garnicht bewusst, ich werde es mal so umsetzen und beobachten.
@mitlesa : ich war davon ausgegangen,dass es keine Rolle spielt, da die
Zahlen für die Division ja die "richtigen" sind
1
uint16_tReadADC(uint8_tADCchannel){
2
//select ADC channel with safety mask
3
ADMUX=(ADMUX&0xF0)|(ADCchannel&0x0F);
4
//single conversion mode
5
ADCSRA|=(1<<ADSC);
6
// wait until ADC conversion is complete
7
while(ADCSRA&(1<<ADSC));
8
returnADC;
9
}
und natürlich werde ich das auslagern in die main :)
mitlesa schrieb:> Christian G. schrieb:>> AdcSum[t] = AdcSum[t] + (uint32_t) ReadADC(t);>> ReadADC () ist hier nicht zu sehen, könnte bisschen länger> dauern als das was man in der ISR tun sollte.
vor Allem, da das ja 6x ausgeführt wird.
Christian G. schrieb:> while( ADCSRA & (1<<ADSC) );
Funktionsaufrufe können lange dauern, vor Allem, wenn viel auf den Stack
muss. Außerdem sind solche Schleifen im Interrupt ein no-go.
Schaue dir mal den "ADC conversion complete" interrupt an!
Verkleinere mal die Datentypen, wo es möglich ist. Ist uint32_t wirklich
nötig???
Außerdem sorgt "CalculationInProgress" dafür, dass sicherlich nicht
jeder timer interrupt den ADC bedient. Gegebenenfalls kann das auch zu
Problemen führen.
Vielen Dank für eure Mühe,
CalculationInProgress hatte ich nur testhalber eingebaut um genau das zu
erreichen was volatile scheinbar macht. Dass die ISR nicht in den Werten
herumschreibt. Bis jetzt scheint es zu laufen, mir waren die Eigenheiten
von volatile in C nicht bekannt. Ich und C werden eh keine Freunde mehr
;)
uint32_t ist leider aufgrund der mess- und kalkulationsgenauigkeit
nötig.
Ich beobachte mal obs noch ausreißer gibt, bis dahin hab ich erstmal
viel anderen Code zu fixen ...
"volatile" ist keine Memory Barrier. Die Zugriffe auf dieses Flag sind
per Sprachdefinition zwar in sich geordnet, müssen das aber nicht
gegenüber den Zugriffen auf die Arrays sein. Auch wenn das nun
wahrscheinlich so rauskommt.
https://www.nongnu.org/avr-libc/user-manual/group__avr__cpufunc.html
