Datum:
Guten Morgen, fange gerade an mich mit C auf dem AVR zu beschäftigen. Folgendes Problem habe ich mit dem Code im Anhang. Es wird die Variable dcc_daten[] in der ISR und in main verwendet. Sobald ich den GCC den Code optimieren lasse, ist der Inhalt von dcc_daten[] nur noch in der ISR selbst korrekt. In main lese ich nur noch "Müll" aus der Variablen. Was mache ich falsch?
#ifndef F_CPU #define F_CPU 8000000 #endif #define BAUD 19200 #include <avr/io.h> #include <stdint.h> //#include <util/delay.h> #include <util/setbaud.h> #include <avr/interrupt.h> #define L_FIFO 32 // größe des fifo tx buffer volatile uint8_t fifo[(L_FIFO+3)]; // fifo (+3) //zum DCC einlesen #define DCCport PIND #define DCCpin PIND4 static volatile uint8_t Lowpegel = 1; static volatile uint8_t Highpegel = 1; static volatile uint8_t EmpPos; #define Preamble 0 static volatile uint8_t Bitzaehler = 0; static volatile uint8_t dcc_in = 0; static volatile uint8_t dcc_xor = 0; static volatile uint8_t dcc_daten[6]; static volatile struct { unsigned neuerBefehl:1; // 1 Bit für neuerBefehl unsigned keinDCC:1; // 1 Bit für keinDCC unsigned bit:1; // 1 Bit für bit } Flag; //####################################### //sende Byte über UART-FIFO uint8_t UART_FIFO( uint8_t data ) //####################################### { if( UCSRA & ((1<<UDRE)|(1<<TXC))){ // werden keine Daten übertragen und ist UDR bereit für neue Daten TCCR1A |= (1<<COM1A1); // Träger einschalten (OCR Pin verbinden) UDR = data; return(1); // i.o. daten werden gesendet } else{ // es werden schon Daten übertragen -> neue Daten in FIFO uint8_t fifo_anzahl = fifo[0]; if (fifo_anzahl == (L_FIFO-1)){ // ist fifo voll return(0); // Fehler fifo ist voll } else{ fifo_anzahl++; fifo[0] = fifo_anzahl; uint8_t fifo_wp = fifo[2]; fifo[(fifo_wp+3)] = data; fifo_wp++; if(fifo_wp == L_FIFO){ fifo_wp = 0; } fifo[2] = fifo_wp; return(1); } } } //#################################################### //int0 wird bei steigender Flanke ausgelöst ISR(INT0_vect) //#################################################### { TCCR1A &= ~(1<<COM1A1); // Träger ausschalten (OCR Pin trennen) } //#################################################### //int1 wird bei fallender Flanke ausgelöst ISR(INT1_vect) //#################################################### { TCCR1A |= (1<<COM1A1); // Träger einschalten (OCR Pin verbinden) } //#################################################### //TXC Interrupt wird ausgelöst bei TX fertig //kommt erst bei TX fertig, nicht schon bei UDR //bereit für neue Daten ISR(USART_TX_vect) //#################################################### { uint8_t fifo_anzahl = fifo[0]; if (fifo_anzahl == 0){ // fifo ist leer TCCR1A &= ~(1<<COM1A1); // Träger ausschalten (OCR Pin trennen) } else{ fifo_anzahl--; fifo[0] = fifo_anzahl; uint8_t fifo_rp = fifo[1]; uint8_t data = fifo[(fifo_rp+3)]; UDR = data; fifo_rp++; if(fifo_rp == L_FIFO){ fifo_rp = 0; } fifo[1] = fifo_rp; } } //#################################################### //Timer 0 CTC CompA //wird alle 10us ausgelöst liest bit vom DCCpin ein // ISR(TIMER0_COMPA_vect) //#################################################### { if ( DCCport & (1<<DCCpin) ) { // ist DCCpin High Highpegel++; // inc Highpegel if (Highpegel == 0){ // wenn Überlauf Flag.keinDCC = 1; // kein DCC setzen Highpegel = 1; Lowpegel = 12; return; // und fertig } } else{ Lowpegel++; // wenn DCCpin Low inc Lowpegel if (Lowpegel == 0){ Flag.keinDCC = 1; // bei Überlauf wieder fertig Highpegel = 1; Lowpegel = 12; return; } } if (Lowpegel == 4) Highpegel = 1; // beim 3.mal (3+1) Lowpegel Highpegelreset machen if (Highpegel != 4) return; // wenn nicht 3x (3+1) Highpegel war fertig if (Lowpegel < 8) Flag.bit = 1; // wenn Lowpegel kleiner 8 neues bit high else Flag.bit = 0; // sonst neues bit low Lowpegel = 1; // Reset Lowpegel Highpegel++; // 1x erhöhen, damit nicht noch ein Bit erkannt wird if (EmpPos == Preamble){ // Preamble if (Flag.bit == 1) Bitzaehler++; // empf.Bit high -> erhöhe Bitzähler else { // empf.Bit low if (Flag.neuerBefehl == 1) { // wenn letzter Befehl noch nicht bearbeitet neuen verwerfen Bitzaehler = 0; return; } if (Bitzaehler >= 10) EmpPos++; // und mindestens 10 high empfangen -> schalte auf Byte A lesen um Bitzaehler = 0; // und lösche Bitzähler wieder dcc_xor = 0; } return; } else { // DCC Daten einlesen if (Bitzaehler < 8){ // von 0-7 bits empfangen dcc_in += dcc_in; // neues bit einschieben if( Flag.bit == 1 ) dcc_in++; Bitzaehler++; // bitzähler auf nächstes bit } else { // wenn 8 bits empfangen dcc_xor ^= dcc_in; // xor zur Fehlererkennung dcc_daten[(EmpPos-1)] = dcc_in; if (Flag.bit == 0){ // kam 0 -> Trennbit EmpPos++; // auf nächstes Byte umschalten Bitzaehler = 0; // und bitzähler reset } else { // kam 1 -> Endbit if ((EmpPos >= 2) && (dcc_xor == 0)){ // nachsehen ob mindest 3 Byte empfangen wurden und xor = 0 ist Flag.neuerBefehl = 1; // wenn ja flag für neue Daten setzten Flag.keinDCC = 0; // flag kein dcc löschen } EmpPos = Preamble; // DCC Schleife auf start Bitzaehler = 0; } } } return; } //################################################################################################## //Hauptprogramm int main (void) //################################################################################################## { //PORTS Initalisierung DDRA = 0b00000000; // 1=Ausgang/0=Eingang DDRB = 0b00001000; DDRD = 0b00000010; PORTA = 0; // alle Ausgänge auf Low PORTB = 0; PORTD = 0; //UART Initalisierung UBRRH = UBRRH_VALUE; // UBR_Wert aus setbaud.h UBRRL = UBRRL_VALUE; #if USE_2X // U2X-Modus erforderlich UCSRA |= (1 << U2X); #else // U2X-Modus nicht erforderlich UCSRA &= ~(1 << U2X); #endif UCSRB = (1<<TXCIE)|(1<<TXEN); // UART TX + TX fertig int einschalten UCSRC = (1<<UCSZ1)|(1<<UCSZ0); // Asynchron 8N1 //init_fifo fifo[0] = 0; fifo[1] = 0; fifo[2] = 0; //Timer1 init erzeugt den IR Träger PWM PhaseCorrect mit clk/1 top = ICR1 TCCR1A = (1<<WGM11); // Clear OC1A on Compare Match (Set output to low level) TCCR1B = (1<<WGM13)|(1<<CS10); // TCCR1A (1<<COM1A1) wird später gesetzt TCCR1C = 0; ICR1H = 0; // Timer max auf 8,8 (8MHz/455KHz)/2 ICR1L = 9; // 455KHz = 2,2µs 8MHZ = 0,125µs *2 PhaseCorrect = 0,25µs/Timerschritt -> 8,8 OCR1AH = 0; // Timer Pulsbreite auf (0,2-1,1 us) kurzer Puls mit max Strom OCR1AL = 1; // 1* 0,25 //Ext Int init MCUCR = (1<<ISC11)|(1<<ISC01)|(1<<ISC00); // steigende Flanke INT0 + fallende Flanke INT1 erzeugt Interrupt GIMSK = (1<<INT1)|(1<<INT0); // INT0+INT1 Interrupt enable //Timer0 init alle 10us ein CTC interrupt TCCR0A = (1<<WGM01); // CTC OCR0A = Top TCCR0B = (1<<CS00); // clk/(No prescaling) TCNT0 = 0; OCR0A = 80; OCR0B = 0; TIMSK |= (1<<OCIE0A); // Timer/Counter0 Output Compare Match A Interrupt Enable TIFR |= (1<<OCF0A); // Flag löschen sei(); UART_FIFO('S'); UART_FIFO('t'); UART_FIFO('a'); UART_FIFO('r'); UART_FIFO('t'); while(1) { // Hauptschleife if (Flag.neuerBefehl == 1){ UART_FIFO(dcc_daten[0]); UART_FIFO(dcc_daten[1]); UART_FIFO(dcc_daten[2]); UART_FIFO(dcc_daten[3]); UART_FIFO(dcc_daten[4]); UART_FIFO(dcc_daten[5]); UART_FIFO(13); Flag.neuerBefehl = 0; } } } |
Datum:
Ohne einen konkreten Verdacht bezüglich dcc_daten[] zu haben, hier etwas anderes: Ist dir bewusst dass der Zugriff auf die einzelnen Bits von Flag nicht atomar ist? Die komplette struct Flag wird vom Compiler in eine einzelne (vermutlich sizeof (char) große?) Variable gepackt. Wenn du sachen wie
Flag.neuerBefehl = 0;
|
Schreibst, dann macht der Compiler folgendes: 1. Er liest die Variable mit allen Bits 2. Maskiert das Bit neuerBefehl 3. Schreibt diesen Wert zurück in die Flag Variable Wenn zwischen diesen 3 Schritten die ISR aufgerufen wird, und dabei Flag verändert, dann überschreibt main() diese änderung der Flag Variable sobald die ISR beendet wird. Vielleicht ist dein dcc_daten[] Problem ja nur ein Symptom dessen.
Datum:
Danke. Wie gesagt ist meine erste C Übung. Das ganze Projekt läuft schon mit 760 Byte in ASM und ich dachte mir, das ist genau die richtige Größe um das mal zur Übung in C umzusetzen. Mit der Bit Variablen hast Du natürlich recht, das könnte schief gehen muss ich überdenken (cli/sei einfügen). Wenn die ISR da zwischen funkt, passiert aber auch nichts, da es in der ISR nur wieder gesetzt wird, wenn es vorher erfolgreich gelöscht wurde. Ansonsten werden die neuen Daten einfach verworfen. dcc_daten[] wird in der ISR auch nur geändert, wenn das Flag.neuerBefehl erfolgreich auf Null gesetzt wurde. Das sollte auch nicht mein Fehler sein. Ich denke eher, ich habe die Variablen nicht richtig C -konform deklariert, so das der Compiler mir da etwas wegoptimiert. Wie gesagt ohne Optimierung geht das so (mit 1300 Byte) mit Optimierung (egal welche) werden es dann ca. 800 Byte aber der Inhalt von dcc_daten[] ist nur noch in der ISR richtig.
Datum:
Toralf Wilhelm schrieb: > um das mal zur Übung in C umzusetzen. Mit der Bit Variablen hast Du > natürlich recht, das könnte schief gehen muss ich überdenken (cli/sei > einfügen). Wenn die ISR da zwischen funkt, passiert aber auch nichts, da > es in der ISR nur wieder gesetzt wird, Ähm. Es geht NICHT um das Flag.NeuerBefehl es geht um die anderen Flags in dieser Struktur! > Das sollte auch nicht mein Fehler sein. Ich denke eher, ich habe die > Variablen nicht richtig C -konform deklariert, Beim Drüberschauen ist mir nichts aufgefallen. Wie hast du festgestellt, dass die Werte in der ISR noch in Ordnung sind?
Datum:
Deine FIFO Routinen sind IMHO fehlerhaft. Die Manipulation von fifo_anzahl ist nicht interruptfest. Am besten wirst du diese Anzahl überhaupt los. Du brauchst sie nicht. Durch den Write und den Read Pointer kannst du immer eine Aussage darüber treffen ob der FIFO voll ist bzw. die Anzahl errechnen, wenn diese gebraucht werden würde. (und über die Technik, im Array 2 Einträge auf die harte Tour zu zweckentfremden, breiten wir jetzt erst mal den Mantel des Schweigens) In Summe denke ich, dass du zuviel Code auf einmal geschrieben hast. Da jetzt den entscheidenden Fehler zu finden, wird ziemlich schwierig. Zumindest mir ist da jetzt nichts offensichtliches aufgefallen.
Datum:
Stimmen die Komentare?
//Timer 0 CTC CompA //wird alle 10us ausgelöst liest bit vom DCCpin ein // ISR(TIMER0_COMPA_vect) |
10µs ist schon ganz schön sportlich. Bei 8Mhz sind das 80 Takte von einer ISR zur nächsten.
Datum:
if( UCSRA & ((1<<UDRE)|(1<<TXC))){ // werden keine Daten übertragen und ist UDR bereit für neue Daten |
Hier passt der Kommentar nicht zum Code. Die Bedingung ist auch wahr, wenn nur eines der Bits (UDRE oder TXC) 1 ist. Das Testen des TXC-Flags macht eh keinen Sinn. Du benutzt den TXC-Interrupt, der das Flag zurücksetzt, also wird es an dieser Stelle praktisch nie 1 sein.
Datum:
Karl Heinz Buchegger schrieb: > Ähm. Es geht NICHT um das > > Flag.NeuerBefehl > > es geht um die anderen Flags in dieser Struktur! ok, das ist wohl war, ich werde das ändern. > Deine FIFO Routinen sind IMHO fehlerhaft. Die Manipulation von > fifo_anzahl ist nicht interruptfest. Am besten wirst du diese Anzahl > überhaupt los. Du brauchst sie nicht. Durch den Write und den Read > Pointer kannst du immer eine Aussage darüber treffen ob der FIFO voll > ist bzw. die Anzahl errechnen, wenn diese gebraucht werden würde. ich wollte mir die eine Rechenoperation sparen, werde ich ändern. Soll ich die beiden Zeiger besser extra anlegen? fifo_anzahl könnte auch mein Problem sein, weil den Test das die Daten in der ISR stimmen, habe ich natürlich gemacht, in dem ich mir in der ISR die Daten in den fifo geschrieben habe und da kann er ja nicht mehr unterbrochen werden. alle 10us/80 Tackte eine ISR ist richtig, ich weiß, das das sportlich ist, aber es ist nur jede 12. oder 24. ISR länger (je nach empfangenden Daten) die anderen sind nur ein Paar Takte lang. Stefan Ernst schrieb : > Hier passt der Kommentar nicht zum Code. Die Bedingung ist auch wahr, > wenn nur eines der Bits (UDRE oder TXC) 1 ist. Das Testen des TXC-Flags > macht eh keinen Sinn. Du benutzt den TXC-Interrupt, der das Flag > zurücksetzt, also wird es an dieser Stelle praktisch nie 1 sein. Das stimmt, muss ich noch anpassen, das stammt noch aus dem FIFO Test ohne ISR. Wobei das oder sowie so falsch war, sollte schon & sein. Ich werde das alles mal ändern. Dank erst einmal Willi
Datum:
Toralf Wilhelm schrieb: > fifo_anzahl könnte auch mein Problem sein, weil den Test das die Daten > in der ISR stimmen, habe ich natürlich gemacht, in dem ich mir in der > ISR die Daten in den fifo geschrieben habe und da kann er ja nicht mehr > unterbrochen werden. Kannst du ja auf die Schnelle feststellen. Kapsle den Inhalt der UART_FIFO in cli() und sei(). Dann hast du schon mal einen ersten Anhaltspunkt, ob die FIFO da was damit zu tun hat.
Datum:
Toralf Wilhelm schrieb: > Wobei das oder sowie so falsch war, sollte schon & sein. Ne, wäre auch falsch. Wenn du das | durch & ersetzt, dann ist die Bedingung immer false.
Datum:
Karl Heinz Buchegger schrieb: > Toralf Wilhelm schrieb: > > >> fifo_anzahl könnte auch mein Problem sein, weil den Test das die Daten >> in der ISR stimmen, habe ich natürlich gemacht, in dem ich mir in der >> ISR die Daten in den fifo geschrieben habe und da kann er ja nicht mehr >> unterbrochen werden. > > Kannst du ja auf die Schnelle feststellen. > Kapsle den Inhalt der UART_FIFO in cli() und sei(). Dann hast du schon > mal einen ersten Anhaltspunkt, ob die FIFO da was damit zu tun hat. mache ich, komme aber erst heute Abend dazu. Stefan Ernst schrieb: > Ne, wäre auch falsch. Wenn du das | durch & ersetzt, dann ist die > Bedingung immer false. ja hast Du recht, habe ich mich doof ausgedrückt, das stammt noch aus der Testphase, ohne TXC ISR und da war es wichtig das nicht nur UDR frei war sondern auch TX fertig war, weil nur dann durfte der 455KHz Träger für die IR Übertragung abgeschaltet werden. Jetzt nutze ich ja die TXC ISR und das bedeutet das UDR dann eh schon für neue Daten bereit ist und wenn keine neuen Daten mehr im FiFo sind auch der Träger abgeschaltet werden kann (weil TXC). Ohne ISR hatte ich zum senden nur auf UDR frei getestet und da musste ich noch warten bis TX fertig ist, bevor ich den Träger abschalten konnte.
Datum:
Karl Heinz Buchegger schrieb: > (und über die Technik, im Array 2 Einträge auf die harte Tour zu > zweckentfremden, breiten wir jetzt erst mal den Mantel des Schweigens) noch einmal dazu eine Frage, ich habe den FIFO mehr oder weniger aus meinem ASM Code portiert. Da ist es einfacher bzw. schneller die Anzahl in einer dritten Variable abzulegen. Die ist schon überflüssig, man erhält die auch aus den beiden lese/schreib Pointer. Sollte ich diese besser so anlegen:
volatile uint8_t fifo[(FIFO_Groeße)]; static volatile uint8_t fifo_write_Pointer; static volatile uint8_t fifo_read_Pointer; |
Datum:
Toralf Wilhelm schrieb: > Karl Heinz Buchegger schrieb: > >> (und über die Technik, im Array 2 Einträge auf die harte Tour zu >> zweckentfremden, breiten wir jetzt erst mal den Mantel des Schweigens) > > noch einmal dazu eine Frage, ich habe den FIFO mehr oder weniger aus > meinem ASM Code portiert. Da ist es einfacher bzw. schneller die Anzahl > in einer dritten Variable abzulegen. Die ist schon überflüssig, man > erhält die auch aus den beiden lese/schreib Pointer. Sollte ich diese > besser so anlegen: Du sollst in deinem C Buch über Strukturen nachlesen. Das ist dein 'Werkzeug', wie du dir eine Datenstruktur machen kannst, die heterogen ist und in der du die Dinge zusammenfasst, die zusammengehören.
struct FIFO { uint8_t Anzahl; uint8_t ReadPtr; uint8_t WritePtr; uint8_t Data[L_FIFO]; }; volatile struct FIFO myFifo; |
Datum:
Danke
Datum:
Versuche mal ob es sich anders verhält, wenn Du alle Variabeln nur "volatile" ohne "static" deklarierst. p.s. Du solltest nur die Variabeln als Volatile deklarieren, welche Du in der ISR und auch ausserhalb der ISR verwendest.
Datum:
Danke noch einmal an alle, die mir hier geholfen haben. Abgesehen von meinen C Schwächen, lag das Problem einfach mal an der 10µs ISR. Ohne Optimierung bzw. mit einer zusätzlichen Verzögerung in der ISR passte meine Signalabtastung. Ich habe mich um 10µs bei der Signallänge verhauen und das viel mit der nicht mehr alle 10µs ausgeführten ISR nicht auf. Ich habe das alles inzwischen überarbeitet und es läuft genauso wie die ASM Version, allerdings mit 264 Byte mehr Code. Damit kann ich gut leben.