Seltsamer Fehler mit Festkommaarithmetik und 7-Segment

[Philipp (Gast)]

Nabend,

ich bin leider noch ziemlich unerfahren in der Mikrocontroller 
Programmierung und habe gerade mit einem Problem zu kämpfen. 
Warscheinlich ist es nur eine Kleinigkeit aber ich komm gerade einfach 
nicht drauf.

      if(flag.multiplex==1)
      {
        flag.multiplex=0;
        result = ADC;
        ADCSRA  |= (1<<ADSC);

        set_display_number(result*176/1024);
        set_dot(zehn);
      }

Um diesen Codeausschnitt handelt es sich, wobei result als uint32_t 
deklariert wurde. Der angezeigte Wert ist im Prinzip auch korrekt, 
ausser bei ganzen Zahlen: Wenn z.B. 10.0 angezeigt werden müsste wird 
komischerweise 00.0 angezeigt und bei 12.0 wird 11.0 angezeigt.

An der set_display_number() Methode liegt es nicht, denn wenn ich dieser 
die Werte direkt übergebe werden sie richtig angezeigt.


Über einen kleinen Denkanstoß würde ich mich sehr freuen!

Gruß,
Philipp

[Daniel H. (Firma: keine) (commander)]

Philipp schrieb:
> set_display_number(result*176/1024);

Ich vermute du wirst an der Stelle einen Overflow haben, d.h. result*176 
wird größer als es uint32_t zulässt. Übrigens würde ich mir an der 
Stelle die Division sparen und einfach

> result * 0.17185

rechnen.

[Dennis H. (c-logic)]

oder x 11 / 64

[Philipp (Gast)]

Hallo,

leider funktioniert es damit auch nicht. Der Fehler bleibt bestehen.

Die 176 kommen übrigends dadurch zustande, dass die Referenzspannung 
1,1V beträgt und die zu messende Spannung per Spannungsteiler durch 16 
geteilt wird. Dann noch ein Faktor 100 um aus den 1/000V wieder meine 
1/10V zu machen.

Also quasi: set_display_number(result*1100*16*100/1024)

Liegt der Fehler vielleicht dort?

[Philipp (Gast)]

Philipp schrieb:

> Also quasi: set_display_number(result*1100*16*100/1024)

Sorry! Ich meinte natürlich /100 und nicht *100!

[Falk B. (falk)]

@  Daniel H. (Firma: keine) (commander)

>> set_display_number(result*176/1024);

>Ich vermute du wirst an der Stelle einen Overflow haben, d.h. result*176
>wird größer als es uint32_t zulässt.

Kaum, denn result als ADC ergebniss ist max. 10 Bit, mal 176 (8 Bit 
macht) maximal 18 Bit. Ok, result ist wahrscheinlich 16 Bit, dann wird 
es ein Overflow. Also eher so.

set_display_number(result*176L/1024);

> Übrigens würde ich mir an der
>Stelle die Division sparen und einfach

>> result * 0.17185

>rechnen.

Dann wäre der Sinn von Festkommaarithmetik verfehlt.

[fonsana (Gast)]

Teile die Zeile in einzelne Schritte und gib die einzelnen 
Zwischenergebnisse aus.

fonsana

[Matthias S. (Firma: matzetronics) (mschoeldgen)]

Falk Brunner schrieb:
>> Übrigens würde ich mir an der
>>Stelle die Division sparen und einfach
>
>>> result * 0.17185
>
>>rechnen.
>
> Dann wäre der Sinn von Festkommaarithmetik verfehlt.

Jo, Vorsicht. Eine einzige float in so einer Berechnung zieht die math 
library rein und bläht den Code so richtig auf. Am besten immer ein Auge 
auf die Codegrösse beim Compilieren werfen, damit das nicht passiert.

Philipp schrieb:
> Die 176 kommen übrigends dadurch zustande, dass die Referenzspannung
> 1,1V beträgt und die zu messende Spannung per Spannungsteiler durch 16
> geteilt wird.

Das klingt übrigens so, als würdest du freiwillig eine Menge Auflösung 
verschenken. Absicht?

[Bernhard S. (b_spitzer)]

Wie sieht den der Funktionsprototyp von set_display_number() aus? Hat 
der auch uint32 oder nur uint16? Wenn uint16, dann versuche mal einen 
Type-Cast mit

set_display_number((uint16t)((result*11)/64));

Die Klammern habe ich gesetzt, damit der Compiler auch sicher nicht 
zuerst 11/16 rechnen will. Dabei kommt nämlich 1 raus...

tschuessle
Bernhard

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

Bernhard Spitzer schrieb:
> damit der Compiler auch sicher nicht zuerst 11/16 rechnen will.
Will er sicher nicht, denn da steht 64...
> zuerst 11/16 rechnen will. Dabei kommt nämlich 1 raus...
Bei mir kommt mit 11/16 nur 0 raus...


Man könnte da mal eine Zwischenvariable einführen, und die genauer 
anschauen. So würde ich das machen...

Philipp schrieb:
> Wenn z.B. 10.0 angezeigt werden müsste wird
> komischerweise 00.0 angezeigt und bei 12.0 wird 11.0 angezeigt.
Evtl. passt da auch einfach was mit dem Multiplexen nicht zusammen, und 
es wird die falsche Ziffer an der falschen Stelle angezeigt...

[Karl H. (kbuchegg)]

Bernhard Spitzer schrieb:
> Wie sieht den der Funktionsprototyp von set_display_number() aus? Hat
> der auch uint32 oder nur uint16? Wenn uint16, dann versuche mal einen
> Type-Cast mit
>

set_display_number((uint16t)((result*11)/64));

> Die Klammern habe ich gesetzt, damit der Compiler auch sicher nicht
> zuerst 11/16 rechnen will.

Das darf er sowieso nicht.
Neben Precedence gibt es ja auch noch die Assoziativität, die das klar 
regelt.

[Peter D. (peda)]

Philipp schrieb:
> Um diesen Codeausschnitt handelt es sich

Das ist ungefähr so, als würde von Dir jemand verlangen, ein Buch durchs 
Schlüsselloch zu lesen.

Code-Pfitzelchen bringen rein garnichts!
Da muß man ewig rumraten, wie alle Funktionen, Macros und Variablen 
deklariert sind und was sie vermutlich bedeuten könnten.

Zeig einen compilierbaren relevanten Code als Anhang.


Peter

[Karl H. (kbuchegg)]

Peter Dannegger schrieb:
> Philipp schrieb:
>> Um diesen Codeausschnitt handelt es sich
>
> Das ist ungefähr so, als würde von Dir jemand verlangen, ein Buch durchs
> Schlüsselloch zu lesen.

Den muss ich mir merken. Der ist gut.

[MWS (Gast)]

> set_display_number(result*176/1024);

Da mangels ausführlichen Codes sowieso Ratestunde ist, tippe ich mal 
darauf dass es egal ist, wie result hier deklariert ist, falls es erst 
dem Funktionsparameter übergeben und dann darauf die Rechnung ausgeführt 
wird.

Leicht feststellbar, indem man den Parameter auf 32Bit ändert. Oder sich 
lss ansieht.

[Philipp (Gast)]

Hallo,
Danke erstmal für die vielen antworten. Ich reiche den vollständigen 
Code heute Abend noch nach. Ich komme dem Fehler nicht auf die Spur.

Als ich mir gestern zum testen mal den adc-Wert aus result 
(übrigends32bit) ausgeben lassen habe ist mir aufgefallen, dass der Wert 
manchmal nicht korrekt ist und "springt". Z.b. Kommt nach dem Wert 696 
nicht 697 sondern 607 und danach kommt 698.

Meine set number Methode zeigt aber alle Zahlen von 0-999 korrekt an(mit 
for getestet).

Bis später,
Philipp

[Stefan W. (dl6dx)]

Philipp schrieb:
> Ich reiche den vollständigen Code heute Abend noch nach.

Hallo Philipp,

wichtig ist auch die Information, welchen Compiler du nutzt und auf 
welchem Prozessor das Ganze läuft.

Grüße

Stefan

[Falk B. (falk)]

@  Philipp (Gast)

>Meine set number Methode zeigt aber alle Zahlen von 0-999 korrekt an(mit
>for getestet).

Wie? So hier?

      if(flag.multiplex==1)
      {
        flag.multiplex=0;
        //result = ADC;
        // test
        result = 1023;
        ADCSRA  |= (1<<ADSC);

        set_display_number(result*176/1024);
        set_dot(zehn);
      }

[Philipp (Gast)]

Hallo,

hier ist der Programmcode. Ich habe das übrigends in Eclipse (mit 
AVR-Plugin) geschrieben und mit AVR-Studio4 auf meinen Atmega168PA 
gebrannt. Leider gibt es in Eclipse den Atmega168PA nicht zur Auswahl 
und bei dem Atmega168P lassen sich die Register nicht ansprechen, 
weshalb ich den Atmega168 ausgewählt habe(Ich hoffe es liegt nicht 
daran?).

Falk Brunner schrieb:
> @  Philipp (Gast)
>
>>Meine set number Methode zeigt aber alle Zahlen von 0-999 korrekt an(mit
>>for getestet).
>
> Wie? So hier?
>       if(flag.multiplex==1)
>       {
>         flag.multiplex=0;
>         //result = ADC;
>         // test
>         result = 1023;
>         ADCSRA  |= (1<<ADSC);
>
>         set_display_number(result*176/1024);
>         set_dot(zehn);
>       }

Es wird 175 Ausgegeben. Wenn ich anstelle von 1023 result per 
for-schleife durchzähle funktioniert die Ausgabe bei allen Zahlen 
Fehlerfrei. Ich schätze es muss irgendwie mit dem ADC-Wert 
zusammenhängen, da dort manchmal schon völlig falsche Werte ausgespuckt 
werden.(?)

#define taster_channel PC1
#define taster_mode PC2
#define hundert 2
#define zehn 1
#define eins 0

#include <avr/io.h>
#include <avr/sleep.h>
#include <avr/interrupt.h>
#include <avr/eeprom.h>
#include <util/delay.h>

#include "debounce.h"
#include "7segment.h"

typedef struct {      // Flag Register
  unsigned display:1;
  unsigned dot:1;
  unsigned multiplex:1;

} flagbyte;

volatile flagbyte flag;
volatile uint16_t count=0;

ISR(TIMER0_OVF_vect)
{
  flag.multiplex=1;
  if(flag.display)
  {
    if((PORTC&0b00111000)==0b00011000)
    {
      PORTD=0;
      PORTC=0b101000;
      PORTD=led_data[1];
    }
    else if((PORTC&0b00111000)==0b00101000)
    {
      PORTD=0;
      PORTC=0b110000;
      PORTD=led_data[0];
    }
    else if((PORTC&0b00111000)==0b00110000)
    {
      PORTD=0;
      PORTC=0b011000;
      PORTD=led_data[2];
    }
  }
  else
  {
      PORTD=0xFF;
      PORTC=0b00011000;
  }
}

int main(void)
{
  DDRD=0xFF;
  PORTD=0xFF;

    DDRC=0b00111000;
    PORTC=0b00011000;

    TIMSK0 = (1<<TOIE0);
    TCCR0B = (1<<CS02);

    TIMSK1 = (1<<TOIE1);
    TCCR1B = (1<<CS11);

    uint32_t result;
  DIDR0=1;
  ADMUX   = (1<<REFS1) | (1<<REFS0);
  ADCSRA  = (1<<ADEN) | (1<<ADPS0) | (1<<ADPS1) | (1<<ADPS2);
  ADCSRA  |= (1<<ADSC);
  while (ADCSRA & (1<<ADSC) ) {}
  result = ADC;

    flag.display=1;

    sei();

    while (1)
    {
      if(debounce (PINC, taster_channel))
      {
        toggle_dot(zehn);
      }
      if(debounce (PINC, taster_mode))
      {
        flag.display^=1;
      }
      if(flag.multiplex==1)
      {
        flag.multiplex=0;
        result = ADC;
        ADCSRA  |= (1<<ADSC);
        set_display_number(result*176/1024);
        //set_display_number(result);
      }
    }
    return 1;
}

Und hier noch der Inhalt aus der 7-Segment.h Datei:

volatile uint8_t led_data[3];

volatile uint8_t segmente[]={
  0b10000001,    //0
  0b11010111,    //1
  0b10100100,    //2
  0b10010100,    //3
  0b11010010,    //4
  0b10011000,    //5
  0b10001000,    //6
  0b11010101,    //7
  0b10000000,    //8
  0b10010000,    //9
  0b11000000,    //A =10
  0b10101001,    //C =11
  0b10101000,    //E =12
  0b11000010,    //H =13
  0b11111110,    //- =14
  0b11110000    //° =15
};

void set_display_number(uint16_t number)
{
  if(number>=999)
  {
    led_data[hundert]=segmente[9];
    led_data[zehn]=segmente[9];
    led_data[eins]=segmente[9];
  }
  else
  {
    uint8_t counter=0;
    while(number>99)
    {
      counter++;
      number -= 100;
    }
    led_data[hundert]=segmente[counter];
    counter=0;
    while(number>9)
    {
      counter++;
      number -= 10;
    }
    led_data[zehn]=segmente[counter];
    counter=0;
    led_data[eins]=segmente[number];
  }
}

void set_dot(uint8_t ziffer)
{
  led_data[ziffer]&=~(0b10000000);
}

void delete_dot(uint8_t ziffer)
{
  led_data[ziffer]|=(0b10000000);
}

void toggle_dot(uint8_t ziffer)
{
  led_data[ziffer]^=(0b10000000);
}

(Ich hoffe das ist jetzt nicht zu viel Code für einen Beitrag)

[Karl H. (kbuchegg)]

Was macht der Timer 1?


>    TIMSK1 = (1<<TOIE1);
>    TCCR1B = (1<<CS11);

Du gibst da einen Interrupt frei, für den ich die ISR nicht sehen kann. 
Nicht gut.


> Ich schätze es muss irgendwie mit dem ADC-Wert
> zusammenhängen, da dort manchmal schon völlig falsche
> Werte ausgespuckt werden.(?)

Das lässt sich ja überprüfen. Du hast eine Ausgabe zur Verfügung (wenn 
auch nur 3-stellig. Lässt du dir halt einfach immer die Hälfte vom ADC 
Wert anzeigen, dann passt der da rein. Dein Problem hängt ja nicht damit 
zusammen, dass der ADC Wert um +- 1 schwankt)

[Karl H. (kbuchegg)]

Wie ist die Aussenbeschaltung vom ADC?
ARef, AVcc?

[Philipp (Gast)]

Karl Heinz Buchegger schrieb:
> Du gibst da einen Interrupt frei, für den ich die ISR nicht sehen kann.
> Nicht gut.

Oh Sorry, das ist noch ein Überbleibsel als ich ausprobiert habe was 
passiert wenn ich den ADC in einem seperaten langsam laufenden 
Timer-Interrupt auslese.

Karl Heinz Buchegger schrieb:
> Das lässt sich ja überprüfen. Du hast eine Ausgabe zur Verfügung (wenn
> auch nur 3-stellig. Lässt du dir halt einfach immer die Hälfte vom ADC
> Wert anzeigen, dann passt der da rein. Dein Problem hängt ja nicht damit
> zusammen, dass der ADC Wert um +- 1 schwankt)

Habe jetzt mal folgendes ausgeben lassen:

      if(flag.multiplex==1)
      {
        flag.multiplex=0;
        //result = ADC;
        set_display_number(ADC/2);
        ADCSRA  |= (1<<ADSC);

        //set_display_number(ADC/2);
        //set_display_number(result);
      }

Dabei kommt das hier heraus:
http://www.youtube.com/watch?v=gZnhV_pHSr8

Also wieder total komische Werte (von 199 -> 190 -> 101 -> 102 -> 200 -> 
201). Und zwar nicht zufällig sondern diese Sprünge sind immer an der 
gleichen Stelle im Wertebereich. Ist meine 7-Segment Routine so OK oder 
könnte es vielleicht doch irgendwie daran liegen? Ich weiß nicht was ich 
noch testen könnte.

Kann mein Atmega vielleicht auch eine Macke haben?

Karl Heinz Buchegger schrieb:
> Wie ist die Aussenbeschaltung vom ADC?
> ARef, AVcc?

Aref ist mit 100nF nach GND entkoppelt und AVCC mit 22µH + 100µF nach 
VCC.

[Stefan W. (dl6dx)]

Philipp schrieb:
> void set_display_number(uint16_t number)

Das dürfte die entscheidende Information sein.

set_display_number(result*176/1024) enthält also einen impliziten cast 
des Ausdrucks result*176/1024 auf uint16_t.

Wenn ich den Abschnitt "Conversions" (C89, identisch ISO/IEC 9899:1999) 
richtig verstehe, sollte dieser cast aber erst nach der Auswertung des 
rvalue greifen. result*176/1024 müsste also noch uint32_t sein.

Ok, schauen wir mal:
ADC 10 bit. *176 gibt 28 bit. /1024 ist ein Rechtshift um 10 bit, 
bleiben also 18 bit übrig. -> Das passt nicht in einen uint16_t.

Du hast aber nur 3 signifikante Ziffern, der Wert ist also eh zu weit 
aufgelöst. Ist der Umrechnungsfaktor wirklich passend?

Grüße

Stefan

[Karl H. (kbuchegg)]

Stefan Wagner schrieb:

> Ok, schauen wir mal:
> ADC 10 bit. *176 gibt 28 bit. /1024 ist ein Rechtshift um 10 bit,
> bleiben also 18 bit übrig. -> Das passt nicht in einen uint16_t.

Irgendwo musst du dich da verrechnet haben.
Das Ergebnis kann nur Werte im Bereich 0 bis 176 sein.

[Stefan W. (dl6dx)]

Ach ja, was mir noch auffiel:

Philipp schrieb:
> volatile uint8_t segmente[]={

Wenn ich das richtig sehe, ist das doch die Tabelle mit den Werten für 
die Ansteuerung der Segmente.

Warum dann die Deklaration als volatile? Diese Werte sind doch konstant 
und sollen es auch bleiben. Einer Optimierung durch den Compiler steht 
doch nicht im Wege, selbst wenn auch aus einer ISR auf das Array 
zugegriffen wird. (Die Deklaration const uint8_t segmente[] hielte ich 
für wesentlich sinnvoller.)

Grüße

Stefan

[Karl H. (kbuchegg)]

Philipp schrieb:

> Also wieder total komische Werte (von 199 -> 190 -> 101 -> 102 -> 200 ->
> 201).

Das sieht so aus, als ob die Segmenttabelle um 1 verschoben wäre.
eine angezeigte 9 ist eigentlich 8
                0 ist eigentlich 9
                1 ist eigentlich 0
usw.

> Und zwar nicht zufällig sondern diese Sprünge sind immer an der
> gleichen Stelle im Wertebereich. Ist meine 7-Segment Routine so OK

Ich hätt jetzt gesagt: ja

Schreibs mal so

void set_display_number(uint16_t number)
{
  if(number >= 999)
  {
    led_data[hundert] = segmente[9];
    led_data[zehn]    = segmente[9];
    led_data[eins]    = segmente[9];
  }
  else
  {
    led_data[eins] = segmente[ number % 10 ];
    number = number / 10;

    led_data[zehn] = segmente[ number % 10 ];
    number = number / 10;

    led_data[hundert] = segmente[ number ];
  }
}

[Stefan W. (dl6dx)]

Karl Heinz Buchegger schrieb:
> Irgendwo musst du dich da verrechnet haben.
> Das Ergebnis kann nur Werte im Bereich 0 bis 176 sein.

Ups. Ja. Und das hätte mir beim genauen Hinsehen auch auffallen müssen. 
(1023/1024 ist doch recht nahe an 1.) Ich war wohl doch schon zu müde.

Grüße

Stefan

[Philipp (Gast)]

Danke nochmal an euch! Mittlerweile funktioniert es sogar, obwohl ich 
nicht sagen kann an was das Problem jetzt genau lag. Ich lese den ADC 
jetzt einfach doch in einem extra Interrupt aus.

Gruß,
Philipp