Fehler in Integer-Berechnung

[Christian (Gast)]

Moin,

ich habe hier folgende Funktion geschrieben. Urpsrünglich hatte ich sie 
für float Werte geschrieben, was auch funktioniert hat. Jetzt möchte ich 
sie auf Integer Werte umschreiben und bekomme falsche Werte. Sicherlich 
läuft ein Integer über oder so, aber ich weiss nicht wo.

#define PWM_TOP 1600

void SetPWM(uint8_t dutycycle)
{
  uint16_t _ocr1a = 0;
  // Fehlerhafte Eingabe abfangen
  if (dutycycle > 100) dutycycle = 100;
  if (dutycycle < 0)   dutycycle = 0;  
  
        //Vergleichswert berechnen
  _ocr1a = ((PWM_TOP*dutycycle) / 100); 
  
  sprintf(sbuf,"OCR1A: %u", _ocr1a);
  usb_send_str(sbuf);
  
  //Vergleichswert setzen
  OCR1A = _ocr1a;
}

Erwarten würde ich:
_ocr1a = 1600*50/100 = 80.000/100 = 800
Das Ergebnis ist:
_ocr1a = 144

Ich dachte erst, dass das Zwischenergebnis von 80.000 das Problem wäre, 
da es nicht in uint16_t passt. Aber die Berechnung mit uint32_t ergab 
das gleiche unsinnige Ergebnis.

Ich sehe meinen Fehler gerade echt nicht...

Viele Grüße,
Christian

[Sven S. (boldie)]

Probiers mal so. Unsinnigen Vergleich <0 kannste weglassen und dutycycle 
must du erst umwandeln.

   uint16_t _ocr1a = 0;
   // Fehlerhafte Eingabe abfangen
   if (dutycycle > 100) dutycycle = 100; 
         //Vergleichswert berechnen
   _ocr1a = (PWM_TOP/100)*(uint16_t)(dutycycle);
 ...

[Oliver S. (oliverso)]

Christian schrieb:
> Ich dachte erst, dass das Zwischenergebnis von 80.000 das Problem wäre,

Das ist auch das Problem. Die "Falle" in C ist, daß der Compiler erst 
rechnet, und danach in das Ergebnisformat umwandelt. In deinem Fall wird 
mit unsigned int (= uint16_t) gerechnet, und da passiert der Überlauf. 
Ob das falsche Ergebnis dann einem uint16_t oder einem uint32_t 
zugewiesen wird, ändert nichts mehr.

Willst du das vermeiden, musst du die Operanden casten:

_ocr1a = (((uint32_t)PWM_TOP*dutycycle) / 100);

Dann wird mit 32 Bit gerechnet, und alles wird gut (und dauert dafür 
länger).

Oliver

[Christian (Gast)]

Stimmt der Vergleich kleiner Null macht jetzt in Festkommaarithmetik 
keinen sinn mehr ;-)

Ich hatte dutycycle auch schon im Funktionsaufruf als uint16_t 
deklariert. Das Ergebnis war das gleiche. ich Probiers nochmal mit 
deiner Rechenreihenfolge! Danke!

[Christian (Gast)]

Danke Sven, mit der geänderten Berechnungsreihenfolge läufts!
Auch ohne 32Bit cast!

[Dirk B. (dirkb2)]

Christian schrieb:
> mit der geänderten Berechnungsreihenfolge läufts!
> Auch ohne 32Bit cast!
Aber nur richtig, wenn PWM_TOP ein Vielfaches von 100 ist.

Oliver S. schrieb:
> In deinem Fall wird
> mit unsigned int (= uint16_t) gerechnet,
Da alle beteiligten Werte (PWM_TOP, dutycycle, 100) in ein int passen, 
erfolgt die Berechnung sogar als int.

[Oliver S. (oliverso)]

Dirk B. schrieb:
> Da alle beteiligten Werte (PWM_TOP, dutycycle, 100) in ein int passen,
> erfolgt die Berechnung sogar als int.

Stimmt ;)

Oliver

[Kaj (Gast)]

Christian schrieb:
> void SetPWM(uint8_t dutycycle)
> {
>   uint16_t _ocr1a = 0;
>   // Fehlerhafte Eingabe abfangen
>   if (dutycycle > 100) dutycycle = 100;
>   if (dutycycle < 0)   dutycycle = 0;
Wie soll dutycycle denn bitte kleiner 0 werden?
Tipp: void SetPWM(uint8_t dutycycle)
Was haben unsigned Datentypen fuer eine Eigenschaft?

[Dirk B. (dirkb2)]

Kaj schrieb:
> Was haben unsigned Datentypen fuer eine Eigenschaft?
Das hat der TO ja schon vor über fünf Stunden eingesehen

Christian schrieb im Beitrag #3664190 am 22.05.2014 08:53:
> Stimmt der Vergleich kleiner Null macht jetzt in Festkommaarithmetik
> keinen sinn mehr ;-)

[Christian (Gast)]

Richtig, das war ein Überbleibsel aus der ursprünglichen Funktion, die 
mit double gearbeitet hat.