itoa gibt fehlerhafte werte aus

OP #2569903
Lesenswert?

Hallo,
über einen Interrupt lese ich eine Variable ein, die in- oder 
dekrementiert werden kann (ja, ich weiß, INT verwendet man nicht für 
tasten)
1
ISR(INT5_vect)
2
{
3
k++;
4
}
5

6
ISR(INT6_vect)
7
{
8
k--;
9
}

In einer Dauerschleife frage ich diesen Wert ab
1
lcd_goto(1,13);
2
itoa(k,num,10);
3
lcd_write(num);

stdlib.h eingebunden, k als volatile int, num als char mit [4] 
definiert.
nur zeigt das Display immer falsche Werte an. zum beispiel 9891, auch 
negative Zahlen kommen vor. Initialisiert wird mit 0, also müsste der 
erste angezeigte Wert eine 1 sein. Setze ich k manuell auf einen fixen 
Wert, so funktioniert itoa() richtig. Kann zwischen Interrupt und 
Subroutine etwas verloren gehen?
Bin für Ratschläge dankbar!
Viele Grüße,
wastlB
#2569910
Lesenswert?

* zeig alles
* wenn du eh weißt, dass sich externe Interrupts nicht gut für
  Tasten eignen, warum tust du es dann
* wie sind die Interrupts konfiguriert?
  Hast du sie korrekt auf Flanke konfiguriert?
* Wenn du dein Array nur 4 Elemente groß machst, dann mag das zwar
  deiner Rechnung nach reichen, sicherheitshalber solltest du aber
  zumindest während der Entwicklung das Array so groß machen, dass
  der mit einem int größte mögliche int noch hineinpasst. Und das ist
  -32768. Das sind 6 angezeigte Stellen, also sollte das Array
  mindestens eine Länge von 7 haben
* Hast du daran gedacht, dass beim Überschreiben eines längeren
  Textes (zb "100") durch einen kürzeren Text (zb "99") die nicht
  überschriebenen Textteile weiterhin sichtbar sind?
#2569914
Lesenswert?

Hast du wirklich nur diesen Code und die Tasten direkt auf die Pins die 
den Interrupt triggern?
Dann muss man gar nicht weitersuchen, ohne Entprellen wird das nie was.

Tipp:
Schreib erst mal ein Programm das Testzahlen sauber aufs Display 
schreibt damit du WIRKLICH SICHER sein kannst das das Schreiben 
funktioniert.

Dann machst du eine saubere Entprellung. Alles andere wird Gefriemel das 
dir nicht wirklich nützt.
Moderator Persönliche Seite #2569918
Lesenswert?

Auch bei der Auswertung von k aufpassen. Wenn das ein Int16 ist, dann 
kann Dir die ISR z.B. das Lowbyte überschreiben, während Du das Highbyte 
auswertest (bzw. hier den Wert an itoa übergibst).

Also: entweder beim Aufruf von itoa mit gesperrtem Interrupt arbeiten 
oder besser eigene Funktionen  implementieren, die auf jeden Fall 
saubere Werte der "Interruptveränderlichen" zurückliefern.

Chris D.
OP #2569923
Lesenswert?

Tastensetup, entprellt über RC Tiefpass, getriggert nach erkannter 
Low-Flanke.
Nach Aufruf der init_port() Routine werden die Interrupts mit sei(); 
gestartet.
1
void init_port()
2
{  
3
DDRE  |= (0<<PE5)|(0<<PE6)|(0<<PE7);    //Ports für Taster
4
PORTE |= (1<<PE5)|(1<<PE6)|(1<<PE7);    
5

6
EICRB |= (0<<ISC71)|(0<<ISC70)|(0<<ISC61)|(0<<ISC60)|(0<<ISC51)|(0<<ISC50);
7
EIMSK |= (1<<INT7)|(1<<INT6)|(1<<INT5);
8
}

Display löschen ist keine Option, da sich bereits Informationen auf dem 
Display (2x16) befinden.
Ausserdem wird das Feld ja mit goto(x,y) immer wieder ab der gleichen 
Stelle beschrieben.

Das Problem des Überschreibens ist mir bewusst, in Zukunft sollen aber 
nur Zahlen von 1-5 angezeigt werden. (mit if-Bedingung wird dann ein 
Überschreiten des Wertes keine erneute Inkrementierung zur Folge haben)
#2569929
Lesenswert?

Wastl B. schrieb:
> Tastensetup, entprellt über RC Tiefpass, getriggert nach erkannter
> Low-Flanke.
> Nach Aufruf der init_port() Routine werden die Interrupts mit sei();
> gestartet.

Weißt du was:

Beschreib nicht deine Code... Zeig ihn!
Und zwar nicht in homoöpathischen Dosen sondern alles auf einmal. Soviel 
wirds ja wohl nicht sein. Und wenn doch, dann speck deinen Code aufs 
wesentliche ab, stell sicher, dass er immer noch das gleiche Verhalten 
zeigt und poste das.

Aber viele hier haben einfach keine Lust mehr sich aus Bruchstücken 
gedanklich deinen Code zusammenzusetzen und die Teile die du nicht 
zeigst zu erfinden. Du siehst in dem geposteten keinen Fehler, wir haben 
ein paar Ideen was falsch sein könnte, können es aber nicht überprüfen, 
weil du es nicht herzeigst. Wie soll man da vernünftig arbeiten?

> Display löschen ist keine Option, da sich bereits Informationen auf dem
> Display (2x16) befinden.

Nichts desto trotz musst du dich darum kümmern, dass beim Überschreiben 
alles weggenommen wird.

> Ausserdem wird das Feld ja mit goto(x,y) immer wieder ab der gleichen
> Stelle beschrieben.

Und was hilft dir das?

Um Display steht

   +---+---+---+---+
   | 1 | 0 | 0 |   |
   +---+---+---+---+

die nächste auszugebende Zahl ist 99. Also schreibst du drüber

   +---+---+---+---+
   | 9 | 9 | 0 |   |
   +---+---+---+---+

und dein Benutzer liest 990 ab, weil die letzte 0 nie überschrieben 
wurde.
OP #2569947
Lesenswert?

Zu dem beschreiben des Displays: klar, wenn 100 initial da steht, daß 
beim nächsten mal 200 steht. Aber immerhin die erste Stelle muss richtig 
gezeigt werden, und das ist ja schon nicht mehr der Fall.

hier meine main.c, in der init.h werden alle setups gemacht
1
#include <stdio.h>
2
#include <stdlib.h>
3
#include <avr/io.h>
4
#include <avr/interrupt.h>
5
#include <avr/init.h>
6

7
void get_menu()
8
{
9
if(key==1)
10
  {
11
  lcd_clear();
12
  lcd_goto(1,1);
13
  lcd_write("Programm: PG");
14
  m_state=1;
15
  key=0;
16
}
17
if(m_state==1)
18
  {
19
      m_state=2;
20
      lcd_goto(1,13);
21
      itoa(k,num,10);
22
      lcd_write(num);
23
      m_state=1;
24
  }
25
}
26

27
int main(void)
28
  {
29
    init_port();
30
    init_timer();
31
    init_pwm();
32
    sei();             //SREG I=1
33
    init_display();
34
    wait_ms(10);
35
    lcd_goto(1,1);
36
    lcd_write("CapnoCheck V3.1");
37
    lcd_goto(2,1);
38
    lcd_write("HW3.0 SW3.1");
39
    while(1)
40
      {
41
      get_menu();
42
      }
43
  
44
    return 0;
45
  }
46

47

48
//Interrupt-Routinen
49

50
ISR(TIMER0_COMP_vect)
51
{
52
ms++;
53
}
54

55
ISR(INT5_vect)
56
{
57
if(m_state==1)
58
{
59
k++;
60
}
61
}
62

63
ISR(INT6_vect)
64
{
65
if(m_state==1)
66
{
67
k--;
68
}
69
}
70

71
ISR(INT7_vect)
72
{
73
if(key==0)
74
{
75
key=1;
76
}
77
}

und die init.h
1
//Initialisierung  Variablen
2
volatile int ms, t_state=0, b_state=0, key=0, m_state=0, m=0, k=0;
3
char num[3];
4

5
//Initialisierung Timer
6
void init_timer()
7
{
8
//Timer 0
9
TCCR0 |= (1<<WGM01)|(0<<WGM00)|(0<<COM01)|(0<<COM00)|(0<<CS02)|(1<<CS01)|(1<<CS00);
10
TIMSK |= (1<<OCIE0);
11
TIFR |= (1<<OCF0);
12
OCR0 = 112;          //1ms bei  clk/64
13
}
14

15
//Initialisierung PWM
16
void init_pwm()
17
{
18
//PWM an PB4 (OC0)
19
TCCR0 |= (1<<WGM01)|(1<<WGM00)|(1<<COM01)|(0<<COM00)|(0<<CS02)|(0<<CS01)|(1<<CS00);
20
OCR0 = 0;
21

22
//PWM an PB5 (OC1A)
23
TCCR1A|= (0<<WGM11)|(1<<WGM10)|(1<<COM1A1)|(0<<COM1A0)|(0<<CS12)|(0<<CS11)|(1<<CS10);
24
OCR1A = 0;
25

26
//PWM an PB6 (OC1B)
27
TCCR1A|= (1<<COM1B1)|(0<<COM1B0);
28
TCCR1B|=(0<<WGM13)|(1<<WGM12)|(1<<WGM10)|(0<<CS12)|(0<<CS11)|(1<<CS10);
29
OCR1B = 0;
30

31
//PWM an PB7 (OC2)
32
TCCR2 |= (1<<WGM21)|(1<<WGM20)|(1<<COM21)|(0<<COM20)|(0<<CS22)|(0<<CS21)|(1<<CS20);
33
OCR2 = 0;
34
}
35

36
//Initialisierung Ports
37
void init_port()
38
{
39
DDRB  |= (1<<PB4)|(1<<PB5)|(1<<PB6)|(1<<PB7);//Ports für PWM-Ventile
40
PORTB |= (1<<PB4)|(1<<PB5)|(1<<PB6)|(1<<PB7);            
41

42
//DDRD  |= (1<<PD4)|(1<<PD5);          
43
//PORTD |= (1<<PD4)|(0<<PD5);          //LCD Backlight and LED
44
  
45
DDRE  |= (0<<PE5)|(0<<PE6)|(0<<PE7);    //Ports für Taster
46
PORTE |= (1<<PE5)|(1<<PE6)|(1<<PE7);    
47

48
EICRB |= (0<<ISC71)|(0<<ISC70)|(0<<ISC61)|(0<<ISC60)|(0<<ISC51)|(0<<ISC50);
49
EIMSK |= (1<<INT7)|(1<<INT6)|(1<<INT5);
50
}
51

52
void wait_ms(int wait)
53
{
54
  if(t_state==0)
55
  {
56
  t_state=1;
57
  do
58
  {
59
  }
60
  while(ms<wait);
61
  t_state=0;
62
  ms=0;
63
  }
64
}

das Programm ist momentan ein Draft, es wird also kein Anspruch auf 
Sinnhaftigkeit aller Routinen und Variablen erhoben. Es geht hier 
lediglich um die Funktion itoa() und deren Nicht-funktionieren.

edit: es handelt sich um einen atmega128a
#2569964
Lesenswert?

Wastl B. schrieb:
> Aber immerhin die erste Stelle muss richtig
> gezeigt werden, und das ist ja schon nicht mehr der Fall.

In dem Moment, in dem du dein char-Array überläufst, aus welchem Grund 
auch immer (und sei es nur ein anderer Programmfehler), sind alle 
Lichter aus. Alles mögliche kann passieren. Das bedeutet aber auch, dass 
du Fehlern nachläufst, die eigentlich Folgefehler eines anderen Fehlers 
sind. -32768 passt nicht in ein char[4], egal wie du es drehst und 
wendest.
#2569968
Lesenswert?

Wastl B. schrieb:
> sorry, falsche zeile. ich wollte "The low level of INTn generates an
> interrupt request." erreichen, das geht wenn beide ISCxx auf 0 sind.

Und was bringt dir das?
Du drückst eine Taste, die erzeugt einen Low-Level und der INterrupt 
wird mit einem Affenzahn viele zehn-tausend mal in der Sekunde 
aufgerufen. Nicht wirklich das, was man sich als Benutzer so vorstellt.

Low Level bedeutet: SOLANGE der Pin auf Low IST, wird der Interrupt 
ausgelöst. Wieder und immer wieder.
Gast #2569972
Lesenswert?

>   EICRB |= (0<<ISC71)|(0<<ISC70)|(0<<ISC61)|(0<<ISC60)|(0<<ISC51)|(0<<ISC50);

angenommen EICRB steht auf 01010101 und du willst 01000000 erhalten
dann bringt es nichts eine 0 zu shiften und das dann mit 01010101 zu 
ver-odern. 01010101 oder 00000000 ist immer noch 01010101. Löschen von 
Bits geht anders.
OP #2569978
Lesenswert?

Habe jetzt
EICRB |= 
(1<<ISC71)|(0<<ISC70)|(1<<ISC61)|(0<<ISC60)|(1<<ISC51)|(0<<ISC50);
gesetzt, nun habe ich wenigstens auf- und absteigende Zahlen. Auch 
richtige. natürlich habe ich jetzt das von kbuchegg angesprochene 
Problem. Wie stelle ich sicher, daß der Interrupt nur einmal ausgelöst 
wird, bis die Taste wieder losgelassen wurde?
Ich kann doch nicht alles mit irgendwelchen Statusvariablen 
zupflastern...
Gast #2569989
Lesenswert?

Wastl B. schrieb:
> EICRB |=
> (1<<ISC71)|(0<<ISC70)|(1<<ISC61)|(0<<ISC60)|(1<<ISC51)|(0<<ISC50);
> gesetzt, nun habe ich wenigstens auf- und absteigende Zahlen. Auch
> richtige. natürlich habe ich jetzt das von kbuchegg angesprochene
> Problem. Wie stelle ich sicher, daß der Interrupt nur einmal ausgelöst
> wird, bis die Taste wieder losgelassen wurde?
> Ich kann doch nicht alles mit irgendwelchen Statusvariablen
> zupflastern...

Nochmal, eine 0 zu shiften löscht nicht das Bit. Es ändert einfach gar 
nichts in der Verbindung mit dem ODER. Löschen macht man mit UND.
z.B.
1
PORTB &= ~(1<<PB2); /* löscht Bit 2 in PORTB und setzt damit Pin PB2 auf low */
#2569999
Lesenswert?

AVR schrieb im Beitrag #2569989:

> Nochmal, eine 0 zu shiften löscht nicht das Bit. Es ändert einfach gar
> nichts in der Verbindung mit dem ODER. Löschen macht man mit UND.

Prinzipiell alles richtig.
Allerdings kannst du nach einem Reset davon ausgehen, dass das Register 
schon auf 0 steht.
#2570000
Lesenswert?

Wastl B. schrieb:

> Ich kann doch nicht alles mit irgendwelchen Statusvariablen
> zupflastern...

Womit wir wieder bei der allerersten Aussage wären:
Taster werden sinnvoller Weise nicht mittels externem Interrupt 
angebunden.

> Wie stelle ich sicher, daß der Interrupt nur einmal ausgelöst
> wird, bis die Taste wieder losgelassen wurde?

Wenn du den Interrupt auf Flanke stellst, dann wird er auch nur einmal 
ausgelöst. Wenn er es doch mehrmals tut, dann ist deine Entprellung mit 
RC-Glied Müll.
Womit wir wieder beim ersten Punkt wären.


Du kannst es drehen und wenden wie du willst: Tastenauswertung mittels 
externem Interrupt ist nicht angebracht. Es gibt nur einen Fall, in dem 
das Sinn macht: Wenn der Interrupt den µC aus dem Tiefschlaf holen muss. 
Und selbst dort lässt man den µC nur aufwachen und wertet dann die 
Tasten wie gewohnt mittels Polling und Software-Entprellung aus.
Gast #2570006
Lesenswert?

Karl Heinz Buchegger schrieb:
> Allerdings kannst du nach einem Reset davon ausgehen, dass das Register
> schon auf 0 steht.

Sollte man sich dennoch nicht angewöhnen. Es macht einfach nicht das, 
was er erwartet. An solchen Sachen sitzt man als Anfänger schon mal ein 
paar Stunden und fragt sich warum es nicht klappt. Das setzten und 
löschen von Bits sollte schon sitzen. Damit fängt man eigentlich an, 
bevor man sich an Timer wagt. Ist auch nur ein gut gemeinter Rat.
#2570011
Lesenswert?

AVR schrieb im Beitrag #2570006:
> Karl Heinz Buchegger schrieb:
>> Allerdings kannst du nach einem Reset davon ausgehen, dass das Register
>> schon auf 0 steht.
>
> Sollte man sich dennoch nicht angewöhnen. Es macht einfach nicht das,
> was er erwartet.

Schon klar.
Viele machen das allerdings um sich in der Initialisierungsphase die 
Bits hinzuschreiben und zu dokumentieren, dass sie auf 0 bleiben.

(Allerdings macht man dann meistens eine Zuweisung und keinen |=. Der 
verwirrt dann)

> bevor man sich an Timer wagt. Ist auch nur ein gut gemeinter Rat.

Schon klar. Ist auch richtig so. Nur ist das in dem Fall nicht sein 
Problem.
Gast #2570089
Lesenswert?

RC-Entprelung ist irgendwie elektrische Steinzeit.
Bei einem Bastler-Einzelstück ist das noch wurscht, wenn auch nicht 
schön. Aber schon beim zweiten Aufbau wird das dann zu viel Kram.
Mach doch in Software, was Du in Software machen kannst. Je weniger 
Hardware umso besser.

Antwort schreiben

Bitte melde dich an, um einen Beitrag zu schreiben.

oder

Mit Google-Account einloggen

Die Registrierung ist kostenlos und dauert nur eine Minute.

Jetzt registrieren