Globale Variable im Interrupt u. Daten an PORT ausgeben?

[Arne S. (Firma: RDE) (gurax)]

Hallo ;)

Ich habe da 2 kleine Probleme... aber erst mal kurz der Sinn des 
Programms und meine HW:

Ich habe ein Crumb128 v3.0 Test board von chip45.com das ist ein 
AT90CAN128 mit einen CAN Treiber drauf über den ich CAN Messages sende 
und empfange. Über UART ist noch ein weiterer Chip angeschlossen zum 
abspielen von Sounddateien (MP3orator von avrchip.com). Je nach CAN 
Message sollen nun bestimmte Zeichenketten an den MP3orator gesendet 
werden oder Ports geschaltet werden.


1. Problem
Das leidige Thema der globalen Variablen in Interrupt Routinen.
Aber obwohl die Variablen vom Typ volatile char (Zeile 32) ist wird die 
änderung die ich in der Main mache nicht in der Interrupt Routine 
übernommen.

2. Problem
Wenn ich die CAN Message sende um den PORTC auf High zu setzen blinken 
die LEDs nur kurz auf und PORTC ist nicht mehr 0xFF wie ich es gesetzt 
habe sondern 0x00....

Ich hoffe ihr könnt mir bei meinen Problemen helfen ...

Und schon mal Danke im Vorraus!!!

[Peter (Gast)]

Ich kenne jetzt den AT90CAN128 nicht, aber in einer Interupt routinge 
sind eh alle Interrupts gesperrt als macht man soetwas nicht.,

ISR( SIG_INTERRUPT0 )
{
  ...
  cli();
  ...
  sei();
  ...
}

[Stefan B. (stefan)]

Das beisst sich schon:

const char* sound[] = 
  { 
    "PF TEST.WAV",    /* 0 */
    "PF BLUB.WAV",    /* 1 */         
    "PF BOING.WAV",   /* 2 */     
    "PF EXPLO1.WAV",  /* 3 */   
    "PF EXPLO3.WAV",  /* 4 */       
    "PF AAAAAGH.WAV", /* 5 */
    "PF YO.WAV"       /* 6 */
  };
...
  put_str( MP3, (char*)sound[interruptSound] );
...
  interruptSound = 0x19;

[Stefan B. (stefan)]

Arne Schroeder wrote:

> 2. Problem
> Wenn ich die CAN Message sende um den PORTC auf High zu setzen blinken
> die LEDs nur kurz auf und PORTC ist nicht mehr 0xFF wie ich es gesetzt
> habe sondern 0x00....

1. Du setzt in main() in der 4. Anweisung PORTC auf 0x00 nicht auf 0xFF

2. Vielleicht ändert auch folgende Funktion den Zustand von PORTC: 
can_send_message(). Man kann nicht reinsehen, weil sie im Quelltext 
fehlt...

[Arne S. (Firma: RDE) (gurax)]

Vielen Dank auf jedenfall schon mal für die Hinweise ...

Werd noch mal genau nachlesen ob der AT90CAN128 die Interrupts 
abschaltet oder nicht.

Und der Überlauf ist mir erst agrnicht aufgefallen ...  aber auch wenn 
ich dort 0x02 eintrage läst sich der Wert nicht ändern in der CAN 
Message steht dann immer 02 an der stelle auch wenn ich ihn über eine 
CAN Message geändert hab.

Als CAN Implementierung verwende ich die can-lib von kreatives-chaos.com 
[http://www.kreatives-chaos.com/artikel/universelle-can-bibliothek] und 
dort habe ich keine Zugriffe auf PORTC gefunden ...

ich hoffe es hat noch jemand eine Idee ...

[Johannes M. (johnny-m)]

Arne Schroeder wrote:
> Werd noch mal genau nachlesen ob der AT90CAN128 die Interrupts
> abschaltet oder nicht.
Alle AVRs setzen beim Sprung in den Interrupt-Vektor das I-Bit im 
Statusregister zurück und setzen es beim Rücksprung ins Hauptprogramm 
wieder!

[Oliver (Gast)]

>ich hoffe es hat noch jemand eine Idee ...

Wird die ISR überhaupt aufgerufen?

>Das leidige Thema der globalen Variablen in Interrupt Routinen.

ist kein leidiges Thema, sondern klar und einfach. Wird eine Variable im 
normalen Programm UND in einer ISR gelesen oder geschrieben, muß sie als 
volatile deklariert werden. Das ist alles. Deine Lösung ist richtig, das 
das Programm nicht funktioniert, liegt nicht daran.

Ansonsten ist die ganze Programmstruktur ziemlich verworren. Wartende 
while-Schliefen, putstr und dergleichen sollten in einer ISR eigentlich 
nie vorkommen. Und was die CAN-Senderoutine alles enthält, musst du 
selber wissen. Erkennen, was das Programm wann und warum machen soll, 
kann ich zumindest nicht.

Oliver

[Arne S. (Firma: RDE) (gurax)]

@ Oliver
Ja der Interrupt wird aufgerufen denn die CAN Message empfange ich die 
im Interrupt gesendet wird.

Also bist du der Meinung ich sollte im Interruot nur ein Flag setzen und 
das in der main-schleife abfragen und dort die gewünschte aktion 
ausführen ?


Ich werde es einfach mal ändern vielleicht beheben sich die Probleme 
dann ja ganz von alleine.

[Oliver (Gast)]

>Ich werde es einfach mal ändern vielleicht beheben sich die Probleme
>dann ja ganz von alleine.

Solange du kein Timing-Problem dadurch bekommst, daß die ISR sehr lange 
den normalen Programmablauf unterbricht, gibt es ja keine Probleme. Nur 
sollte du dir dessen ganz sicher sein, den so etwas zu debuggen ist 
übel.

Oliver

[Arne S. (Firma: RDE) (gurax)]

Habe das Problem gefunden ....

die beiden URAT Schnittstellen shceinen nicht richtig konfiguriert zu 
sein den sobald ich alle interrupts an schalte  benimmt er sich völlig 
seltsam  und wenn ich UART ganz weg lasse läuft es soweit normal !

aber danke für die Tips und Hilfen !!

[Marvin R. (mvn)]

Trotzdem möchte ich noch mal anmerken, was schon angemerkt wurd: In 
Interrupt-Service-Routinen Schleifen und Dergleichen einzubinden ist ein 
absolutes No-Go! Wie du schon selber richtig erkannt hast ist es 
bedeutend besser, wenn du in deiner ISR ein Flag setzt und dieses in der 
main() abfragst (nennt sich Polling). Noch schöner ist es, wenn du 
sobald die Abfrage true ergibt ein Unterprogramm aufrufst in der die 
tatsächliche Behandlung steht. Dies dient der Übersichtlichkeit und 
hilft ungemein bei der Fehlersuche.

Bsp.:

void Unterprogramm(void);  // Damit die main() diese Methoden
void InitABC(void);        // kennt obwohl sie erst später auftauchen

uint8_t gFlag; // globale Variable als Flag
uint8_t gZaehler;

void main(void)
{
  InitABC();   // nötige Initialisierungen
  sei();       // generelle Interrupts zulassen
  while(true)
  {
    if(gFlag = 1)
    {
      Unterprogramm();
      gFlag = 0;
    }
  }
  return 0;
}

void InitABC(void)
{
  gFlag = 0;
  gZaehler = 0;
  DDRC = 0xff  // als Bsp. Datenrichtungsregister wird initialisiert

       // hier müssten dann naütlich noch Initialisierungen für
       // den Interrupt rein
}

void Unterprogramm(void)
{
  gZaehler ++;      // Zähler eins hochzählen
  gZaehler %= 10;   // modulo 10, damit der nicht höher als 10 zählt
  PORTC = gZaehler; // an PORTC ausgeben
}

ISR(TIMER0_COMP_vect)
{
  gFlag = 1;  // Flag setzen damit die main() bescheid weiß
}

Um cli() und sei() musst du dir hier keine Sorgen machen. Diese werden 
durch Aufruf der ISR selbstständig getan. Die einzige ausnahme ist, das 
du aus irgendeinem Grund den Interrupt schon freigeben möchtest bevor 
die ISR fertig ist.

MfG Marvin

[Marvin R. (mvn)]

Achja was mir noch aufgefallen ist, du includest ein selbsterstelltes 
Headerfile. Wieso schreibst du dir nicht ein Headerfile, in der gleich 
sämtliche Schritte die für UART von nöten sind enthalten sind? Diese ist 
dann wiederverwendtbar, spart Zeit und es bleibt ebenfalls 
übersichtlicher.

Außerdem ist dein Formatierungsstil nicht gerade der schönste. ;)

Sachen wie:

UCSR0C = (0<<UMSEL0) | (0<<UPM00) | (0<<UPM01) | (0<<USBS0) | (1<<UCSZ00) | (1<<UCSZ01) | (0<<UCSZ02);

sind eher unübersichtlich und die Schreibweise verfehlt ihr Ziel.
Statt dessen würde ich lieber:

UCSR0C = 0x06; //set UCSZ00 & UCSZ01 high, other low.

schreiben und mit Kommentaren arbeiten.

Naja wollte nur mal versuchen die Übersichtlichkeit in der 
Programmierung zu vertreten. :)

MfG Marvin

[Johannes M. (johnny-m)]

Marvin Richter wrote:
> Achja was mir noch aufgefallen ist, du includest ein selbsterstelltes
> Headerfile. Wieso schreibst du dir nicht ein Headerfile, in der gleich
> sämtliche Schritte die für UART von nöten sind enthalten sind? Diese ist
> dann wiederverwendtbar, spart Zeit und es bleibt ebenfalls
> übersichtlicher.
In ein Headerfile gehören normalerweise keine Anweisungen, Nur 
Deklarationen. Eine Funktionsdefinition für eine UART-Initialisierung 
gehört wenn überhaupt in ein separates .c-File, für das dann wiederum 
eine Headerdatei existiert, die die nötigen Deklarationen enthält.

> UCSR0C = (0<<UMSEL0) | (0<<UPM00) | (0<<UPM01) | (0<<USBS0) |
> (1<<UCSZ00) | (1<<UCSZ01) | (0<<UCSZ02);
>

>
> sind eher unübersichtlich und die Schreibweise verfehlt ihr Ziel.
> Statt dessen würde ich lieber:
>
>

> UCSR0C = 0x06; //set UCSZ00 & UCSZ01 high, other low.
>

>
> schreiben und mit Kommentaren arbeiten.
Das ist Quatsch! Auf die Weise muss man bei Änderungen immer zwei 
Sachen ändern, nämlich den Code und den Kommentar! Das einzige, was an 
der obigen Schreibweise auszusetzen ist, sind die "0 << 
IRGENDWAS"-Ausdrücke, die man einfach weglassen sollte.

Wenn man die Möglichkeit hat, Code zu schreiben, der selbsterklärend ist 
(sich selbst kommentiert), dann sollte man das auch tun. Und die 
Scheibweise mit den Bitnamen ist selbsterklärend. Im Regelfall braucht 
ein erfahrener, mit der Controllerfamilie vertrauter Programmierer dann 
nicht mal mehr das Datenblatt zur Hand zu nehmen, um zu wissen was da 
passiert bzw. was evtl. falsch ist.

Bei Deiner Variante muss man immer noch im Datenblatt schauen, ob die 
Bits überhaupt an den richtigen Stellen sitzen. Und das ist Murks.

> Naja wollte nur mal versuchen die Übersichtlichkeit in der
> Programmierung zu vertreten. :)
Nö. Übersichtlich ist das nicht. Und erfahrungsgemäß passiert dabei 
genau das, was ich hier schon so oft gesehen habe: Code funktioniert 
nicht, lange Fehlersuche, sieht eigentlich alles richtig aus, bis man 
irgendwann die Stelle findet, an der Code und Kommentar nicht 
zusammenpassen und der betreffende sich auf den Kommentar verlassen 
hatte.

[Marvin R. (mvn)]

>In ein Headerfile gehören normalerweise keine Anweisungen, Nur
>Deklarationen. Eine Funktionsdefinition für eine UART-Initialisierung
>gehört wenn überhaupt in ein separates .c-File, für das dann wiederum
>eine Headerdatei existiert, die die nötigen Deklarationen enthält.

Du hast natürlich Recht. Ich habe mich lediglich verschrieben. Natürlich 
meinte ich auch ein separates .c-File.

Zu dem zweiten Punkt muss ich sagen, dass es erstens eine 
Geschmackssache ist und zweitens, sprichst du von erfahrenen 
Programmierern. Ein erfahrener Programmierer, benötigt meist weder 
Manual noch Definitionen sonder weiß, wenn da Register = 0x07 steht 
welche Bits dort gesetzt sind. Jedoch hast du auch Recht was den 
"selbsterklärenden Programmcode" angeht und muss meine Aussage teilweise 
zurücknehmen. ;)

Mein Statement bezüglich der Übersicht war auch gleichermaßen auf meinen 
Artikel davor bezogen.

Achja und was die Fehlersuche angeht möchte ich noch Flussdiagramme 
erwähnen. Jeder, der ein etwas komplexeres Programm schreiben möchte, 
sollte sich erstmal ein vernünftiges Flussdiagramm erstellen. Die 
meisten Denkfehler werden schon dort aufgedeckt und können umgangen 
werden.

dazu nochmal ein Link: http://de.wikipedia.org/wiki/Programmablaufplan

MfG

[Karl H. (kbuchegg)]

Marvin Richter wrote:

> Zu dem zweiten Punkt muss ich sagen, dass es erstens eine
> Geschmackssache ist

Das sehen die meisten hier nicht so.
Und die Performance hier im Forum gibt uns recht. In einer grossen Zahl 
von Fällen, in denen es Probleme bei einer Timerinitialisierung gab, die 
nicht so funktioniert hat wie gewünscht, war deine vielgepriesene Syntax 
mit Bit-Setzen über Hexzahlen (oder noch schlimmer: Dezimalzahlen) im 
Spiel. Mal wurde ein Vorteilerbit vergessen, mal das falsche Bit gesetzt 
und der falsche Interrupt ausgelöst, etc.

> und zweitens, sprichst du von erfahrenen
> Programmierern. Ein erfahrener Programmierer, benötigt meist weder
> Manual noch Definitionen sonder weiß, wenn da Register = 0x07 steht
> welche Bits dort gesetzt sind.

Das möchte ich ehrlich gesagt bezweifeln.
Ich weiss zwar, dass es im TIMSK Register ein Bit namens TOIE0 gibt 
(zumindest bei den meisten AVR, hab nicht alle gecheckt), ich hab aber 
keine Ahnung welches Bit das ist. Ich will es auch gar nicht wissen. 
Wenn ich

    TIMSK |= ( 1 << TOIE0 );

schreibe, wird es gesetzt. Egal welche Bitnummer das nun ist. Und das 
ist mir tausend mal lieber, als zu schreiben

    TIMSK |= 0x01;   // setze TOIE0 Bit

Schon alleine aus dem Grund, weil in einem Kommentar vieles stehen kann. 
Ob der Kommentar stimmt oder nicht, muss ich erst wieder mühsam 
überprüfen, indem ich die Bitbedeutung im Datenblatt nachschlage. Da 
ziehe ich die selbsterklärende Version von 'Setzt TOIE0 Bit im TIMSK 
Register' eindeutig vor. Die benötigt keinen Kommtar, ist schneller zu 
tippen und ich muss mir auch nicht merken welches Bit im Register denn 
nun das Gewünschte ist.

Das Einzige was mir noch zu meinem Glück fehlen würde, wäre eine 
Möglichkeit, wie der Compiler prüfen kann, ob es in TIMSK dieses Bit 
überhaupt gibt. Damit wären dann all die kleinen lästigen Problemchen, 
dass manchmal identische Bits in verschiedenen Prozessoren in 
unterschiedlichen Registern beheimatet sind, zumindest soweit 
entschärft, dass der Compiler sie finden kann.